[dpdk-dev] [PATCH 2/4] i40e: split function for input set change of hash and fdir

[Chilikin, Andrey]

Hi Jingjing,

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, January 21, 2016 1:29 AM
> To: Chilikin, Andrey; dev at dpdk.org
> Cc: Zhang, Helin; Pei, Yulong; Ananyev, Konstantin
> Subject: RE: [PATCH 2/4] i40e: split function for input set change of hash and
> fdir
> 
> Hi, Andrey
> 
> Thanks for your comments. You are correct, I removed the
> I40E_INSET_FLEX_PAYLOAD from valid fdir input set values, and this is one
> reason why I splited function for input set change of hash and and it is because
> all flex payload configuration can be set in struct rte_fdir_conf during device
> configure phase. And it is a more flexible configuration including flexpayload's
> selection, input set selection by word and mask setting in bits.

Should it be then two patches? First patch to split fdir and hash input set configuration and then second one to remove existing functionality? At the moment it is not obvious that this patch not just splits fdir input set configuration but removes some features in a way that fdir it is not compatible with DPDK 2.2 anymore.

> 
> If I enable it in the input set change API, it will be duplicate. And the input set
> change on flexible payload only on word, just some ability compared with
> rte_fdir_conf.
> If flexible selection isn't done in  struct rte_fdir_conf, the input set selection in
> input set change API doesn't make sense. If flexible selection is done in struct
> rte_fdir_conf, why not selection input set in struct rte_fdir_conf at the same
> time?

I do not have a problem with selecting it at the same time - it always was this way with the legacy systems. But now new NIC supports new way of configuring input set with flexible payload as a part of this input set. So why not to have new way of configuration available as well and change input set using one API call instead of splitting single configuration in to two parts?

> And about you concern, "when application has to run on an old NIC and on a
> new one", The rte_fdir_conf is for each eth_dev, so it will be fine.
> 
> Thanks
> Jingjing

Regards,
Andrey

> 
> 
> > -----Original Message-----
> > From: Chilikin, Andrey
> > Sent: Thursday, January 21, 2016 4:05 AM
> > To: Wu, Jingjing; dev at dpdk.org
> > Cc: Zhang, Helin; Pei, Yulong; Ananyev, Konstantin
> > Subject: RE: [PATCH 2/4] i40e: split function for input set change of
> > hash and fdir
> >
> > Hi Jingjing,
> >
> > As I can see this patch not only splits fdir functionality from common
> > fdir/hash code but also removes compatibility with DPDK 2.2 as it
> > deletes I40E_INSET_FLEX_PAYLOAD from valid fdir input set values. Yes,
> > flexible payload configuration can be set for fdir separately at the
> > port initialization, but this is more legacy from the previous
> > generations of NICs which did not support dynamic input set
> > configuration. I believe it would better to have
> > I40E_INSET_FLEX_PAYLOAD valid for fdir input set same as in DPDK 2.2.
> > So in legacy mode, when application has to run on an old NIC and on a
> > new one, only legacy configuration would be used, but for applications
> targeting new HW single point of configuration would be used instead of mix of
> two.
> >
> > Regards,
> > Andrey
> >