On Sun, Jul 5, 2020 at 6:00 PM Eli Britstein <[email protected]> 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
>
> > Signed-off-by: Sriharsha Basavapatna <[email protected]
> >
> > ---
> > 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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
