On Fri, Jul 10, 2020 at 4:01 PM Sriharsha Basavapatna < sriharsha.basavapa...@broadcom.com> wrote:
> > > On Sun, Jul 5, 2020 at 6:00 PM Eli Britstein <el...@mellanox.com> wrote: > >> >> On 7/5/2020 2:48 PM, Sriharsha Basavapatna wrote: >> > The offload layer clears the L4 protocol mask in the L3 item, when the >> > L4 item is passed for matching, as an optimization. This can be >> confusing >> > while parsing the headers in the PMD. Also, the datapath flow specifies >> > this field to be matched. This optimization is best left to the PMD. >> > This patch restores the code to pass the L4 protocol type in L3 match. >> > >> > Fixes: 900fe00784ca ("netdev-offload-dpdk: Dynamically allocate pattern >> items.") >> >> It's arguable if it's really a fix. > > It is better not to ignore a field that is specified to be matched by the > datapath flow. > > >> I don't see any further information >> the PMD can use, but it's harmless anyway, so OK by me either with this >> commit or without. > > If you insist it's a fix, this is the correct commit that did it in the >> first place: >> >> e8a2b5bf92bb netdev-dpdk: implement flow offload with rte flow >> > > Thanks, I'll update the "fixes" field in v2. > -Harsha > I'll send v2 of this patch in a patchset with a couple of other fixes. -Harsha > >> > Signed-off-by: Sriharsha Basavapatna < >> sriharsha.basavapa...@broadcom.com> >> > --- >> > lib/netdev-offload-dpdk.c | 22 ---------------------- >> > 1 file changed, 22 deletions(-) >> > >> > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >> > index 4c652fd82..165fd1f47 100644 >> > --- a/lib/netdev-offload-dpdk.c >> > +++ b/lib/netdev-offload-dpdk.c >> > @@ -596,7 +596,6 @@ static int >> > parse_flow_match(struct flow_patterns *patterns, >> > const struct match *match) >> > { >> > - uint8_t *next_proto_mask = NULL; >> > uint8_t proto = 0; >> > >> > /* Eth */ >> > @@ -667,7 +666,6 @@ parse_flow_match(struct flow_patterns *patterns, >> > /* Save proto for L4 protocol setup. */ >> > proto = spec->hdr.next_proto_id & >> > mask->hdr.next_proto_id; >> > - next_proto_mask = &mask->hdr.next_proto_id; >> > } >> > >> > if (proto != IPPROTO_ICMP && proto != IPPROTO_UDP && >> > @@ -701,11 +699,6 @@ parse_flow_match(struct flow_patterns *patterns, >> > mask->hdr.tcp_flags = ntohs(match->wc.masks.tcp_flags) & 0xff; >> > >> > add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_TCP, spec, >> mask); >> > - >> > - /* proto == TCP and ITEM_TYPE_TCP, thus no need for proto >> match. */ >> > - if (next_proto_mask) { >> > - *next_proto_mask = 0; >> > - } >> > } else if (proto == IPPROTO_UDP) { >> > struct rte_flow_item_udp *spec, *mask; >> > >> > @@ -719,11 +712,6 @@ parse_flow_match(struct flow_patterns *patterns, >> > mask->hdr.dst_port = match->wc.masks.tp_dst; >> > >> > add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_UDP, spec, >> mask); >> > - >> > - /* proto == UDP and ITEM_TYPE_UDP, thus no need for proto >> match. */ >> > - if (next_proto_mask) { >> > - *next_proto_mask = 0; >> > - } >> > } else if (proto == IPPROTO_SCTP) { >> > struct rte_flow_item_sctp *spec, *mask; >> > >> > @@ -737,11 +725,6 @@ parse_flow_match(struct flow_patterns *patterns, >> > mask->hdr.dst_port = match->wc.masks.tp_dst; >> > >> > add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_SCTP, spec, >> mask); >> > - >> > - /* proto == SCTP and ITEM_TYPE_SCTP, thus no need for proto >> match. */ >> > - if (next_proto_mask) { >> > - *next_proto_mask = 0; >> > - } >> > } else if (proto == IPPROTO_ICMP) { >> > struct rte_flow_item_icmp *spec, *mask; >> > >> > @@ -755,11 +738,6 @@ parse_flow_match(struct flow_patterns *patterns, >> > mask->hdr.icmp_code = (uint8_t) ntohs(match->wc.masks.tp_dst); >> > >> > add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_ICMP, spec, >> mask); >> > - >> > - /* proto == ICMP and ITEM_TYPE_ICMP, thus no need for proto >> match. */ >> > - if (next_proto_mask) { >> > - *next_proto_mask = 0; >> > - } >> > } >> > >> > add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_END, NULL, NULL); >> > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev