diff mbox series

[iwl-next,v4,02/13] ice: Prohibit rx mode change in switchdev mode

Message ID 20230524122121.15012-3-wojciech.drewek@intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series ice: switchdev bridge offload | expand

Checks

Context Check Description
netdev/series_format warning Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers warning 6 maintainers not CCed: kuba@kernel.org jesse.brandeburg@intel.com davem@davemloft.net anthony.l.nguyen@intel.com pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Wojciech Drewek May 24, 2023, 12:21 p.m. UTC 
Don't allow to change promisc mode in switchdev mode.
When switchdev is configured, PF netdev is set to be a
default VSI. This is needed for the slow-path to work correctly.
All the unmatched packets will be directed to PF netdev.

It is possible that this setting might be overwritten by
ndo_set_rx_mode. Prevent this by checking if switchdev is
enabled in ice_set_rx_mode.

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Simon Horman June 4, 2023, 1:59 p.m. UTC | #1 
On Wed, May 24, 2023 at 02:21:10PM +0200, Wojciech Drewek wrote:
> Don't allow to change promisc mode in switchdev mode.
> When switchdev is configured, PF netdev is set to be a
> default VSI. This is needed for the slow-path to work correctly.
> All the unmatched packets will be directed to PF netdev.
> 
> It is possible that this setting might be overwritten by
> ndo_set_rx_mode. Prevent this by checking if switchdev is
> enabled in ice_set_rx_mode.
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Buvaneswaran, Sujai June 12, 2023, 7:30 a.m. UTC | #2 
> Don't allow to change promisc mode in switchdev mode.
> When switchdev is configured, PF netdev is set to be a default VSI. This is
> needed for the slow-path to work correctly.
> All the unmatched packets will be directed to PF netdev.
> 
> It is possible that this setting might be overwritten by ndo_set_rx_mode.
> Prevent this by checking if switchdev is enabled in ice_set_rx_mode.
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

During our testing, we found out that PF netdev promisc mode settings can be changed in Switchdev mode.
Is this expected as per the implementation of this patch?

[root@wolfpass-switchdev ~]# ip link show dev ens802f0np0
193: ens802f0np0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq master br0 state UP mode DEFAULT group default qlen 1000
    link/ether 6c:fe:54:5a:18:98 brd ff:ff:ff:ff:ff:ff
    vf 0     link/ether 52:52:00:00:00:01 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
    vf 1     link/ether 52:52:00:00:00:02 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
    altname enp175s0f0np0
[root@wolfpass-switchdev ~]# ip link set dev ens802f0np0 promisc on
[root@wolfpass-switchdev ~]# ip link show dev ens802f0np0
193: ens802f0np0: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500 qdisc mq master br0 state UP mode DEFAULT group default qlen 1000
    link/ether 6c:fe:54:5a:18:98 brd ff:ff:ff:ff:ff:ff
    vf 0     link/ether 52:52:00:00:00:01 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
    vf 1     link/ether 52:52:00:00:00:02 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
    altname enp175s0f0np0
[root@wolfpass-switchdev ~]# ip link set dev ens802f0np0 promisc off
[root@wolfpass-switchdev ~]# ip link show dev ens802f0np0
193: ens802f0np0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq master br0 state UP mode DEFAULT group default qlen 1000
    link/ether 6c:fe:54:5a:18:98 brd ff:ff:ff:ff:ff:ff
    vf 0     link/ether 52:52:00:00:00:01 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
    vf 1     link/ether 52:52:00:00:00:02 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
    altname enp175s0f0np0
Wojciech Drewek June 13, 2023, 9:41 a.m. UTC | #3 
> -----Original Message-----
> From: Buvaneswaran, Sujai <sujai.buvaneswaran@intel.com>
> Sent: poniedziałek, 12 czerwca 2023 09:30
> To: Drewek, Wojciech <wojciech.drewek@intel.com>; intel-wired-lan@lists.osuosl.org
> Cc: pmenzel@molgen.mpg.de; netdev@vger.kernel.org; simon.horman@corigine.com; dan.carpenter@linaro.org
> Subject: RE: [Intel-wired-lan] [PATCH iwl-next v4 02/13] ice: Prohibit rx mode change in switchdev mode
> 
> > Don't allow to change promisc mode in switchdev mode.
> > When switchdev is configured, PF netdev is set to be a default VSI. This is
> > needed for the slow-path to work correctly.
> > All the unmatched packets will be directed to PF netdev.
> >
> > It is possible that this setting might be overwritten by ndo_set_rx_mode.
> > Prevent this by checking if switchdev is enabled in ice_set_rx_mode.
> >
> > Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> > Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> 
> During our testing, we found out that PF netdev promisc mode settings can be changed in Switchdev mode.
> Is this expected as per the implementation of this patch?
> 
> [root@wolfpass-switchdev ~]# ip link show dev ens802f0np0
> 193: ens802f0np0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq master br0 state UP mode DEFAULT group default
> qlen 1000
>     link/ether 6c:fe:54:5a:18:98 brd ff:ff:ff:ff:ff:ff
>     vf 0     link/ether 52:52:00:00:00:01 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
>     vf 1     link/ether 52:52:00:00:00:02 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
>     altname enp175s0f0np0
> [root@wolfpass-switchdev ~]# ip link set dev ens802f0np0 promisc on
> [root@wolfpass-switchdev ~]# ip link show dev ens802f0np0
> 193: ens802f0np0: <BROADCAST,MULTICAST,PROMISC,UP,LOWER_UP> mtu 1500 qdisc mq master br0 state UP mode DEFAULT group
> default qlen 1000
>     link/ether 6c:fe:54:5a:18:98 brd ff:ff:ff:ff:ff:ff
>     vf 0     link/ether 52:52:00:00:00:01 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
>     vf 1     link/ether 52:52:00:00:00:02 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
>     altname enp175s0f0np0
> [root@wolfpass-switchdev ~]# ip link set dev ens802f0np0 promisc off
> [root@wolfpass-switchdev ~]# ip link show dev ens802f0np0
> 193: ens802f0np0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq master br0 state UP mode DEFAULT group default
> qlen 1000
>     link/ether 6c:fe:54:5a:18:98 brd ff:ff:ff:ff:ff:ff
>     vf 0     link/ether 52:52:00:00:00:01 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
>     vf 1     link/ether 52:52:00:00:00:02 brd ff:ff:ff:ff:ff:ff, spoof checking on, link-state enable, trust off
>     altname enp175s0f0np0

I think everything is fine here, actual promisc mode can be checked using -d option, "promiscuity" is indicating if the mode is enabled.
Once the uplink is added to the bridge I couldn't turn it off as expected.
Buvaneswaran, Sujai June 13, 2023, 10:14 a.m. UTC | #4 
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Wojciech Drewek
> Sent: Wednesday, May 24, 2023 5:51 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: pmenzel@molgen.mpg.de; netdev@vger.kernel.org;
> simon.horman@corigine.com; dan.carpenter@linaro.org
> Subject: [Intel-wired-lan] [PATCH iwl-next v4 02/13] ice: Prohibit rx mode
> change in switchdev mode
> 
> Don't allow to change promisc mode in switchdev mode.
> When switchdev is configured, PF netdev is set to be a default VSI. This is
> needed for the slow-path to work correctly.
> All the unmatched packets will be directed to PF netdev.
> 
> It is possible that this setting might be overwritten by ndo_set_rx_mode.
> Prevent this by checking if switchdev is enabled in ice_set_rx_mode.
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
Not able to change the promiscuity to 0 once the interface is added to bridge.

[root@wolfpass-switchdev sbuvanes]# ip -d link show ens802f0np0 | grep promi
    link/ether 6c:fe:54:5a:18:98 brd ff:ff:ff:ff:ff:ff promiscuity 1  allmulti 1 minmtu 68 maxmtu 9702 
[root@wolfpass-switchdev sbuvanes]# ip link set dev ens802f0np0 promisc on
[root@wolfpass-switchdev sbuvanes]# ip -d link show ens802f0np0 | grep promi
    link/ether 6c:fe:54:5a:18:98 brd ff:ff:ff:ff:ff:ff promiscuity 2  allmulti 1 minmtu 68 maxmtu 9702 
[root@wolfpass-switchdev sbuvanes]# ip link set dev ens802f0np0 promisc off
[root@wolfpass-switchdev sbuvanes]# ip -d link show ens802f0np0 | grep promi
    link/ether 6c:fe:54:5a:18:98 brd ff:ff:ff:ff:ff:ff promiscuity 1  allmulti 1 minmtu 68 maxmtu 9702
Buvaneswaran, Sujai June 13, 2023, 10:14 a.m. UTC | #5 
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Wojciech Drewek
> Sent: Wednesday, May 24, 2023 5:51 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: pmenzel@molgen.mpg.de; netdev@vger.kernel.org;
> simon.horman@corigine.com; dan.carpenter@linaro.org
> Subject: [Intel-wired-lan] [PATCH iwl-next v4 02/13] ice: Prohibit rx mode
> change in switchdev mode
> 
> Don't allow to change promisc mode in switchdev mode.
> When switchdev is configured, PF netdev is set to be a default VSI. This is
> needed for the slow-path to work correctly.
> All the unmatched packets will be directed to PF netdev.
> 
> It is possible that this setting might be overwritten by ndo_set_rx_mode.
> Prevent this by checking if switchdev is enabled in ice_set_rx_mode.
> 
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
Tested-by: Sujai Buvaneswaran <sujai.buvaneswaran@intel.com>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index b0d1e6116eb9..d8193de0715a 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5767,7 +5767,7 @@  static void ice_set_rx_mode(struct net_device *netdev)
 	struct ice_netdev_priv *np = netdev_priv(netdev);
 	struct ice_vsi *vsi = np->vsi;
 
-	if (!vsi)
+	if (!vsi || ice_is_switchdev_running(vsi->back))
 		return;
 
 	/* Set the flags to synchronize filters