* [PATCH] net/ixgbevf: fix promiscuous and allmulti
@ 2022-09-29 12:21 Olivier Matz
2022-10-10 1:30 ` Wu, Wenjun1
0 siblings, 1 reply; 5+ messages in thread
From: Olivier Matz @ 2022-09-29 12:21 UTC (permalink / raw)
To: dev; +Cc: Qiming Yang, Wenjun Wu, Wei Zhao
The configuration of allmulti and promiscuous modes conflicts
together. For instance, if we enable promiscuous mode, then enable and
disable allmulti, then the promiscuous mode is wrongly disabled.
Fix this behavior by:
- doing nothing when we set/unset allmulti if promiscuous mode is on
- restorting the proper mode (none or allmulti) when we disable
promiscuous mode
Fixes: 1f4564ed7696 ("net/ixgbevf: enable promiscuous mode")
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
Hi,
For reference, this was tested with this plan:
echo 8 > "/sys/bus/pci/devices/0000:01:00.1/sriov_numvfs"
ip link set dev eno2 up
ip link set dev eno2 promisc on
bridge link set dev eno2 hwmode veb
ip link set dev eno2 mtu 9000
ip link set dev eno2 vf 0 mac ac:1f:6b:fe:ba:b0
ip link set dev eno2 vf 0 spoofchk off
ip link set dev eno2 vf 0 trust on
ip link set dev eno2 vf 1 mac ac:1f:6b:fe:ba:b1
ip link set dev eno2 vf 1 spoofchk off
ip link set dev eno2 vf 1 trust on
python3 usertools/dpdk-devbind.py -s
python3 usertools/dpdk-devbind.py -b vfio-pci 0000:01:10.1 # vf 0
python3 usertools/dpdk-devbind.py -b ixgbevf 0000:01:10.3 # vf 1
# in another terminal
scapy
while True:
sendp(Ether(dst='ac:1f:6b:00:00:00'), iface='eno2v1') # wrong mac
sendp(Ether(dst='ac:1f:6b:fe:ba:b0'), iface='eno2v1') # correct mac
time.sleep(1)
./build/app/dpdk-testpmd -l 1,2 -a 0000:01:10.1 -- -i --total-num-mbufs=32768
show port info all
set fwd rxonly
set verbose 1
set promisc all off
set allmulti all off
start
# ok, only packets to dst='ac:1f:6b:fe:ba:b0' are received
# ok, both packets are received
set promisc all on
# nok, only packets to dst='ac:1f:6b:fe:ba:b0' are received
set allmulti all on
set allmulti all off
drivers/net/ixgbe/ixgbe_ethdev.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 8cec951d94..cc8383c5a9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -7785,9 +7785,13 @@ static int
ixgbevf_dev_promiscuous_disable(struct rte_eth_dev *dev)
{
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ int mode = IXGBEVF_XCAST_MODE_NONE;
int ret;
- switch (hw->mac.ops.update_xcast_mode(hw, IXGBEVF_XCAST_MODE_NONE)) {
+ if (dev->data->all_multicast)
+ mode = IXGBEVF_XCAST_MODE_ALLMULTI;
+
+ switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
case IXGBE_SUCCESS:
ret = 0;
break;
@@ -7809,6 +7813,9 @@ ixgbevf_dev_allmulticast_enable(struct rte_eth_dev *dev)
int ret;
int mode = IXGBEVF_XCAST_MODE_ALLMULTI;
+ if (dev->data->promiscuous)
+ return 0;
+
switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
case IXGBE_SUCCESS:
ret = 0;
@@ -7830,6 +7837,9 @@ ixgbevf_dev_allmulticast_disable(struct rte_eth_dev *dev)
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
int ret;
+ if (dev->data->promiscuous)
+ return 0;
+
switch (hw->mac.ops.update_xcast_mode(hw, IXGBEVF_XCAST_MODE_MULTI)) {
case IXGBE_SUCCESS:
ret = 0;
--
2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] net/ixgbevf: fix promiscuous and allmulti
2022-09-29 12:21 [PATCH] net/ixgbevf: fix promiscuous and allmulti Olivier Matz
@ 2022-10-10 1:30 ` Wu, Wenjun1
2022-10-13 14:45 ` Olivier Matz
0 siblings, 1 reply; 5+ messages in thread
From: Wu, Wenjun1 @ 2022-10-10 1:30 UTC (permalink / raw)
To: Matz, Olivier, dev; +Cc: Yang, Qiming, Zhao1, Wei
Hi Olivier,
> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Thursday, September 29, 2022 8:22 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>; Wu, Wenjun1
> <wenjun1.wu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [PATCH] net/ixgbevf: fix promiscuous and allmulti
>
> The configuration of allmulti and promiscuous modes conflicts together. For
> instance, if we enable promiscuous mode, then enable and disable allmulti,
> then the promiscuous mode is wrongly disabled.
>
> Fix this behavior by:
> - doing nothing when we set/unset allmulti if promiscuous mode is on
> - restorting the proper mode (none or allmulti) when we disable
> promiscuous mode
>
> Fixes: 1f4564ed7696 ("net/ixgbevf: enable promiscuous mode")
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>
> Hi,
>
> For reference, this was tested with this plan:
>
> echo 8 > "/sys/bus/pci/devices/0000:01:00.1/sriov_numvfs"
> ip link set dev eno2 up
> ip link set dev eno2 promisc on
> bridge link set dev eno2 hwmode veb
> ip link set dev eno2 mtu 9000
>
> ip link set dev eno2 vf 0 mac ac:1f:6b:fe:ba:b0 ip link set dev eno2 vf 0
> spoofchk off ip link set dev eno2 vf 0 trust on
>
> ip link set dev eno2 vf 1 mac ac:1f:6b:fe:ba:b1 ip link set dev eno2 vf 1
> spoofchk off ip link set dev eno2 vf 1 trust on
>
> python3 usertools/dpdk-devbind.py -s
> python3 usertools/dpdk-devbind.py -b vfio-pci 0000:01:10.1 # vf 0
> python3 usertools/dpdk-devbind.py -b ixgbevf 0000:01:10.3 # vf 1
>
>
> # in another terminal
> scapy
> while True:
> sendp(Ether(dst='ac:1f:6b:00:00:00'), iface='eno2v1') # wrong mac
> sendp(Ether(dst='ac:1f:6b:fe:ba:b0'), iface='eno2v1') # correct mac
> time.sleep(1)
>
>
> ./build/app/dpdk-testpmd -l 1,2 -a 0000:01:10.1 -- -i --total-num-
> mbufs=32768 show port info all set fwd rxonly set verbose 1 set promisc all
> off set allmulti all off start
>
> # ok, only packets to dst='ac:1f:6b:fe:ba:b0' are received
>
>
> # ok, both packets are received
> set promisc all on
>
>
> # nok, only packets to dst='ac:1f:6b:fe:ba:b0' are received set allmulti all on
> set allmulti all off
>
>
> drivers/net/ixgbe/ixgbe_ethdev.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 8cec951d94..cc8383c5a9 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -7785,9 +7785,13 @@ static int
> ixgbevf_dev_promiscuous_disable(struct rte_eth_dev *dev) {
> struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> + int mode = IXGBEVF_XCAST_MODE_NONE;
> int ret;
>
> - switch (hw->mac.ops.update_xcast_mode(hw,
> IXGBEVF_XCAST_MODE_NONE)) {
> + if (dev->data->all_multicast)
> + mode = IXGBEVF_XCAST_MODE_ALLMULTI;
> +
> + switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
> case IXGBE_SUCCESS:
> ret = 0;
> break;
> @@ -7809,6 +7813,9 @@ ixgbevf_dev_allmulticast_enable(struct
> rte_eth_dev *dev)
> int ret;
> int mode = IXGBEVF_XCAST_MODE_ALLMULTI;
>
> + if (dev->data->promiscuous)
> + return 0;
> +
> switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
> case IXGBE_SUCCESS:
> ret = 0;
> @@ -7830,6 +7837,9 @@ ixgbevf_dev_allmulticast_disable(struct
> rte_eth_dev *dev)
> struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> int ret;
>
> + if (dev->data->promiscuous)
> + return 0;
> +
It seems that we cannot actually turn off allmulticast mode when promiscuous
mode is enabled, so can we return error and add a log message here as a
reminder?
Thanks,
Wenjun
> switch (hw->mac.ops.update_xcast_mode(hw,
> IXGBEVF_XCAST_MODE_MULTI)) {
> case IXGBE_SUCCESS:
> ret = 0;
> --
> 2.30.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net/ixgbevf: fix promiscuous and allmulti
2022-10-10 1:30 ` Wu, Wenjun1
@ 2022-10-13 14:45 ` Olivier Matz
2022-10-14 1:14 ` Wu, Wenjun1
0 siblings, 1 reply; 5+ messages in thread
From: Olivier Matz @ 2022-10-13 14:45 UTC (permalink / raw)
To: Wu, Wenjun1; +Cc: dev, Yang, Qiming, Zhao1, Wei
Hi Wenjun,
On Mon, Oct 10, 2022 at 01:30:54AM +0000, Wu, Wenjun1 wrote:
> Hi Olivier,
>
> > -----Original Message-----
> > From: Olivier Matz <olivier.matz@6wind.com>
> > Sent: Thursday, September 29, 2022 8:22 PM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; Wu, Wenjun1
> > <wenjun1.wu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> > Subject: [PATCH] net/ixgbevf: fix promiscuous and allmulti
> >
> > The configuration of allmulti and promiscuous modes conflicts together. For
> > instance, if we enable promiscuous mode, then enable and disable allmulti,
> > then the promiscuous mode is wrongly disabled.
> >
> > Fix this behavior by:
> > - doing nothing when we set/unset allmulti if promiscuous mode is on
> > - restorting the proper mode (none or allmulti) when we disable
> > promiscuous mode
> >
> > Fixes: 1f4564ed7696 ("net/ixgbevf: enable promiscuous mode")
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > ---
> >
> > Hi,
> >
> > For reference, this was tested with this plan:
> >
> > echo 8 > "/sys/bus/pci/devices/0000:01:00.1/sriov_numvfs"
> > ip link set dev eno2 up
> > ip link set dev eno2 promisc on
> > bridge link set dev eno2 hwmode veb
> > ip link set dev eno2 mtu 9000
> >
> > ip link set dev eno2 vf 0 mac ac:1f:6b:fe:ba:b0 ip link set dev eno2 vf 0
> > spoofchk off ip link set dev eno2 vf 0 trust on
> >
> > ip link set dev eno2 vf 1 mac ac:1f:6b:fe:ba:b1 ip link set dev eno2 vf 1
> > spoofchk off ip link set dev eno2 vf 1 trust on
> >
> > python3 usertools/dpdk-devbind.py -s
> > python3 usertools/dpdk-devbind.py -b vfio-pci 0000:01:10.1 # vf 0
> > python3 usertools/dpdk-devbind.py -b ixgbevf 0000:01:10.3 # vf 1
> >
> >
> > # in another terminal
> > scapy
> > while True:
> > sendp(Ether(dst='ac:1f:6b:00:00:00'), iface='eno2v1') # wrong mac
> > sendp(Ether(dst='ac:1f:6b:fe:ba:b0'), iface='eno2v1') # correct mac
> > time.sleep(1)
> >
> >
> > ./build/app/dpdk-testpmd -l 1,2 -a 0000:01:10.1 -- -i --total-num-
> > mbufs=32768 show port info all set fwd rxonly set verbose 1 set promisc all
> > off set allmulti all off start
> >
> > # ok, only packets to dst='ac:1f:6b:fe:ba:b0' are received
> >
> >
> > # ok, both packets are received
> > set promisc all on
> >
> >
> > # nok, only packets to dst='ac:1f:6b:fe:ba:b0' are received set allmulti all on
> > set allmulti all off
> >
> >
> > drivers/net/ixgbe/ixgbe_ethdev.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 8cec951d94..cc8383c5a9 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -7785,9 +7785,13 @@ static int
> > ixgbevf_dev_promiscuous_disable(struct rte_eth_dev *dev) {
> > struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > + int mode = IXGBEVF_XCAST_MODE_NONE;
> > int ret;
> >
> > - switch (hw->mac.ops.update_xcast_mode(hw,
> > IXGBEVF_XCAST_MODE_NONE)) {
> > + if (dev->data->all_multicast)
> > + mode = IXGBEVF_XCAST_MODE_ALLMULTI;
> > +
> > + switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
> > case IXGBE_SUCCESS:
> > ret = 0;
> > break;
> > @@ -7809,6 +7813,9 @@ ixgbevf_dev_allmulticast_enable(struct
> > rte_eth_dev *dev)
> > int ret;
> > int mode = IXGBEVF_XCAST_MODE_ALLMULTI;
> >
> > + if (dev->data->promiscuous)
> > + return 0;
> > +
> > switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
> > case IXGBE_SUCCESS:
> > ret = 0;
> > @@ -7830,6 +7837,9 @@ ixgbevf_dev_allmulticast_disable(struct
> > rte_eth_dev *dev)
> > struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > int ret;
> >
> > + if (dev->data->promiscuous)
> > + return 0;
> > +
>
> It seems that we cannot actually turn off allmulticast mode when promiscuous
> mode is enabled, so can we return error and add a log message here as a
> reminder?
I think we should not return an error here: when we disable all_multi, there are
2 cases:
1/ promiscuous is off: no issue here, we do as before, we ask the PF to disable
the all_multi mode
2/ promiscuous is on: we have nothing to ask to the PF, because we still want
to stay in promisc mode. We just need to remember that all_multi is disabled,
and this is done already done by the ethdev layer.
On the PF dpdk driver, the behavior is the same: we can enable or disable
promisc and all_multi independently, and changing the all_multi value when
promisc is enabled is allowed and does not impact the rx traffic.
Thanks,
Olivier
> Thanks,
> Wenjun
>
> > switch (hw->mac.ops.update_xcast_mode(hw,
> > IXGBEVF_XCAST_MODE_MULTI)) {
> > case IXGBE_SUCCESS:
> > ret = 0;
> > --
> > 2.30.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] net/ixgbevf: fix promiscuous and allmulti
2022-10-13 14:45 ` Olivier Matz
@ 2022-10-14 1:14 ` Wu, Wenjun1
2022-11-17 4:15 ` Zhang, Qi Z
0 siblings, 1 reply; 5+ messages in thread
From: Wu, Wenjun1 @ 2022-10-14 1:14 UTC (permalink / raw)
To: Matz, Olivier; +Cc: dev, Yang, Qiming, Zhao1, Wei
> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Thursday, October 13, 2022 10:46 PM
> To: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhao1, Wei
> <wei.zhao1@intel.com>
> Subject: Re: [PATCH] net/ixgbevf: fix promiscuous and allmulti
>
> Hi Wenjun,
>
> On Mon, Oct 10, 2022 at 01:30:54AM +0000, Wu, Wenjun1 wrote:
> > Hi Olivier,
> >
> > > -----Original Message-----
> > > From: Olivier Matz <olivier.matz@6wind.com>
> > > Sent: Thursday, September 29, 2022 8:22 PM
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming <qiming.yang@intel.com>; Wu, Wenjun1
> > > <wenjun1.wu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> > > Subject: [PATCH] net/ixgbevf: fix promiscuous and allmulti
> > >
> > > The configuration of allmulti and promiscuous modes conflicts
> > > together. For instance, if we enable promiscuous mode, then enable
> > > and disable allmulti, then the promiscuous mode is wrongly disabled.
> > >
> > > Fix this behavior by:
> > > - doing nothing when we set/unset allmulti if promiscuous mode is on
> > > - restorting the proper mode (none or allmulti) when we disable
> > > promiscuous mode
> > >
> > > Fixes: 1f4564ed7696 ("net/ixgbevf: enable promiscuous mode")
> > >
> > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > ---
> > >
> > > Hi,
> > >
> > > For reference, this was tested with this plan:
> > >
> > > echo 8 > "/sys/bus/pci/devices/0000:01:00.1/sriov_numvfs"
> > > ip link set dev eno2 up
> > > ip link set dev eno2 promisc on
> > > bridge link set dev eno2 hwmode veb
> > > ip link set dev eno2 mtu 9000
> > >
> > > ip link set dev eno2 vf 0 mac ac:1f:6b:fe:ba:b0 ip link set dev eno2
> > > vf 0 spoofchk off ip link set dev eno2 vf 0 trust on
> > >
> > > ip link set dev eno2 vf 1 mac ac:1f:6b:fe:ba:b1 ip link set dev eno2
> > > vf 1 spoofchk off ip link set dev eno2 vf 1 trust on
> > >
> > > python3 usertools/dpdk-devbind.py -s
> > > python3 usertools/dpdk-devbind.py -b vfio-pci 0000:01:10.1 # vf 0
> > > python3 usertools/dpdk-devbind.py -b ixgbevf 0000:01:10.3 # vf 1
> > >
> > >
> > > # in another terminal
> > > scapy
> > > while True:
> > > sendp(Ether(dst='ac:1f:6b:00:00:00'), iface='eno2v1') # wrong mac
> > > sendp(Ether(dst='ac:1f:6b:fe:ba:b0'), iface='eno2v1') # correct mac
> > > time.sleep(1)
> > >
> > >
> > > ./build/app/dpdk-testpmd -l 1,2 -a 0000:01:10.1 -- -i --total-num-
> > > mbufs=32768 show port info all set fwd rxonly set verbose 1 set
> > > promisc all off set allmulti all off start
> > >
> > > # ok, only packets to dst='ac:1f:6b:fe:ba:b0' are received
> > >
> > >
> > > # ok, both packets are received
> > > set promisc all on
> > >
> > >
> > > # nok, only packets to dst='ac:1f:6b:fe:ba:b0' are received set
> > > allmulti all on set allmulti all off
> > >
> > >
> > > drivers/net/ixgbe/ixgbe_ethdev.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > index 8cec951d94..cc8383c5a9 100644
> > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > @@ -7785,9 +7785,13 @@ static int
> > > ixgbevf_dev_promiscuous_disable(struct rte_eth_dev *dev) {
> > > struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > > >dev_private);
> > > + int mode = IXGBEVF_XCAST_MODE_NONE;
> > > int ret;
> > >
> > > - switch (hw->mac.ops.update_xcast_mode(hw,
> > > IXGBEVF_XCAST_MODE_NONE)) {
> > > + if (dev->data->all_multicast)
> > > + mode = IXGBEVF_XCAST_MODE_ALLMULTI;
> > > +
> > > + switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
> > > case IXGBE_SUCCESS:
> > > ret = 0;
> > > break;
> > > @@ -7809,6 +7813,9 @@ ixgbevf_dev_allmulticast_enable(struct
> > > rte_eth_dev *dev)
> > > int ret;
> > > int mode = IXGBEVF_XCAST_MODE_ALLMULTI;
> > >
> > > + if (dev->data->promiscuous)
> > > + return 0;
> > > +
> > > switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
> > > case IXGBE_SUCCESS:
> > > ret = 0;
> > > @@ -7830,6 +7837,9 @@ ixgbevf_dev_allmulticast_disable(struct
> > > rte_eth_dev *dev)
> > > struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > > >dev_private);
> > > int ret;
> > >
> > > + if (dev->data->promiscuous)
> > > + return 0;
> > > +
> >
> > It seems that we cannot actually turn off allmulticast mode when
> > promiscuous mode is enabled, so can we return error and add a log
> > message here as a reminder?
>
> I think we should not return an error here: when we disable all_multi, there
> are
> 2 cases:
>
> 1/ promiscuous is off: no issue here, we do as before, we ask the PF to
> disable
> the all_multi mode
>
> 2/ promiscuous is on: we have nothing to ask to the PF, because we still want
> to stay in promisc mode. We just need to remember that all_multi is
> disabled,
> and this is done already done by the ethdev layer.
>
> On the PF dpdk driver, the behavior is the same: we can enable or disable
> promisc and all_multi independently, and changing the all_multi value when
> promisc is enabled is allowed and does not impact the rx traffic.
>
>
> Thanks,
> Olivier
>
No more concern if the behavior is consistent.
> > Thanks,
> > Wenjun
> >
> > > switch (hw->mac.ops.update_xcast_mode(hw,
> > > IXGBEVF_XCAST_MODE_MULTI)) {
> > > case IXGBE_SUCCESS:
> > > ret = 0;
> > > --
> > > 2.30.2
> >
Acked-by: Wenjun Wu <wenjun1.wu@intel.com>
Thanks,
Wenjun
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] net/ixgbevf: fix promiscuous and allmulti
2022-10-14 1:14 ` Wu, Wenjun1
@ 2022-11-17 4:15 ` Zhang, Qi Z
0 siblings, 0 replies; 5+ messages in thread
From: Zhang, Qi Z @ 2022-11-17 4:15 UTC (permalink / raw)
To: Wu, Wenjun1, Matz, Olivier; +Cc: dev, Yang, Qiming, Zhao1, Wei
> -----Original Message-----
> From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Sent: Friday, October 14, 2022 9:14 AM
> To: Matz, Olivier <olivier.matz@6wind.com>
> Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhao1, Wei
> <wei.zhao1@intel.com>
> Subject: RE: [PATCH] net/ixgbevf: fix promiscuous and allmulti
>
>
>
> > -----Original Message-----
> > From: Olivier Matz <olivier.matz@6wind.com>
> > Sent: Thursday, October 13, 2022 10:46 PM
> > To: Wu, Wenjun1 <wenjun1.wu@intel.com>
> > Cc: dev@dpdk.org; Yang, Qiming <qiming.yang@intel.com>; Zhao1, Wei
> > <wei.zhao1@intel.com>
> > Subject: Re: [PATCH] net/ixgbevf: fix promiscuous and allmulti
> >
> > Hi Wenjun,
> >
> > On Mon, Oct 10, 2022 at 01:30:54AM +0000, Wu, Wenjun1 wrote:
> > > Hi Olivier,
> > >
> > > > -----Original Message-----
> > > > From: Olivier Matz <olivier.matz@6wind.com>
> > > > Sent: Thursday, September 29, 2022 8:22 PM
> > > > To: dev@dpdk.org
> > > > Cc: Yang, Qiming <qiming.yang@intel.com>; Wu, Wenjun1
> > > > <wenjun1.wu@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> > > > Subject: [PATCH] net/ixgbevf: fix promiscuous and allmulti
> > > >
> > > > The configuration of allmulti and promiscuous modes conflicts
> > > > together. For instance, if we enable promiscuous mode, then enable
> > > > and disable allmulti, then the promiscuous mode is wrongly disabled.
> > > >
> > > > Fix this behavior by:
> > > > - doing nothing when we set/unset allmulti if promiscuous mode is
> > > > on
> > > > - restorting the proper mode (none or allmulti) when we disable
> > > > promiscuous mode
> > > >
> > > > Fixes: 1f4564ed7696 ("net/ixgbevf: enable promiscuous mode")
> > > >
> > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > ---
> > > >
> > > > Hi,
> > > >
> > > > For reference, this was tested with this plan:
> > > >
> > > > echo 8 > "/sys/bus/pci/devices/0000:01:00.1/sriov_numvfs"
> > > > ip link set dev eno2 up
> > > > ip link set dev eno2 promisc on
> > > > bridge link set dev eno2 hwmode veb ip link set dev eno2 mtu 9000
> > > >
> > > > ip link set dev eno2 vf 0 mac ac:1f:6b:fe:ba:b0 ip link set dev
> > > > eno2 vf 0 spoofchk off ip link set dev eno2 vf 0 trust on
> > > >
> > > > ip link set dev eno2 vf 1 mac ac:1f:6b:fe:ba:b1 ip link set dev
> > > > eno2 vf 1 spoofchk off ip link set dev eno2 vf 1 trust on
> > > >
> > > > python3 usertools/dpdk-devbind.py -s
> > > > python3 usertools/dpdk-devbind.py -b vfio-pci 0000:01:10.1 # vf 0
> > > > python3 usertools/dpdk-devbind.py -b ixgbevf 0000:01:10.3 # vf 1
> > > >
> > > >
> > > > # in another terminal
> > > > scapy
> > > > while True:
> > > > sendp(Ether(dst='ac:1f:6b:00:00:00'), iface='eno2v1') # wrong mac
> > > > sendp(Ether(dst='ac:1f:6b:fe:ba:b0'), iface='eno2v1') # correct mac
> > > > time.sleep(1)
> > > >
> > > >
> > > > ./build/app/dpdk-testpmd -l 1,2 -a 0000:01:10.1 -- -i --total-num-
> > > > mbufs=32768 show port info all set fwd rxonly set verbose 1 set
> > > > promisc all off set allmulti all off start
> > > >
> > > > # ok, only packets to dst='ac:1f:6b:fe:ba:b0' are received
> > > >
> > > >
> > > > # ok, both packets are received
> > > > set promisc all on
> > > >
> > > >
> > > > # nok, only packets to dst='ac:1f:6b:fe:ba:b0' are received set
> > > > allmulti all on set allmulti all off
> > > >
> > > >
> > > > drivers/net/ixgbe/ixgbe_ethdev.c | 12 +++++++++++-
> > > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > > index 8cec951d94..cc8383c5a9 100644
> > > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > > > @@ -7785,9 +7785,13 @@ static int
> > > > ixgbevf_dev_promiscuous_disable(struct rte_eth_dev *dev) {
> > > > struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > > > >dev_private);
> > > > + int mode = IXGBEVF_XCAST_MODE_NONE;
> > > > int ret;
> > > >
> > > > - switch (hw->mac.ops.update_xcast_mode(hw,
> > > > IXGBEVF_XCAST_MODE_NONE)) {
> > > > + if (dev->data->all_multicast)
> > > > + mode = IXGBEVF_XCAST_MODE_ALLMULTI;
> > > > +
> > > > + switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
> > > > case IXGBE_SUCCESS:
> > > > ret = 0;
> > > > break;
> > > > @@ -7809,6 +7813,9 @@ ixgbevf_dev_allmulticast_enable(struct
> > > > rte_eth_dev *dev)
> > > > int ret;
> > > > int mode = IXGBEVF_XCAST_MODE_ALLMULTI;
> > > >
> > > > + if (dev->data->promiscuous)
> > > > + return 0;
> > > > +
> > > > switch (hw->mac.ops.update_xcast_mode(hw, mode)) {
> > > > case IXGBE_SUCCESS:
> > > > ret = 0;
> > > > @@ -7830,6 +7837,9 @@ ixgbevf_dev_allmulticast_disable(struct
> > > > rte_eth_dev *dev)
> > > > struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > > > >dev_private);
> > > > int ret;
> > > >
> > > > + if (dev->data->promiscuous)
> > > > + return 0;
> > > > +
> > >
> > > It seems that we cannot actually turn off allmulticast mode when
> > > promiscuous mode is enabled, so can we return error and add a log
> > > message here as a reminder?
> >
> > I think we should not return an error here: when we disable all_multi,
> > there are
> > 2 cases:
> >
> > 1/ promiscuous is off: no issue here, we do as before, we ask the PF
> > to disable
> > the all_multi mode
> >
> > 2/ promiscuous is on: we have nothing to ask to the PF, because we still
> want
> > to stay in promisc mode. We just need to remember that all_multi is
> > disabled,
> > and this is done already done by the ethdev layer.
> >
> > On the PF dpdk driver, the behavior is the same: we can enable or
> > disable promisc and all_multi independently, and changing the
> > all_multi value when promisc is enabled is allowed and does not impact the
> rx traffic.
> >
> >
> > Thanks,
> > Olivier
> >
>
> No more concern if the behavior is consistent.
>
> > > Thanks,
> > > Wenjun
> > >
> > > > switch (hw->mac.ops.update_xcast_mode(hw,
> > > > IXGBEVF_XCAST_MODE_MULTI)) {
> > > > case IXGBE_SUCCESS:
> > > > ret = 0;
> > > > --
> > > > 2.30.2
> > >
>
> Acked-by: Wenjun Wu <wenjun1.wu@intel.com>
Applied to dpdk-next-net-intel.
Thanks
Qi
>
> Thanks,
> Wenjun
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-11-17 4:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29 12:21 [PATCH] net/ixgbevf: fix promiscuous and allmulti Olivier Matz
2022-10-10 1:30 ` Wu, Wenjun1
2022-10-13 14:45 ` Olivier Matz
2022-10-14 1:14 ` Wu, Wenjun1
2022-11-17 4:15 ` Zhang, Qi Z
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).