From: Olivier Matz <olivier.matz@6wind.com>
To: "Wu, Wenjun1" <wenjun1.wu@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Yang, Qiming" <qiming.yang@intel.com>,
"Zhao1, Wei" <wei.zhao1@intel.com>
Subject: Re: [PATCH] net/ixgbevf: fix promiscuous and allmulti
Date: Thu, 13 Oct 2022 16:45:43 +0200 [thread overview]
Message-ID: <Y0gkl/wj2scNQWoQ@platinum> (raw)
In-Reply-To: <SA0PR11MB457544D44AA00FA21CB34E8FDF209@SA0PR11MB4575.namprd11.prod.outlook.com>
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
>
next prev parent reply other threads:[~2022-10-13 14:45 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-29 12:21 Olivier Matz
2022-10-10 1:30 ` Wu, Wenjun1
2022-10-13 14:45 ` Olivier Matz [this message]
2022-10-14 1:14 ` Wu, Wenjun1
2022-11-17 4:15 ` Zhang, Qi Z
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y0gkl/wj2scNQWoQ@platinum \
--to=olivier.matz@6wind.com \
--cc=dev@dpdk.org \
--cc=qiming.yang@intel.com \
--cc=wei.zhao1@intel.com \
--cc=wenjun1.wu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).