* [dpdk-dev] [PATCH v1] net/ice: revert removing IPID from default hash field
@ 2021-09-07 6:45 Wenjun Wu
2021-09-07 7:09 ` David Marchand
0 siblings, 1 reply; 4+ messages in thread
From: Wenjun Wu @ 2021-09-07 6:45 UTC (permalink / raw)
To: dev, qiming.yang, qi.z.zhang; +Cc: Wenjun Wu
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(-)
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 8d62b84805..0683296584 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -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",
--
2.25.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH v1] net/ice: revert removing IPID from default hash field
2021-09-07 6:45 [dpdk-dev] [PATCH v1] net/ice: revert removing IPID from default hash field Wenjun Wu
@ 2021-09-07 7:09 ` David Marchand
2021-09-07 7:27 ` Wu, Wenjun1
0 siblings, 1 reply; 4+ messages in thread
From: David Marchand @ 2021-09-07 7:09 UTC (permalink / raw)
To: Wenjun Wu, Yigit, Ferruh; +Cc: dev, Qiming Yang, Qi Zhang
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH v1] net/ice: revert removing IPID from default hash field
2021-09-07 7:09 ` David Marchand
@ 2021-09-07 7:27 ` Wu, Wenjun1
2021-09-08 6:01 ` Zhang, Qi Z
0 siblings, 1 reply; 4+ messages in thread
From: Wu, Wenjun1 @ 2021-09-07 7:27 UTC (permalink / raw)
To: David Marchand, Yigit, Ferruh; +Cc: dev, Zhang, Qi Z, Yang, Qiming
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH v1] net/ice: revert removing IPID from default hash field
2021-09-07 7:27 ` Wu, Wenjun1
@ 2021-09-08 6:01 ` Zhang, Qi Z
0 siblings, 0 replies; 4+ messages in thread
From: Zhang, Qi Z @ 2021-09-08 6:01 UTC (permalink / raw)
To: Wu, Wenjun1, David Marchand, Yigit, Ferruh; +Cc: dev, Yang, Qiming
> -----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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-08 6:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-07 6:45 [dpdk-dev] [PATCH v1] net/ice: revert removing IPID from default hash field Wenjun Wu
2021-09-07 7:09 ` David Marchand
2021-09-07 7:27 ` Wu, Wenjun1
2021-09-08 6:01 ` Zhang, Qi Z
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).