diff mbox series

[ovs-dev,v5] netdev-offload-dpdk: Support offload of set dscp action.

Message ID 20240709131817.35744-1-sunyang.wu@jaguarmicro.com
State Changes Requested
Delegated to: Simon Horman
Headers show
Series [ovs-dev,v5] netdev-offload-dpdk: Support offload of set dscp action. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Sunyang Wu July 9, 2024, 1:18 p.m. UTC 
Add the "set dscp action" parsing function,
so that the "set dscp action" can be offloaded.

Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com>
---
 Documentation/howto/dpdk.rst |  5 +++--
 lib/netdev-offload-dpdk.c    | 27 ++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 3 deletions(-)

Comments

Simon Horman July 10, 2024, 8:35 a.m. UTC | #1 
On Tue, Jul 09, 2024 at 09:18:17PM +0800, Sunyang Wu wrote:
> Add the "set dscp action" parsing function,
> so that the "set dscp action" can be offloaded.
> 
> Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com>
> ---
>  Documentation/howto/dpdk.rst |  5 +++--
>  lib/netdev-offload-dpdk.c    | 27 ++++++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index 04609b20b..f0e46c95b 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -395,10 +395,11 @@ Supported actions for hardware offload are:
>  - Output.
>  - Drop.
>  - Modification of Ethernet (mod_dl_src/mod_dl_dst).
> -- Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl).
> +- Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl/mod_nw_tos).
>  - Modification of TCP/UDP (mod_tp_src/mod_tp_dst).
>  - VLAN Push/Pop (push_vlan/pop_vlan).
> -- Modification of IPv6 (set_field:<ADDR>->ipv6_src/ipv6_dst/mod_nw_ttl).
> +- Modification of IPv6 (set_field:<ADDR>->ipv6_src/ipv6_dst/
> +mod_nw_ttl/mod_nw_tos).
>  - Clone/output (tnl_push and output) for encapsulating over a tunnel.
>  - Tunnel pop, for packets received on physical ports.
>  

HI Sunyang Wu,

Unfortunately this causes building documentation to fail.

  Warning, treated as error:
  .../dpdk.rst:402:Bullet list ends without a blank line; unexpected unindent.

> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 623005b1c..5a74c6d19 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -791,6 +791,17 @@ dump_flow_action(struct ds *s, struct ds *s_extra,
>              ds_put_format(s, "port %"PRIu16" ", ntohs(set_tp->port));
>          }
>          ds_put_cstr(s, "/ ");
> +    } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP ||
> +               actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP) {
> +        const struct rte_flow_action_set_dscp *set_dscp = actions->conf;
> +        char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> +                       ? "set_ipv4_dscp " : "set_ipv6_dscp ";
> +
> +        ds_put_cstr(s, dirstr);
> +        if (set_dscp) {
> +            ds_put_format(s, "dscp_value %d ", set_dscp->dscp);
> +        }
> +        ds_put_cstr(s, "/ ");
>      } else if (actions->type == RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN) {
>          const struct rte_flow_action_of_push_vlan *of_push_vlan =
>              actions->conf;
> @@ -1836,11 +1847,22 @@ add_set_flow_action__(struct flow_actions *actions,
>              return 0;
>          }
>          if (!is_all_ones(mask, size)) {
> -            VLOG_DBG_RL(&rl, "Partial mask is not supported");
> +            if (attr != RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP ||
> +                attr != RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP) {

Maybe I am reading it wrong.
But the sense of the condition above seems inverted:
we want this logic iif it is DSCP.

> +                if (*(uint8_t *) mask & IP_ECN_MASK) {
> +                    VLOG_DBG_RL(&rl, "ECN hw offload is not supported!");
> +                } else {
> +                    goto add_action;

I think that with a bit of care we can avoid a goto.
Perhaps a helper for the dscp/ecn case?

And as per Ilya's review of v3, don't we also need to check that all the
DSCP bits are set in the mask?

And likewise, as per Ilya's review of v3, "this function will clear the
whole mask below, but we should only clear DSCP bits in this case."

> +                }
> +            } else {
> +                VLOG_DBG_RL(&rl, "Partial mask is not supported");
> +            }
> +
>              return -1;
>          }
>      }
>  
> +add_action:
>      spec = xzalloc(size);
>      memcpy(spec, value, size);
>      add_flow_action(actions, attr, spec);
> @@ -1912,6 +1934,7 @@ parse_set_actions(struct flow_actions *actions,
>              add_set_flow_action(ipv4_src, RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC);
>              add_set_flow_action(ipv4_dst, RTE_FLOW_ACTION_TYPE_SET_IPV4_DST);
>              add_set_flow_action(ipv4_ttl, RTE_FLOW_ACTION_TYPE_SET_TTL);
> +            add_set_flow_action(ipv4_tos, RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP);
>  
>              if (mask && !is_all_zeros(mask, sizeof *mask)) {
>                  VLOG_DBG_RL(&rl, "Unsupported IPv4 set action");
> @@ -1924,6 +1947,8 @@ parse_set_actions(struct flow_actions *actions,
>              add_set_flow_action(ipv6_src, RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC);
>              add_set_flow_action(ipv6_dst, RTE_FLOW_ACTION_TYPE_SET_IPV6_DST);
>              add_set_flow_action(ipv6_hlimit, RTE_FLOW_ACTION_TYPE_SET_TTL);
> +            add_set_flow_action(ipv6_tclass,
> +                                RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP);
>  
>              if (mask && !is_all_zeros(mask, sizeof *mask)) {
>                  VLOG_DBG_RL(&rl, "Unsupported IPv6 set action");
> -- 
> 2.19.0.rc0.windows.1
>
diff mbox series

Patch

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 04609b20b..f0e46c95b 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -395,10 +395,11 @@  Supported actions for hardware offload are:
 - Output.
 - Drop.
 - Modification of Ethernet (mod_dl_src/mod_dl_dst).
-- Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl).
+- Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl/mod_nw_tos).
 - Modification of TCP/UDP (mod_tp_src/mod_tp_dst).
 - VLAN Push/Pop (push_vlan/pop_vlan).
-- Modification of IPv6 (set_field:<ADDR>->ipv6_src/ipv6_dst/mod_nw_ttl).
+- Modification of IPv6 (set_field:<ADDR>->ipv6_src/ipv6_dst/
+mod_nw_ttl/mod_nw_tos).
 - Clone/output (tnl_push and output) for encapsulating over a tunnel.
 - Tunnel pop, for packets received on physical ports.
 
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 623005b1c..5a74c6d19 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -791,6 +791,17 @@  dump_flow_action(struct ds *s, struct ds *s_extra,
             ds_put_format(s, "port %"PRIu16" ", ntohs(set_tp->port));
         }
         ds_put_cstr(s, "/ ");
+    } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP ||
+               actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP) {
+        const struct rte_flow_action_set_dscp *set_dscp = actions->conf;
+        char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
+                       ? "set_ipv4_dscp " : "set_ipv6_dscp ";
+
+        ds_put_cstr(s, dirstr);
+        if (set_dscp) {
+            ds_put_format(s, "dscp_value %d ", set_dscp->dscp);
+        }
+        ds_put_cstr(s, "/ ");
     } else if (actions->type == RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN) {
         const struct rte_flow_action_of_push_vlan *of_push_vlan =
             actions->conf;
@@ -1836,11 +1847,22 @@  add_set_flow_action__(struct flow_actions *actions,
             return 0;
         }
         if (!is_all_ones(mask, size)) {
-            VLOG_DBG_RL(&rl, "Partial mask is not supported");
+            if (attr != RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP ||
+                attr != RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP) {
+                if (*(uint8_t *) mask & IP_ECN_MASK) {
+                    VLOG_DBG_RL(&rl, "ECN hw offload is not supported!");
+                } else {
+                    goto add_action;
+                }
+            } else {
+                VLOG_DBG_RL(&rl, "Partial mask is not supported");
+            }
+
             return -1;
         }
     }
 
+add_action:
     spec = xzalloc(size);
     memcpy(spec, value, size);
     add_flow_action(actions, attr, spec);
@@ -1912,6 +1934,7 @@  parse_set_actions(struct flow_actions *actions,
             add_set_flow_action(ipv4_src, RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC);
             add_set_flow_action(ipv4_dst, RTE_FLOW_ACTION_TYPE_SET_IPV4_DST);
             add_set_flow_action(ipv4_ttl, RTE_FLOW_ACTION_TYPE_SET_TTL);
+            add_set_flow_action(ipv4_tos, RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP);
 
             if (mask && !is_all_zeros(mask, sizeof *mask)) {
                 VLOG_DBG_RL(&rl, "Unsupported IPv4 set action");
@@ -1924,6 +1947,8 @@  parse_set_actions(struct flow_actions *actions,
             add_set_flow_action(ipv6_src, RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC);
             add_set_flow_action(ipv6_dst, RTE_FLOW_ACTION_TYPE_SET_IPV6_DST);
             add_set_flow_action(ipv6_hlimit, RTE_FLOW_ACTION_TYPE_SET_TTL);
+            add_set_flow_action(ipv6_tclass,
+                                RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP);
 
             if (mask && !is_all_zeros(mask, sizeof *mask)) {
                 VLOG_DBG_RL(&rl, "Unsupported IPv6 set action");