[ovs-dev] [PATCH V3 02/12] netdev-offload-dpdk: Add IPv6 pattern matching

Ilya Maximets i.maximets at ovn.org
Mon Jun 29 09:10:56 UTC 2020


On 6/29/20 9:45 AM, Eli Britstein wrote:
> 
> On 6/29/2020 1:42 AM, Ilya Maximets wrote:
>> On 6/21/20 1:19 PM, Eli Britstein wrote:
>>> Add support for IPv6 pattern matching for offloading flows.
>>>
>>> Signed-off-by: Eli Britstein <elibr at mellanox.com>
>>> Reviewed-by: Roni Bar Yanai <roniba at mellanox.com>
>>> ---
>>>   Documentation/howto/dpdk.rst |  2 +-
>>>   NEWS                         |  1 +
>>>   lib/netdev-offload-dpdk.c    | 76 ++++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 78 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
>>> index be950d7ce..8a0eee80c 100644
>>> --- a/Documentation/howto/dpdk.rst
>>> +++ b/Documentation/howto/dpdk.rst
>>> @@ -385,7 +385,7 @@ The validated NICs are:
>>>   Supported protocols for hardware offload matches are:
>>>     - L2: Ethernet, VLAN
>>> -- L3: IPv4
>>> +- L3: IPv4, IPv6
>>>   - L4: TCP, UDP, SCTP, ICMP
>>>     Supported actions for hardware offload are:
>>> diff --git a/NEWS b/NEWS
>>> index 22cacda20..07e23316c 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -9,6 +9,7 @@ Post-v2.13.0
>>>      - DPDK:
>>>        * Deprecated DPDK pdump packet capture support removed.
>>>        * Deprecated DPDK ring ports (dpdkr) are no longer supported.
>>> +     * Add hardware offload support for matching IPv6 protocol.
>> Not experimental? :)
> OK. BTW, when do features stop being experimental but mainstream?

I don't think there is a standardized process for that.  In general feature
becomes non-experimental when we're confident enough in it and it is
thoroughly tested.

BTW, I'm not sure about marking every single bit of HW offloading as
experimental feature since the whole 'hw-offload' is already marked this way.
Also there is no way to disable parts of matching or some of actions.
So, it might not make much sense having experimental tags everywhere.
However, we will likely need to treat different offload providers
differently while considering removing experimental tag from the
'hw-offload' configuration knob.

>>
>>>      - Linux datapath:
>>>        * Support for kernel versions up to 5.5.x.
>>>      - AF_XDP:
>>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>>> index 2f1b42bf7..6b12e9ae3 100644
>>> --- a/lib/netdev-offload-dpdk.c
>>> +++ b/lib/netdev-offload-dpdk.c
>>> @@ -16,6 +16,8 @@
>>>    */
>>>   #include <config.h>
>>>   +#include <sys/types.h>
>>> +#include <netinet/ip6.h>
>>>   #include <rte_flow.h>
>> Can we keep these in alphabetical order?
> OK.
>>
>>>     #include "cmap.h"
>>> @@ -321,6 +323,42 @@ dump_flow_pattern(struct ds *s, const struct rte_flow_item *item)
>>>           } else {
>>>               ds_put_cstr(s, "  Mask = null\n");
>>>           }
>>> +    } else if (item->type == RTE_FLOW_ITEM_TYPE_IPV6) {
>>> +        const struct rte_flow_item_ipv6 *ipv6_spec = item->spec;
>>> +        const struct rte_flow_item_ipv6 *ipv6_mask = item->mask;
>>> +
>>> +        char src_addr_str[INET6_ADDRSTRLEN];
>>> +        char dst_addr_str[INET6_ADDRSTRLEN];
>>> +
>>> +        ds_put_cstr(s, "rte flow ipv6 pattern:\n");
>>> +        if (ipv6_spec) {
>>> +            ipv6_string_mapped(src_addr_str,
>>> +                               (struct in6_addr *)&ipv6_spec->hdr.src_addr);
>>> +            ipv6_string_mapped(dst_addr_str,
>>> +                               (struct in6_addr *)&ipv6_spec->hdr.dst_addr);
>>> +
>>> +            ds_put_format(s, "  Spec:  vtc_flow=%#"PRIx32",  proto=%"PRIu8","
>>> +                          "  hlim=%"PRIu8",  src=%s,  dst=%s\n",
>>> +                          ntohl(ipv6_spec->hdr.vtc_flow), ipv6_spec->hdr.proto,
>>> +                          ipv6_spec->hdr.hop_limits, src_addr_str,
>>> +                          dst_addr_str);
>>> +        } else {
>>> +            ds_put_cstr(s, "  Spec = null\n");
>>> +        }
>>> +        if (ipv6_mask) {
>>> +            ipv6_string_mapped(src_addr_str,
>>> +                               (struct in6_addr *)&ipv6_mask->hdr.src_addr);
>>> +            ipv6_string_mapped(dst_addr_str,
>>> +                               (struct in6_addr *)&ipv6_mask->hdr.dst_addr);
>>> +
>>> +            ds_put_format(s, "  Mask:  vtc_flow=%#"PRIx32",  proto=%#"PRIx8","
>>> +                          "  hlim=%#"PRIx8",  src=%s,  dst=%s\n",
>>> +                          ntohl(ipv6_mask->hdr.vtc_flow), ipv6_mask->hdr.proto,
>>> +                          ipv6_mask->hdr.hop_limits, src_addr_str,
>>> +                          dst_addr_str);
>>> +        } else {
>>> +            ds_put_cstr(s, "  Mask = null\n");
>>> +        }
>>>       } else {
>>>           ds_put_format(s, "unknown rte flow pattern (%d)\n", item->type);
>>>       }
>>> @@ -658,6 +696,44 @@ parse_flow_match(struct flow_patterns *patterns,
>>>       /* ignore mask match for now */
>>>       consumed_masks->nw_frag = 0;
>>>   +    /* IP v6 */
>>> +    if (match->flow.dl_type == htons(ETH_TYPE_IPV6)) {
>>> +        struct rte_flow_item_ipv6 *spec, *mask;
>>> +
>>> +        spec = xzalloc(sizeof *spec);
>>> +        mask = xzalloc(sizeof *mask);
>>> +
>>> +        spec->hdr.proto = match->flow.nw_proto;
>>> +        spec->hdr.hop_limits = match->flow.nw_ttl;
>>> +        spec->hdr.vtc_flow = htonl((uint32_t)match->flow.nw_tos <<
>> Please, add a space between the caset and a value.
>> It also might look better if the whole right side of the assignment
>> will be moved to the next line and joined.
> OK
>>
>>> +                                   RTE_IPV6_HDR_TC_SHIFT);
>>> +        memcpy(spec->hdr.src_addr, &match->flow.ipv6_src,
>>> +               sizeof spec->hdr.src_addr);
>>> +        memcpy(spec->hdr.dst_addr, &match->flow.ipv6_dst,
>>> +               sizeof spec->hdr.dst_addr);
>>> +
>>> +        mask->hdr.proto = match->wc.masks.nw_proto;
>>> +        mask->hdr.hop_limits = match->wc.masks.nw_ttl;
>>> +        mask->hdr.vtc_flow = htonl((uint32_t)match->wc.masks.nw_tos <<
>>> +                                   RTE_IPV6_HDR_TC_SHIFT);
>> Ditto.
> OK
>>
>>> +        memcpy(mask->hdr.src_addr, &match->wc.masks.ipv6_src,
>>> +               sizeof mask->hdr.src_addr);
>>> +        memcpy(mask->hdr.dst_addr, &match->wc.masks.ipv6_dst,
>>> +               sizeof mask->hdr.dst_addr);
>>> +
>>> +        consumed_masks->nw_proto = 0;
>>> +        consumed_masks->nw_ttl = 0;
>>> +        consumed_masks->nw_tos = 0;
>>> +        memset(&consumed_masks->ipv6_src, 0, sizeof consumed_masks->ipv6_src);
>>> +        memset(&consumed_masks->ipv6_dst, 0, sizeof consumed_masks->ipv6_dst);
>>> +
>>> +        add_flow_pattern(patterns, RTE_FLOW_ITEM_TYPE_IPV6, spec, mask);
>>> +
>>> +        /* Save proto for L4 protocol setup */
>> There should be period at the end of the comment.
> OK
>>
>>> +        proto = spec->hdr.proto & mask->hdr.proto;
>>> +        next_proto_mask = &mask->hdr.proto;
>>> +    }
>>> +
>>>       if (proto != IPPROTO_ICMP && proto != IPPROTO_UDP  &&
>>>           proto != IPPROTO_SCTP && proto != IPPROTO_TCP  &&
>>>           (match->wc.masks.tp_src ||
>>>



More information about the dev mailing list