[PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()

Dipendra Khadka posted 1 patch 2 months ago
There is a newer version of this series
drivers/net/ethernet/broadcom/bcmsysport.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
[PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()
Posted by Dipendra Khadka 2 months ago
Add error pointer checks in bcm_sysport_map_queues() and
bcm_sysport_unmap_queues() after calling dsa_port_from_netdev().

Fixes: 1593cd40d785 ("net: systemport: use standard netdevice notifier to detect DSA presence")
Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
---
v4:
 - Removed wrong and used correct Fixes: tag
v3: https://lore.kernel.org/all/20240924185634.2358-1-kdipendra88@gmail.com/
 - Updated patch subject
 - Updated patch description
 - Added Fixes: tags
 - Fixed typo from PRT_ERR to PTR_ERR
 - Error is checked just after  assignment
v2: https://lore.kernel.org/all/20240923053900.1310-1-kdipendra88@gmail.com/
 - Change the subject of the patch to net
v1: https://lore.kernel.org/all/20240922181739.50056-1-kdipendra88@gmail.com/
 drivers/net/ethernet/broadcom/bcmsysport.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index c9faa8540859..a7ad829f11d4 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -2331,11 +2331,15 @@ static const struct net_device_ops bcm_sysport_netdev_ops = {
 static int bcm_sysport_map_queues(struct net_device *dev,
 				  struct net_device *slave_dev)
 {
-	struct dsa_port *dp = dsa_port_from_netdev(slave_dev);
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	struct bcm_sysport_tx_ring *ring;
 	unsigned int num_tx_queues;
 	unsigned int q, qp, port;
+	struct dsa_port *dp;
+
+	dp = dsa_port_from_netdev(slave_dev);
+	if (IS_ERR(dp))
+		return PTR_ERR(dp);
 
 	/* We can't be setting up queue inspection for non directly attached
 	 * switches
@@ -2386,11 +2390,15 @@ static int bcm_sysport_map_queues(struct net_device *dev,
 static int bcm_sysport_unmap_queues(struct net_device *dev,
 				    struct net_device *slave_dev)
 {
-	struct dsa_port *dp = dsa_port_from_netdev(slave_dev);
 	struct bcm_sysport_priv *priv = netdev_priv(dev);
 	struct bcm_sysport_tx_ring *ring;
 	unsigned int num_tx_queues;
 	unsigned int q, qp, port;
+	struct dsa_port *dp;
+
+	dp = dsa_port_from_netdev(slave_dev);
+	if (IS_ERR(dp))
+		return PTR_ERR(dp));
 
 	port = dp->index;
 
-- 
2.43.0
Re: [PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()
Posted by kernel test robot 2 months ago
Hi Dipendra,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Dipendra-Khadka/net-systemport-Add-error-pointer-checks-in-bcm_sysport_map_queues-and-bcm_sysport_unmap_queues/20240925-233508
base:   net/main
patch link:    https://lore.kernel.org/r/20240925152927.4579-1-kdipendra88%40gmail.com
patch subject: [PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240926/202409261836.eGX8TVKA-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 7773243d9916f98ba0ffce0c3a960e4aa9f03e81)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240926/202409261836.eGX8TVKA-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409261836.eGX8TVKA-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/net/ethernet/broadcom/bcmsysport.c:11:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/net/ethernet/broadcom/bcmsysport.c:11:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/net/ethernet/broadcom/bcmsysport.c:11:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   In file included from drivers/net/ethernet/broadcom/bcmsysport.c:14:
   In file included from include/linux/netdevice.h:38:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:10:
   In file included from include/linux/mm.h:2232:
   include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     517 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> drivers/net/ethernet/broadcom/bcmsysport.c:2401:21: error: expected ';' after return statement
    2401 |                 return PTR_ERR(dp));
         |                                   ^
         |                                   ;
   7 warnings and 1 error generated.


vim +2401 drivers/net/ethernet/broadcom/bcmsysport.c

  2389	
  2390	static int bcm_sysport_unmap_queues(struct net_device *dev,
  2391					    struct net_device *slave_dev)
  2392	{
  2393		struct bcm_sysport_priv *priv = netdev_priv(dev);
  2394		struct bcm_sysport_tx_ring *ring;
  2395		unsigned int num_tx_queues;
  2396		unsigned int q, qp, port;
  2397		struct dsa_port *dp;
  2398	
  2399		dp = dsa_port_from_netdev(slave_dev);
  2400		if (IS_ERR(dp))
> 2401			return PTR_ERR(dp));
  2402	
  2403		port = dp->index;
  2404	
  2405		num_tx_queues = slave_dev->real_num_tx_queues;
  2406	
  2407		for (q = 0; q < dev->num_tx_queues; q++) {
  2408			ring = &priv->tx_rings[q];
  2409	
  2410			if (ring->switch_port != port)
  2411				continue;
  2412	
  2413			if (!ring->inspect)
  2414				continue;
  2415	
  2416			ring->inspect = false;
  2417			qp = ring->switch_queue;
  2418			priv->ring_map[qp + port * num_tx_queues] = NULL;
  2419		}
  2420	
  2421		return 0;
  2422	}
  2423	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()
Posted by kernel test robot 2 months ago
Hi Dipendra,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Dipendra-Khadka/net-systemport-Add-error-pointer-checks-in-bcm_sysport_map_queues-and-bcm_sysport_unmap_queues/20240925-233508
base:   net/main
patch link:    https://lore.kernel.org/r/20240925152927.4579-1-kdipendra88%40gmail.com
patch subject: [PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240926/202409261447.R2kfrGVq-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240926/202409261447.R2kfrGVq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409261447.R2kfrGVq-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   drivers/net/ethernet/broadcom/bcmsysport.c: In function 'bcm_sysport_unmap_queues':
>> drivers/net/ethernet/broadcom/bcmsysport.c:2401:35: error: expected ';' before ')' token
    2401 |                 return PTR_ERR(dp));
         |                                   ^
         |                                   ;
>> drivers/net/ethernet/broadcom/bcmsysport.c:2400:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
    2400 |         if (IS_ERR(dp))
         |         ^~
   drivers/net/ethernet/broadcom/bcmsysport.c:2401:35: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
    2401 |                 return PTR_ERR(dp));
         |                                   ^
>> drivers/net/ethernet/broadcom/bcmsysport.c:2401:35: error: expected statement before ')' token


vim +2401 drivers/net/ethernet/broadcom/bcmsysport.c

  2389	
  2390	static int bcm_sysport_unmap_queues(struct net_device *dev,
  2391					    struct net_device *slave_dev)
  2392	{
  2393		struct bcm_sysport_priv *priv = netdev_priv(dev);
  2394		struct bcm_sysport_tx_ring *ring;
  2395		unsigned int num_tx_queues;
  2396		unsigned int q, qp, port;
  2397		struct dsa_port *dp;
  2398	
  2399		dp = dsa_port_from_netdev(slave_dev);
> 2400		if (IS_ERR(dp))
> 2401			return PTR_ERR(dp));
  2402	
  2403		port = dp->index;
  2404	
  2405		num_tx_queues = slave_dev->real_num_tx_queues;
  2406	
  2407		for (q = 0; q < dev->num_tx_queues; q++) {
  2408			ring = &priv->tx_rings[q];
  2409	
  2410			if (ring->switch_port != port)
  2411				continue;
  2412	
  2413			if (!ring->inspect)
  2414				continue;
  2415	
  2416			ring->inspect = false;
  2417			qp = ring->switch_queue;
  2418			priv->ring_map[qp + port * num_tx_queues] = NULL;
  2419		}
  2420	
  2421		return 0;
  2422	}
  2423	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()
Posted by Dipendra Khadka 2 months ago
I will fix this and will send v5.

On Thu, 26 Sept 2024 at 13:06, kernel test robot <lkp@intel.com> wrote:
>
> Hi Dipendra,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on net/main]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Dipendra-Khadka/net-systemport-Add-error-pointer-checks-in-bcm_sysport_map_queues-and-bcm_sysport_unmap_queues/20240925-233508
> base:   net/main
> patch link:    https://lore.kernel.org/r/20240925152927.4579-1-kdipendra88%40gmail.com
> patch subject: [PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()
> config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240926/202409261447.R2kfrGVq-lkp@intel.com/config)
> compiler: alpha-linux-gcc (GCC) 13.3.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240926/202409261447.R2kfrGVq-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202409261447.R2kfrGVq-lkp@intel.com/
>
> All error/warnings (new ones prefixed by >>):
>
>    drivers/net/ethernet/broadcom/bcmsysport.c: In function 'bcm_sysport_unmap_queues':
> >> drivers/net/ethernet/broadcom/bcmsysport.c:2401:35: error: expected ';' before ')' token
>     2401 |                 return PTR_ERR(dp));
>          |                                   ^
>          |                                   ;
> >> drivers/net/ethernet/broadcom/bcmsysport.c:2400:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
>     2400 |         if (IS_ERR(dp))
>          |         ^~
>    drivers/net/ethernet/broadcom/bcmsysport.c:2401:35: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
>     2401 |                 return PTR_ERR(dp));
>          |                                   ^
> >> drivers/net/ethernet/broadcom/bcmsysport.c:2401:35: error: expected statement before ')' token
>
>
> vim +2401 drivers/net/ethernet/broadcom/bcmsysport.c
>
>   2389
>   2390  static int bcm_sysport_unmap_queues(struct net_device *dev,
>   2391                                      struct net_device *slave_dev)
>   2392  {
>   2393          struct bcm_sysport_priv *priv = netdev_priv(dev);
>   2394          struct bcm_sysport_tx_ring *ring;
>   2395          unsigned int num_tx_queues;
>   2396          unsigned int q, qp, port;
>   2397          struct dsa_port *dp;
>   2398
>   2399          dp = dsa_port_from_netdev(slave_dev);
> > 2400          if (IS_ERR(dp))
> > 2401                  return PTR_ERR(dp));
>   2402
>   2403          port = dp->index;
>   2404
>   2405          num_tx_queues = slave_dev->real_num_tx_queues;
>   2406
>   2407          for (q = 0; q < dev->num_tx_queues; q++) {
>   2408                  ring = &priv->tx_rings[q];
>   2409
>   2410                  if (ring->switch_port != port)
>   2411                          continue;
>   2412
>   2413                  if (!ring->inspect)
>   2414                          continue;
>   2415
>   2416                  ring->inspect = false;
>   2417                  qp = ring->switch_queue;
>   2418                  priv->ring_map[qp + port * num_tx_queues] = NULL;
>   2419          }
>   2420
>   2421          return 0;
>   2422  }
>   2423
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Best Regards,
Dipendra
Re: [PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()
Posted by Simon Horman 2 months ago
On Wed, Sep 25, 2024 at 03:29:26PM +0000, Dipendra Khadka wrote:
> Add error pointer checks in bcm_sysport_map_queues() and
> bcm_sysport_unmap_queues() after calling dsa_port_from_netdev().
> 
> Fixes: 1593cd40d785 ("net: systemport: use standard netdevice notifier to detect DSA presence")
> Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> ---
> v4:
>  - Removed wrong and used correct Fixes: tag
> v3: https://lore.kernel.org/all/20240924185634.2358-1-kdipendra88@gmail.com/
>  - Updated patch subject
>  - Updated patch description
>  - Added Fixes: tags
>  - Fixed typo from PRT_ERR to PTR_ERR
>  - Error is checked just after  assignment
> v2: https://lore.kernel.org/all/20240923053900.1310-1-kdipendra88@gmail.com/
>  - Change the subject of the patch to net
> v1: https://lore.kernel.org/all/20240922181739.50056-1-kdipendra88@gmail.com/

Thanks for all the updates, this version looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>
Re: [PATCH net v4] net: systemport: Add error pointer checks in bcm_sysport_map_queues() and bcm_sysport_unmap_queues()
Posted by Florian Fainelli 2 months ago

On 9/25/2024 8:29 AM, Dipendra Khadka wrote:
> Add error pointer checks in bcm_sysport_map_queues() and
> bcm_sysport_unmap_queues() after calling dsa_port_from_netdev().
> 
> Fixes: 1593cd40d785 ("net: systemport: use standard netdevice notifier to detect DSA presence")
> Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian