diff mbox series

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

Message ID 20200621111924.12397-3-elibr@mellanox.com
State Changes Requested
Headers show
Series netdev datapath offload: Support IPv6 and VXLAN encap | expand

Commit Message

Eli Britstein June 21, 2020, 11:19 a.m. UTC 
Add support for IPv6 pattern matching for offloading flows.

Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roni Bar Yanai <roniba@mellanox.com>
---
 Documentation/howto/dpdk.rst |  2 +-
 NEWS                         |  1 +
 lib/netdev-offload-dpdk.c    | 76 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 1 deletion(-)

Comments

Ilya Maximets June 28, 2020, 10:42 p.m. UTC | #1 
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@mellanox.com>
> Reviewed-by: Roni Bar Yanai <roniba@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? :)

>     - 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?

>  
>  #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.

> +                                   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.

> +        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.

> +        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 ||
>
Eli Britstein June 29, 2020, 7:45 a.m. UTC | #2 
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@mellanox.com>
>> Reviewed-by: Roni Bar Yanai <roniba@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?
>
>>      - 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 ||
>>
Ilya Maximets June 29, 2020, 9:10 a.m. UTC | #3 
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@mellanox.com>
>>> Reviewed-by: Roni Bar Yanai <roniba@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 ||
>>>
Eli Britstein June 29, 2020, 4:22 p.m. UTC | #4 
On 6/29/2020 10: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@mellanox.com>
>>> Reviewed-by: Roni Bar Yanai <roniba@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?
>>
>>>      - 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.

It fails travis. I'll keep it that order.

../../include/sparse/netinet/in.h:24:2: error: "Must include 
<sys/types.h> before <netinet/in.h> for FreeBSD support"

>>
>>>     #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 ||
>>>
diff mbox series

Patch

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.
    - 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>
 
 #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 <<
+                                   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);
+        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 */
+        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 ||