RE: [PATCH v2] net/i40e: support enabling/disabling source pruning

["Zhang, Ke1X" <ke1x.zhang@intel.com>]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, February 1, 2023 7:11 PM
> To: David Marchand <david.marchand@redhat.com>;
> mb@smartsharesystems.com; ferruh.yigit@amd.com; Zhang, Ke1X
> <ke1x.zhang@intel.com>
> Cc: dev@dpdk.org; Matz, Olivier <olivier.matz@6wind.com>; Zhang, Yuying
> <yuying.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [PATCH v2] net/i40e: support enabling/disabling source pruning
> 
> 31/01/2023 04:28, Zhang, Ke1X:
> > From: David Marchand <david.marchand@redhat.com>
> > > On Mon, Jan 30, 2023 at 9:23 AM Ke Zhang <ke1x.zhang@intel.com>
> wrote:
> > > >
> > > > VRRP advertisement packets are dropped on i40e PF devices 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.
> > > >
> > > > Bugzilla ID: 648
> > > >
> > > > Signed-off-by: Ke Zhang <ke1x.zhang@intel.com>
> > > > ---
> > > >  app/test-pmd/cmdline.c          | 84
> > > +++++++++++++++++++++++++++++++++
> > > >  drivers/net/i40e/i40e_ethdev.c  | 43 +++++++++++++++++
> > > > drivers/net/i40e/i40e_ethdev.h  |  1 +
> > > > drivers/net/i40e/rte_pmd_i40e.c | 20 ++++++++
> > > > drivers/net/i40e/rte_pmd_i40e.h | 16 +++++++
> > > >  5 files changed, 164 insertions(+)
> > > >
> > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > > > cb8c174020..76a574affd 100644
> > > > --- a/app/test-pmd/cmdline.c
> > > > +++ b/app/test-pmd/cmdline.c
> > > > @@ -776,6 +776,9 @@ static void cmd_help_long_parsed(void
> > > > *parsed_result,
> > > >
> > > >                         "port cleanup (port_id) txq (queue_id) (free_cnt)\n"
> > > >                         "    Cleanup txq mbufs for a specific Tx queue\n\n"
> > > > +
> > > > +                       "port config (port_id|all) src_prune (on|off)\n"
> > > > +                       "    Set source prune on port_id, or all.\n\n"
> > > >                 );
> > > >         }
> > > >
> > >
> > > - This seems i40e specific, please move to
> drivers/net/i40e/i40e_testpmd.c.
> > >
> > > - Besides, I would prefer that something in the command name clearly
> > > states this is driver (here, i40e) specific.
> > > Like "port config XX i40e_src_prune" or maybe the other way around,
> > > start with a "driver i40e" prefix.
> > >
> > > Maybe others have an opinion.
> > >
> > >
> > > --
> > > David Marchand
> >
> > Thanks for your comments, this command could be used for other Intel
> > NIC like ice, And It is only finished for i40e in this story:
> > https://jira.devtools.intel.com/browse/DPDK-29564
> 
> Only Intel has access to this URL.
> 
> On the principle, you should not update testpmd commands in a patch for
> i40e.
> We add testpmd commands when adding new features in ethdev.
> 
> Here specifically, you are adding rte_pmd_i40e_set_src_prune() with a
> vague explanation: "Enable/Disable source prune on all the PF."
> Without more information, I cannot judge whether the feature is generic or
> not.
> So for now, this is a nack.
> 
Thanks for your comments, this feature is only for intel NIC.
Would you please share a solution for this case?
Thanks.