* [PATCH] mlxsw: spectrum_switchdev: remove duplicated check on multicast_enable
@ 2018-04-24 11:51 ` Colin King
0 siblings, 0 replies; 6+ messages in thread
From: Colin King @ 2018-04-24 11:51 UTC (permalink / raw)
To: Jiri Pirko, Ido Schimmel, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The check on bridge_port->bridge_device->multicast_enable is performed
twice in two nested if statements. I can't see any reason for this, the
second check is redundant and can be removed.
Detected by cppcheck:
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:1722]:
(warning) Identical inner 'if' condition is always true.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index c11c9a635866..989ed19e25c9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1719,12 +1719,9 @@ __mlxsw_sp_port_mdb_del(struct mlxsw_sp_port *mlxsw_sp_port,
int err;
if (bridge_port->bridge_device->multicast_enabled) {
- if (bridge_port->bridge_device->multicast_enabled) {
- err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid,
- false);
- if (err)
- netdev_err(dev, "Unable to remove port from SMID\n");
- }
+ err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid, false);
+ if (err)
+ netdev_err(dev, "Unable to remove port from SMID\n");
}
err = mlxsw_sp_port_remove_from_mid(mlxsw_sp_port, mid);
--
2.17.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] mlxsw: spectrum_switchdev: remove duplicated check on multicast_enable
@ 2018-04-24 11:51 ` Colin King
0 siblings, 0 replies; 6+ messages in thread
From: Colin King @ 2018-04-24 11:51 UTC (permalink / raw)
To: Jiri Pirko, Ido Schimmel, netdev; +Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The check on bridge_port->bridge_device->multicast_enable is performed
twice in two nested if statements. I can't see any reason for this, the
second check is redundant and can be removed.
Detected by cppcheck:
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:1722]:
(warning) Identical inner 'if' condition is always true.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index c11c9a635866..989ed19e25c9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1719,12 +1719,9 @@ __mlxsw_sp_port_mdb_del(struct mlxsw_sp_port *mlxsw_sp_port,
int err;
if (bridge_port->bridge_device->multicast_enabled) {
- if (bridge_port->bridge_device->multicast_enabled) {
- err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid,
- false);
- if (err)
- netdev_err(dev, "Unable to remove port from SMID\n");
- }
+ err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid, false);
+ if (err)
+ netdev_err(dev, "Unable to remove port from SMID\n");
}
err = mlxsw_sp_port_remove_from_mid(mlxsw_sp_port, mid);
--
2.17.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mlxsw: spectrum_switchdev: remove duplicated check on multicast_enable
2018-04-24 11:51 ` Colin King
@ 2018-04-24 12:51 ` Ido Schimmel
-1 siblings, 0 replies; 6+ messages in thread
From: Ido Schimmel @ 2018-04-24 12:51 UTC (permalink / raw)
To: Colin King
Cc: Jiri Pirko, Ido Schimmel, netdev, kernel-janitors, linux-kernel, nogahf
On Tue, Apr 24, 2018 at 12:51:39PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The check on bridge_port->bridge_device->multicast_enable is performed
> twice in two nested if statements. I can't see any reason for this, the
> second check is redundant and can be removed.
>
> Detected by cppcheck:
> drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:1722]:
> (warning) Identical inner 'if' condition is always true.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> index c11c9a635866..989ed19e25c9 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> @@ -1719,12 +1719,9 @@ __mlxsw_sp_port_mdb_del(struct mlxsw_sp_port *mlxsw_sp_port,
> int err;
>
> if (bridge_port->bridge_device->multicast_enabled) {
> - if (bridge_port->bridge_device->multicast_enabled) {
This looks like a copy-paste error. I believe the intention was to check
'!bridge_port->mrouter' so that if port is an mrouter port packets
hitting the MDB entry would still be sent to it even after it was
removed from the MDB entry ports list.
I will send a patch later this week after I run it by Nogah who worked
on this part.
Thanks for the patch.
> - err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid,
> - false);
> - if (err)
> - netdev_err(dev, "Unable to remove port from SMID\n");
> - }
> + err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid, false);
> + if (err)
> + netdev_err(dev, "Unable to remove port from SMID\n");
> }
>
> err = mlxsw_sp_port_remove_from_mid(mlxsw_sp_port, mid);
> --
> 2.17.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mlxsw: spectrum_switchdev: remove duplicated check on multicast_enable
@ 2018-04-24 12:51 ` Ido Schimmel
0 siblings, 0 replies; 6+ messages in thread
From: Ido Schimmel @ 2018-04-24 12:51 UTC (permalink / raw)
To: Colin King
Cc: Jiri Pirko, Ido Schimmel, netdev, kernel-janitors, linux-kernel, nogahf
On Tue, Apr 24, 2018 at 12:51:39PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The check on bridge_port->bridge_device->multicast_enable is performed
> twice in two nested if statements. I can't see any reason for this, the
> second check is redundant and can be removed.
>
> Detected by cppcheck:
> drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:1722]:
> (warning) Identical inner 'if' condition is always true.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> index c11c9a635866..989ed19e25c9 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> @@ -1719,12 +1719,9 @@ __mlxsw_sp_port_mdb_del(struct mlxsw_sp_port *mlxsw_sp_port,
> int err;
>
> if (bridge_port->bridge_device->multicast_enabled) {
> - if (bridge_port->bridge_device->multicast_enabled) {
This looks like a copy-paste error. I believe the intention was to check
'!bridge_port->mrouter' so that if port is an mrouter port packets
hitting the MDB entry would still be sent to it even after it was
removed from the MDB entry ports list.
I will send a patch later this week after I run it by Nogah who worked
on this part.
Thanks for the patch.
> - err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid,
> - false);
> - if (err)
> - netdev_err(dev, "Unable to remove port from SMID\n");
> - }
> + err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid, false);
> + if (err)
> + netdev_err(dev, "Unable to remove port from SMID\n");
> }
>
> err = mlxsw_sp_port_remove_from_mid(mlxsw_sp_port, mid);
> --
> 2.17.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mlxsw: spectrum_switchdev: remove duplicated check on multicast_enable
2018-04-24 12:51 ` Ido Schimmel
@ 2018-04-24 12:55 ` Colin Ian King
-1 siblings, 0 replies; 6+ messages in thread
From: Colin Ian King @ 2018-04-24 12:55 UTC (permalink / raw)
To: Ido Schimmel
Cc: Jiri Pirko, Ido Schimmel, netdev, kernel-janitors, linux-kernel, nogahf
On 24/04/18 13:51, Ido Schimmel wrote:
> On Tue, Apr 24, 2018 at 12:51:39PM +0100, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The check on bridge_port->bridge_device->multicast_enable is performed
>> twice in two nested if statements. I can't see any reason for this, the
>> second check is redundant and can be removed.
>>
>> Detected by cppcheck:
>> drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:1722]:
>> (warning) Identical inner 'if' condition is always true.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
>> index c11c9a635866..989ed19e25c9 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
>> @@ -1719,12 +1719,9 @@ __mlxsw_sp_port_mdb_del(struct mlxsw_sp_port *mlxsw_sp_port,
>> int err;
>>
>> if (bridge_port->bridge_device->multicast_enabled) {
>> - if (bridge_port->bridge_device->multicast_enabled) {
>
> This looks like a copy-paste error. I believe the intention was to check
> '!bridge_port->mrouter' so that if port is an mrouter port packets
> hitting the MDB entry would still be sent to it even after it was
> removed from the MDB entry ports list.
Ah, that makes a lot more sense.
>
> I will send a patch later this week after I run it by Nogah who worked
> on this part.
OK, thanks!
>
> Thanks for the patch.
>
>> - err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid,
>> - false);
>> - if (err)
>> - netdev_err(dev, "Unable to remove port from SMID\n");
>> - }
>> + err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid, false);
>> + if (err)
>> + netdev_err(dev, "Unable to remove port from SMID\n");
>> }
>>
>> err = mlxsw_sp_port_remove_from_mid(mlxsw_sp_port, mid);
>> --
>> 2.17.0
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mlxsw: spectrum_switchdev: remove duplicated check on multicast_enable
@ 2018-04-24 12:55 ` Colin Ian King
0 siblings, 0 replies; 6+ messages in thread
From: Colin Ian King @ 2018-04-24 12:55 UTC (permalink / raw)
To: Ido Schimmel
Cc: Jiri Pirko, Ido Schimmel, netdev, kernel-janitors, linux-kernel, nogahf
On 24/04/18 13:51, Ido Schimmel wrote:
> On Tue, Apr 24, 2018 at 12:51:39PM +0100, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The check on bridge_port->bridge_device->multicast_enable is performed
>> twice in two nested if statements. I can't see any reason for this, the
>> second check is redundant and can be removed.
>>
>> Detected by cppcheck:
>> drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c:1722]:
>> (warning) Identical inner 'if' condition is always true.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
>> index c11c9a635866..989ed19e25c9 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
>> @@ -1719,12 +1719,9 @@ __mlxsw_sp_port_mdb_del(struct mlxsw_sp_port *mlxsw_sp_port,
>> int err;
>>
>> if (bridge_port->bridge_device->multicast_enabled) {
>> - if (bridge_port->bridge_device->multicast_enabled) {
>
> This looks like a copy-paste error. I believe the intention was to check
> '!bridge_port->mrouter' so that if port is an mrouter port packets
> hitting the MDB entry would still be sent to it even after it was
> removed from the MDB entry ports list.
Ah, that makes a lot more sense.
>
> I will send a patch later this week after I run it by Nogah who worked
> on this part.
OK, thanks!
>
> Thanks for the patch.
>
>> - err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid,
>> - false);
>> - if (err)
>> - netdev_err(dev, "Unable to remove port from SMID\n");
>> - }
>> + err = mlxsw_sp_port_smid_set(mlxsw_sp_port, mid->mid, false);
>> + if (err)
>> + netdev_err(dev, "Unable to remove port from SMID\n");
>> }
>>
>> err = mlxsw_sp_port_remove_from_mid(mlxsw_sp_port, mid);
>> --
>> 2.17.0
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-24 12:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-24 11:51 [PATCH] mlxsw: spectrum_switchdev: remove duplicated check on multicast_enable Colin King
2018-04-24 11:51 ` Colin King
2018-04-24 12:51 ` Ido Schimmel
2018-04-24 12:51 ` Ido Schimmel
2018-04-24 12:55 ` Colin Ian King
2018-04-24 12:55 ` Colin Ian King
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.