Re: [ovs-dev] [PATCH 1/3] netdev-offload-dpdk: Set transfer attribute to zero for mark/rss offload
On Mon, Jul 13, 2020 at 1:28 PM Eli Britstein wrote:
>
>
> On 7/13/2020 10:55 AM, Sriharsha Basavapatna wrote:
> > On Mon, Jul 13, 2020 at 1:07 PM Eli Britstein wrote:
> >>
> >> On 7/10/2020 3:07 PM, Sriharsha Basavapatna wrote:
> >>> The offload layer doesn't initialize the 'transfer' attribute
> >>> for mark/rss offload (partial offload). It should be set to 0.
> >>>
> >>> Fixes: 60e778c7533a ("netdev-offload-dpdk: Framework for actions
> >>> offload.")
> >> It is not a bug. .ingress = 1 is also sufficient.
> > ingress and transfer are different attributes.
> Need to initialize only the non-zero fields. Others are implicitly
> initialized to zero.
> >
> >> See
> >> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.gnu.org%2Fsoftware%2Fgnu-c-manual%2Fgnu-c-manual.html%23Initializing-Structure-Members&data=02%7C01%7Celibr%40mellanox.com%7Cadfaff15fd8e4cec042808d827021b34%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637302237280357657&sdata=KCkgeMHqrVUtVkPvciFB4cLO%2FiN%2BTP9WZmcaXMf2LqY%3D&reserved=0
> > Let us set it explicitly to 0, like other members (group etc).
> >> Anyway, this is the commit that introduced it.
> >>
> >> e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
> Remove the fixes. It doesn't fix anything. You can explain to make it
> explicit in the commit msg.
I thought about it, the concern I had was uninitialized value for the
transfer field. Like you pointed out, since it is not an issue (even
for local struct members), I'll drop this patch. I updated the second
patch (L4 proto id) with the right 'fixes' as per your previous
comment. Did you look at the 3rd patch in this set ? any comments ?
Thanks,
-Harsha
> > Ok, thanks.
> > -Harsha
> >
> >>> Signed-off-by: Sriharsha Basavapatna
> >>> ---
> >>>lib/netdev-offload-dpdk.c | 3 ++-
> >>>1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >>> index 26a75f0f2..4c652fd82 100644
> >>> --- a/lib/netdev-offload-dpdk.c
> >>> +++ b/lib/netdev-offload-dpdk.c
> >>> @@ -818,7 +818,8 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns
> >>> *patterns,
> >>>.group = 0,
> >>>.priority = 0,
> >>>.ingress = 1,
> >>> -.egress = 0
> >>> +.egress = 0,
> >>> +.transfer = 0
> >>>};
> >>>struct rte_flow_error error;
> >>>struct rte_flow *flow;
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/3] netdev-offload-dpdk: Set transfer attribute to zero for mark/rss offload
On 7/13/2020 10:55 AM, Sriharsha Basavapatna wrote:
On Mon, Jul 13, 2020 at 1:07 PM Eli Britstein wrote:
On 7/10/2020 3:07 PM, Sriharsha Basavapatna wrote:
The offload layer doesn't initialize the 'transfer' attribute
for mark/rss offload (partial offload). It should be set to 0.
Fixes: 60e778c7533a ("netdev-offload-dpdk: Framework for actions offload.")
It is not a bug. .ingress = 1 is also sufficient.
ingress and transfer are different attributes.
Need to initialize only the non-zero fields. Others are implicitly
initialized to zero.
See
https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.gnu.org%2Fsoftware%2Fgnu-c-manual%2Fgnu-c-manual.html%23Initializing-Structure-Members&data=02%7C01%7Celibr%40mellanox.com%7Cadfaff15fd8e4cec042808d827021b34%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637302237280357657&sdata=KCkgeMHqrVUtVkPvciFB4cLO%2FiN%2BTP9WZmcaXMf2LqY%3D&reserved=0
Let us set it explicitly to 0, like other members (group etc).
Anyway, this is the commit that introduced it.
e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
Remove the fixes. It doesn't fix anything. You can explain to make it
explicit in the commit msg.
Ok, thanks.
-Harsha
Signed-off-by: Sriharsha Basavapatna
---
lib/netdev-offload-dpdk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 26a75f0f2..4c652fd82 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -818,7 +818,8 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
.group = 0,
.priority = 0,
.ingress = 1,
-.egress = 0
+.egress = 0,
+.transfer = 0
};
struct rte_flow_error error;
struct rte_flow *flow;
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/3] netdev-offload-dpdk: Set transfer attribute to zero for mark/rss offload
On Mon, Jul 13, 2020 at 1:07 PM Eli Britstein wrote:
>
>
> On 7/10/2020 3:07 PM, Sriharsha Basavapatna wrote:
> > The offload layer doesn't initialize the 'transfer' attribute
> > for mark/rss offload (partial offload). It should be set to 0.
> >
> > Fixes: 60e778c7533a ("netdev-offload-dpdk: Framework for actions offload.")
>
> It is not a bug. .ingress = 1 is also sufficient.
ingress and transfer are different attributes.
>
> See
> http://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Initializing-Structure-Members
Let us set it explicitly to 0, like other members (group etc).
>
> Anyway, this is the commit that introduced it.
>
> e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
Ok, thanks.
-Harsha
>
> > Signed-off-by: Sriharsha Basavapatna
> > ---
> > lib/netdev-offload-dpdk.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> > index 26a75f0f2..4c652fd82 100644
> > --- a/lib/netdev-offload-dpdk.c
> > +++ b/lib/netdev-offload-dpdk.c
> > @@ -818,7 +818,8 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns
> > *patterns,
> > .group = 0,
> > .priority = 0,
> > .ingress = 1,
> > -.egress = 0
> > +.egress = 0,
> > +.transfer = 0
> > };
> > struct rte_flow_error error;
> > struct rte_flow *flow;
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/3] netdev-offload-dpdk: Set transfer attribute to zero for mark/rss offload
On 7/10/2020 3:07 PM, Sriharsha Basavapatna wrote:
The offload layer doesn't initialize the 'transfer' attribute
for mark/rss offload (partial offload). It should be set to 0.
Fixes: 60e778c7533a ("netdev-offload-dpdk: Framework for actions offload.")
It is not a bug. .ingress = 1 is also sufficient.
See
http://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Initializing-Structure-Members
Anyway, this is the commit that introduced it.
e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
Signed-off-by: Sriharsha Basavapatna
---
lib/netdev-offload-dpdk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 26a75f0f2..4c652fd82 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -818,7 +818,8 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
.group = 0,
.priority = 0,
.ingress = 1,
-.egress = 0
+.egress = 0,
+.transfer = 0
};
struct rte_flow_error error;
struct rte_flow *flow;
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/3] netdev-offload-dpdk: Set transfer attribute to zero for mark/rss offload
The offload layer doesn't initialize the 'transfer' attribute
for mark/rss offload (partial offload). It should be set to 0.
Fixes: 60e778c7533a ("netdev-offload-dpdk: Framework for actions offload.")
Signed-off-by: Sriharsha Basavapatna
---
lib/netdev-offload-dpdk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 26a75f0f2..4c652fd82 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -818,7 +818,8 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
.group = 0,
.priority = 0,
.ingress = 1,
-.egress = 0
+.egress = 0,
+.transfer = 0
};
struct rte_flow_error error;
struct rte_flow *flow;
--
2.25.0.rc2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
