[PATCH] net/ixgbevf: fix promiscuous and allmulti

[PATCH] net/ixgbevf: fix promiscuous and allmulti

[Olivier Matz]

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

RE: [PATCH] net/ixgbevf: fix promiscuous and allmulti

[Wu, Wenjun1]

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

Re: [PATCH] net/ixgbevf: fix promiscuous and allmulti

[Olivier Matz]

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
> 

RE: [PATCH] net/ixgbevf: fix promiscuous and allmulti

[Wu, Wenjun1]

> -----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

RE: [PATCH] net/ixgbevf: fix promiscuous and allmulti

[Zhang, Qi Z]

> -----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