[v1] net/ice: revert removing IPID from default hash field
Checks
Commit Message
We try to refine default RSS for IP fragment packets. However,
the change will lead to more serious errors. The scenario that
there is overlap/conflict between the new characteristics and the
existing ones has not been supported, so non-fragment packets
and fragment packets cannot share the same hash fields, or
all related profiles will be removed.
Therefore, IPID field is necessary for fragment packets.
Fixes: cf37e1e5e9d2 ("net/ice: fix default RSS hash for IP fragment packets")
Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
---
drivers/net/ice/ice_ethdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On Tue, Sep 7, 2021 at 9:05 AM Wenjun Wu <wenjun1.wu@intel.com> wrote:
>
> We try to refine default RSS for IP fragment packets. However,
> the change will lead to more serious errors. The scenario that
> there is overlap/conflict between the new characteristics and the
> existing ones has not been supported, so non-fragment packets
> and fragment packets cannot share the same hash fields, or
> all related profiles will be removed.
>
> Therefore, IPID field is necessary for fragment packets.
>
> Fixes: cf37e1e5e9d2 ("net/ice: fix default RSS hash for IP fragment packets")
>
> Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
- If this is a revert of cf37e1e5e9d2, maybe it is simpler to drop the
original change in next-net before it gets pulled in the main repo.
- A similar change has been applied to net/iavf? Is it still relevant?
Default RSS for outer src/dst IP address field in iavf is not supported before, so it does not cause any error.
However, if it can be dropped, I suggest to do so. It seems to be safer to add IPID field here.
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, September 7, 2021 3:10 PM
> To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Cc: dev <dev@dpdk.org>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi
> Z <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1] net/ice: revert removing IPID from
> default hash field
>
> On Tue, Sep 7, 2021 at 9:05 AM Wenjun Wu <wenjun1.wu@intel.com> wrote:
> >
> > We try to refine default RSS for IP fragment packets. However, the
> > change will lead to more serious errors. The scenario that there is
> > overlap/conflict between the new characteristics and the existing ones
> > has not been supported, so non-fragment packets and fragment packets
> > cannot share the same hash fields, or all related profiles will be
> > removed.
> >
> > Therefore, IPID field is necessary for fragment packets.
> >
> > Fixes: cf37e1e5e9d2 ("net/ice: fix default RSS hash for IP fragment
> > packets")
> >
> > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
>
> - If this is a revert of cf37e1e5e9d2, maybe it is simpler to drop the original
> change in next-net before it gets pulled in the main repo.
> - A similar change has been applied to net/iavf? Is it still relevant?
>
>
> --
> David Marchand
> -----Original Message-----
> From: Wu, Wenjun1 <wenjun1.wu@intel.com>
> Sent: Tuesday, September 7, 2021 3:28 PM
> To: David Marchand <david.marchand@redhat.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v1] net/ice: revert removing IPID from default
> hash field
>
> Default RSS for outer src/dst IP address field in iavf is not supported before, so
> it does not cause any error.
> However, if it can be dropped, I suggest to do so. It seems to be safer to add
> IPID field here.
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Tuesday, September 7, 2021 3:10 PM
> > To: Wu, Wenjun1 <wenjun1.wu@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>
> > Cc: dev <dev@dpdk.org>; Yang, Qiming <qiming.yang@intel.com>; Zhang,
> > Qi Z <qi.z.zhang@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v1] net/ice: revert removing IPID from
> > default hash field
> >
> > On Tue, Sep 7, 2021 at 9:05 AM Wenjun Wu <wenjun1.wu@intel.com>
> wrote:
> > >
> > > We try to refine default RSS for IP fragment packets. However, the
> > > change will lead to more serious errors. The scenario that there is
> > > overlap/conflict between the new characteristics and the existing
> > > ones has not been supported, so non-fragment packets and fragment
> > > packets cannot share the same hash fields, or all related profiles
> > > will be removed.
> > >
> > > Therefore, IPID field is necessary for fragment packets.
> > >
> > > Fixes: cf37e1e5e9d2 ("net/ice: fix default RSS hash for IP fragment
> > > packets")
The original fix has no issue, there should be a fix for the real problem, this patch can be rejected after sync with author.
> > >
> > > Signed-off-by: Wenjun Wu <wenjun1.wu@intel.com>
> >
> > - If this is a revert of cf37e1e5e9d2, maybe it is simpler to drop the
> > original change in next-net before it gets pulled in the main repo.
> > - A similar change has been applied to net/iavf? Is it still relevant?
> >
> >
> > --
> > David Marchand
>
@@ -2981,7 +2981,7 @@ ice_rss_hash_set(struct ice_pf *pf, uint64_t rss_hf)
if (rss_hf & ETH_RSS_FRAG_IPV4) {
cfg.addl_hdrs = ICE_FLOW_SEG_HDR_IPV4 | ICE_FLOW_SEG_HDR_IPV_FRAG;
- cfg.hash_flds = ICE_FLOW_HASH_IPV4;
+ cfg.hash_flds = ICE_FLOW_HASH_IPV4 | BIT_ULL(ICE_FLOW_FIELD_IDX_IPV4_ID);
ret = ice_add_rss_cfg_wrap(pf, vsi->idx, &cfg);
if (ret)
PMD_DRV_LOG(ERR, "%s IPV4_FRAG rss flow fail %d",
@@ -2990,7 +2990,7 @@ ice_rss_hash_set(struct ice_pf *pf, uint64_t rss_hf)
if (rss_hf & ETH_RSS_FRAG_IPV6) {
cfg.addl_hdrs = ICE_FLOW_SEG_HDR_IPV6 | ICE_FLOW_SEG_HDR_IPV_FRAG;
- cfg.hash_flds = ICE_FLOW_HASH_IPV6;
+ cfg.hash_flds = ICE_FLOW_HASH_IPV6 | BIT_ULL(ICE_FLOW_FIELD_IDX_IPV6_ID);
ret = ice_add_rss_cfg_wrap(pf, vsi->idx, &cfg);
if (ret)
PMD_DRV_LOG(ERR, "%s IPV6_FRAG rss flow fail %d",