[PATCH v3] net/i40e: support enable/disable source pruning

[Morten Brørup]

> From: Morten Brørup
> Sent: Monday, 17 April 2023 12.02

Resent to Luca's other email address. The debian.org address could not be reached due to SPF problems. (Luca has been informed directly.)

> 
> > From: David Marchand [mailto:david.marchand at redhat.com]
> > Sent: Monday, 17 April 2023 09.11
> >
> > On Mon, Apr 17, 2023 at 3:58 AM Mingjin Ye <mingjinx.ye at intel.com> wrote:
> > >
> > > VRRP advertisement packets are dropped on i40e PF device because
> > > when a MAC address is added to a device, packets originating from
> > > that MAC address are dropped.
> > > This patch adds a PMD specific API to enable/disable source
> > > pruning to fix above issue.
> 
> Please amend the above sentence to mention that this patch changes the default
> behavior, e.g.:
> 
> This patch fixes the bug by disabling source pruning by default, and adds a
> PMD specific API to enable/disable source pruning.
> 
> > >
> > > Bugzilla ID: 648
> > >
> > > Fixes: 94b3c1a72507 ("net/i40e: move testpmd commands")
> > > Fixes: bce83974ba2c ("net/i40e: set Tx loopback from PF")
> > > Fixes: 96974a6600ec ("net/i40e: move private APIs to a specific file")
> > > Fixes: 689bba33272d ("i40e: add VEB switching support")
> > > Fixes: 440499cf5376 ("net/i40e: support floating VEB")
> > > Fixes: ef4c16fd9148 ("net/i40e: refactor RSS flow")
> >
> > I see this as a new feature.
> 
> The main purpose of PMD change itself is to fix a bug (with low probability,
> but high impact if no workaround has been implemented in the application).
> 
> Adding the new function to set the source pruning behavior makes the code
> readable.
> 
> >
> > Please don't mark faulty any commit that touched this code (especially
> > the ones that only moved code around).
> > This is confusing.
> >
> >
> > > Cc: stable at dpdk.org
> >
> > Cc: stable maintainers.
> >
> > This commit changes the default behavior, with no way to return to the
> > old behavior (afaics).
> > Backporting it poses a risk of breaking existing applications.
> 
> Correct, but it also fixes a bug that may go unnoticed in applications until
> they are deployed in networks where VRRP is used.
> 
> The default behavior was extremely strange, and the new default behavior makes
> perfect sense.
> 
> Acked-by: Morten Brørup <mb at smartsharesystems.com>
> 
> NB: I wanted to add some arguments in favor of this bugfix, which I consider
> important. Please do not ignore David's warnings!