| Message ID | 20191216151047.5967-18-elibr@mellanox.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | netdev datapath actions offload | expand |
On 16.12.2019 16:10, Eli Britstein wrote: > Signed-off-by: Eli Britstein <elibr@mellanox.com> > Reviewed-by: Oz Shlomo <ozsh@mellanox.com> > --- > Documentation/howto/dpdk.rst | 1 + > NEWS | 4 +-- > lib/netdev-offload-dpdk.c | 85 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 88 insertions(+), 2 deletions(-) > In order to reduce code size and improve readability suggesting following incremental (based on top of a full patch-set, compile tested only). Explanations: 1. We're allowed to modify actions here as they are not constant. So, no need to maintain copy of the mask. tc offload does the same. 2. All build asserts moved out of function to a single place. 3. Macros for repeated code pattern. 4. mask could be obtained as just key + 1. --- Subject: [PATCH] netdev-offload-dpdk: Refactor set actions. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- lib/netdev-offload-dpdk.c | 186 +++++++++++--------------------------- 1 file changed, 51 insertions(+), 135 deletions(-) diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index d1c066992..02fe551d0 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -848,11 +848,9 @@ add_output_action(struct netdev *netdev, } static int -add_set_flow_action(struct flow_actions *actions, - const void *value, - const void *mask, - const size_t size, - const int attr) +add_set_flow_action__(struct flow_actions *actions, + const void *value, void *mask, + const size_t size, const int attr) { void *spec; @@ -874,179 +872,97 @@ add_set_flow_action(struct flow_actions *actions, memcpy(spec, value, size); add_flow_action(actions, attr, spec); + /* Clear used mask for later checking. */ + if (mask) { + memset(mask, 0, size); + } return 0; } -/* Mask is at the midpoint of the data. */ -#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1) +BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_mac) == + MEMBER_SIZEOF(struct ovs_key_ethernet, eth_src)); +BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_mac) == + MEMBER_SIZEOF(struct ovs_key_ethernet, eth_dst)); +BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_ipv4) == + MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_src)); +BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_ipv4) == + MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_dst)); +BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_ttl) == + MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_ttl)); +BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_tp) == + MEMBER_SIZEOF(struct ovs_key_tcp, tcp_src)); +BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_tp) == + MEMBER_SIZEOF(struct ovs_key_tcp, tcp_dst)); +BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_tp) == + MEMBER_SIZEOF(struct ovs_key_udp, udp_src)); +BUILD_ASSERT_DECL(sizeof(struct rte_flow_action_set_tp) == + MEMBER_SIZEOF(struct ovs_key_udp, udp_dst)); static int parse_set_actions(struct flow_actions *actions, - const struct nlattr *set_actions, + struct nlattr *set_actions, const size_t set_actions_len, bool masked) { const struct nlattr *sa; unsigned int sleft; +#define add_set_flow_action(field, type) \ + if (add_set_flow_action__(actions, &key->field, \ + mask ? CONST_CAST(void *, &mask->field) : NULL, \ + sizeof key->field, type)) { \ + return -1; \ + } + NL_ATTR_FOR_EACH_UNSAFE (sa, sleft, set_actions, set_actions_len) { if (nl_attr_type(sa) == OVS_KEY_ATTR_ETHERNET) { const struct ovs_key_ethernet *key = nl_attr_get(sa); - const struct ovs_key_ethernet *mask = masked ? - get_mask(sa, struct ovs_key_ethernet) : NULL; - struct ovs_key_ethernet consumed_mask; - - if (masked) { - memcpy(&consumed_mask, mask, sizeof consumed_mask); - } else { - memset(&consumed_mask, 0, sizeof consumed_mask); - } + const struct ovs_key_ethernet *mask = masked ? key + 1 : NULL; - BUILD_ASSERT(sizeof(struct rte_flow_action_set_mac) == - sizeof key->eth_src); - if (add_set_flow_action(actions, &key->eth_src, - mask ? &mask->eth_src : NULL, - sizeof key->eth_src, - RTE_FLOW_ACTION_TYPE_SET_MAC_SRC)) { - return -1; - } - memset(&consumed_mask.eth_src, 0, sizeof consumed_mask.eth_src); - - BUILD_ASSERT(sizeof(struct rte_flow_action_set_mac) == - sizeof key->eth_dst); - if (add_set_flow_action(actions, &key->eth_dst, - mask ? &mask->eth_dst : NULL, - sizeof key->eth_dst, - RTE_FLOW_ACTION_TYPE_SET_MAC_DST)) { - return -1; - } - memset(&consumed_mask.eth_dst, 0, sizeof consumed_mask.eth_dst); + add_set_flow_action(eth_src, RTE_FLOW_ACTION_TYPE_SET_MAC_SRC); + add_set_flow_action(eth_dst, RTE_FLOW_ACTION_TYPE_SET_MAC_DST); - if (!is_all_zeros(&consumed_mask, sizeof consumed_mask)) { + if (mask && !is_all_zeros(mask, sizeof *mask)) { VLOG_DBG_RL(&error_rl, "Unsupported ETHERNET set action"); return -1; } } else if (nl_attr_type(sa) == OVS_KEY_ATTR_IPV4) { const struct ovs_key_ipv4 *key = nl_attr_get(sa); - const struct ovs_key_ipv4 *mask = masked ? - get_mask(sa, struct ovs_key_ipv4) : NULL; - struct ovs_key_ipv4 consumed_mask; - - if (masked) { - memcpy(&consumed_mask, mask, sizeof consumed_mask); - } else { - memset(&consumed_mask, 0, sizeof consumed_mask); - } + const struct ovs_key_ipv4 *mask = masked ? key + 1 : NULL; - BUILD_ASSERT(sizeof(struct rte_flow_action_set_ipv4) == - sizeof key->ipv4_src); - if (add_set_flow_action(actions, &key->ipv4_src, - mask ? &mask->ipv4_src : NULL, - sizeof key->ipv4_src, - RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC)) { - return -1; - } - memset(&consumed_mask.ipv4_src, 0, sizeof consumed_mask.ipv4_src); - - BUILD_ASSERT(sizeof(struct rte_flow_action_set_ipv4) == - sizeof key->ipv4_dst); - if (add_set_flow_action(actions, &key->ipv4_dst, - mask ? &mask->ipv4_dst : NULL, - sizeof key->ipv4_dst, - RTE_FLOW_ACTION_TYPE_SET_IPV4_DST)) { - return -1; - } - memset(&consumed_mask.ipv4_dst, 0, sizeof consumed_mask.ipv4_dst); - - BUILD_ASSERT(sizeof(struct rte_flow_action_set_ttl) == - sizeof key->ipv4_ttl); - if (add_set_flow_action(actions, &key->ipv4_ttl, - mask ? &mask->ipv4_ttl : NULL, - sizeof key->ipv4_ttl, - RTE_FLOW_ACTION_TYPE_SET_TTL)) { - return -1; - } - memset(&consumed_mask.ipv4_ttl, 0, sizeof consumed_mask.ipv4_ttl); + 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); - if (!is_all_zeros(&consumed_mask, sizeof consumed_mask)) { + if (mask && !is_all_zeros(mask, sizeof *mask)) { VLOG_DBG_RL(&error_rl, "Unsupported IPv4 set action"); return -1; } } else if (nl_attr_type(sa) == OVS_KEY_ATTR_TCP) { const struct ovs_key_tcp *key = nl_attr_get(sa); - const struct ovs_key_tcp *mask = masked ? - get_mask(sa, struct ovs_key_tcp) : NULL; - struct ovs_key_tcp consumed_mask; - - if (masked) { - memcpy(&consumed_mask, mask, sizeof consumed_mask); - } else { - memset(&consumed_mask, 0, sizeof consumed_mask); - } + const struct ovs_key_tcp *mask = masked ? key + 1 : NULL; - BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) == - sizeof key->tcp_src); - if (add_set_flow_action(actions, &key->tcp_src, - mask ? &mask->tcp_src : NULL, - sizeof key->tcp_src, - RTE_FLOW_ACTION_TYPE_SET_TP_SRC)) { - return -1; - } - memset(&consumed_mask.tcp_src, 0, sizeof consumed_mask.tcp_src); - - BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) == - sizeof key->tcp_dst); - if (add_set_flow_action(actions, &key->tcp_dst, - mask ? &mask->tcp_dst : NULL, - sizeof key->tcp_dst, - RTE_FLOW_ACTION_TYPE_SET_TP_DST)) { - return -1; - } - memset(&consumed_mask.tcp_dst, 0, sizeof consumed_mask.tcp_dst); + add_set_flow_action(tcp_src, RTE_FLOW_ACTION_TYPE_SET_TP_SRC); + add_set_flow_action(tcp_dst, RTE_FLOW_ACTION_TYPE_SET_TP_DST); - if (!is_all_zeros(&consumed_mask, sizeof consumed_mask)) { + if (mask && !is_all_zeros(mask, sizeof *mask)) { VLOG_DBG_RL(&error_rl, "Unsupported TCP set action"); return -1; } } else if (nl_attr_type(sa) == OVS_KEY_ATTR_UDP) { const struct ovs_key_udp *key = nl_attr_get(sa); - const struct ovs_key_udp *mask = masked ? - get_mask(sa, struct ovs_key_udp) : NULL; - struct ovs_key_udp consumed_mask; - - if (masked) { - memcpy(&consumed_mask, mask, sizeof consumed_mask); - } else { - memset(&consumed_mask, 0, sizeof consumed_mask); - } + const struct ovs_key_udp *mask = masked ? key + 1 : NULL; - BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) == - sizeof key->udp_src); - if (add_set_flow_action(actions, &key->udp_src, - mask ? &mask->udp_src : NULL, - sizeof key->udp_src, - RTE_FLOW_ACTION_TYPE_SET_TP_SRC)) { - return -1; - } - memset(&consumed_mask.udp_src, 0, sizeof consumed_mask.udp_src); - - BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) == - sizeof key->udp_dst); - if (add_set_flow_action(actions, &key->udp_dst, - mask ? &mask->udp_dst : NULL, - sizeof key->udp_dst, - RTE_FLOW_ACTION_TYPE_SET_TP_DST)) { - return -1; - } - memset(&consumed_mask.udp_dst, 0, sizeof consumed_mask.udp_dst); + add_set_flow_action(udp_src, RTE_FLOW_ACTION_TYPE_SET_TP_SRC); + add_set_flow_action(udp_dst, RTE_FLOW_ACTION_TYPE_SET_TP_DST); - if (!is_all_zeros(&consumed_mask, sizeof consumed_mask)) { + if (mask && !is_all_zeros(mask, sizeof *mask)) { VLOG_DBG_RL(&error_rl, "Unsupported UDP set action"); return -1; } } else { VLOG_DBG_RL(&error_rl, - "Unsupported set action type=%d", nl_attr_type(sa)); + "Unsupported set action type %d", nl_attr_type(sa)); return -1; } }
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst index c440bf28e..be950d7ce 100644 --- a/Documentation/howto/dpdk.rst +++ b/Documentation/howto/dpdk.rst @@ -394,6 +394,7 @@ Supported actions for hardware offload are: - Drop. - Modification of Ethernet (mod_dl_src/mod_dl_dst). - Modification of IPv4 (mod_nw_src/mod_nw_dst/mod_nw_ttl). +- Modification of TCP/UDP (mod_tp_src/mod_tp_dst). Further Reading --------------- diff --git a/NEWS b/NEWS index 297ca6fff..b611ed76d 100644 --- a/NEWS +++ b/NEWS @@ -26,8 +26,8 @@ Post-v2.12.0 * DPDK ring ports (dpdkr) are deprecated and will be removed in next releases. * Add support for DPDK 19.11. - * Add hardware offload support for output, drop and set actions of - MAC and IPv4 (experimental). + * Add hardware offload support for output, drop, set of MAC, IPv4 and + TCP/UDP ports actions (experimental). v2.12.0 - 03 Sep 2019 --------------------- diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index 0f0685f4d..0ec840ce1 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -405,6 +405,19 @@ dump_flow_action(struct ds *s, const struct rte_flow_action *actions) } else { ds_put_cstr(s, " Set-ttl = null\n"); } + } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_SRC || + actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_DST) { + const struct rte_flow_action_set_tp *set_tp = actions->conf; + char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_TP_DST + ? "dst" : "src"; + + ds_put_format(s, "rte flow set-tcp/udp-port-%s action:\n", dirstr); + if (set_tp) { + ds_put_format(s, " Set-%s-tcp/udp-port: %"PRIu16"\n", dirstr, + ntohs(set_tp->port)); + } else { + ds_put_format(s, " Set-%s-tcp/udp-port = null\n", dirstr); + } } else { ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); } @@ -957,6 +970,78 @@ parse_set_actions(struct flow_actions *actions, VLOG_DBG_RL(&error_rl, "Unsupported IPv4 set action"); return -1; } + } else if (nl_attr_type(sa) == OVS_KEY_ATTR_TCP) { + const struct ovs_key_tcp *key = nl_attr_get(sa); + const struct ovs_key_tcp *mask = masked ? + get_mask(sa, struct ovs_key_tcp) : NULL; + struct ovs_key_tcp consumed_mask; + + if (masked) { + memcpy(&consumed_mask, mask, sizeof consumed_mask); + } else { + memset(&consumed_mask, 0, sizeof consumed_mask); + } + + BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) == + sizeof key->tcp_src); + if (add_set_flow_action(actions, &key->tcp_src, + mask ? &mask->tcp_src : NULL, + sizeof key->tcp_src, + RTE_FLOW_ACTION_TYPE_SET_TP_SRC)) { + return -1; + } + memset(&consumed_mask.tcp_src, 0, sizeof consumed_mask.tcp_src); + + BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) == + sizeof key->tcp_dst); + if (add_set_flow_action(actions, &key->tcp_dst, + mask ? &mask->tcp_dst : NULL, + sizeof key->tcp_dst, + RTE_FLOW_ACTION_TYPE_SET_TP_DST)) { + return -1; + } + memset(&consumed_mask.tcp_dst, 0, sizeof consumed_mask.tcp_dst); + + if (!is_all_zeros(&consumed_mask, sizeof consumed_mask)) { + VLOG_DBG_RL(&error_rl, "Unsupported TCP set action"); + return -1; + } + } else if (nl_attr_type(sa) == OVS_KEY_ATTR_UDP) { + const struct ovs_key_udp *key = nl_attr_get(sa); + const struct ovs_key_udp *mask = masked ? + get_mask(sa, struct ovs_key_udp) : NULL; + struct ovs_key_udp consumed_mask; + + if (masked) { + memcpy(&consumed_mask, mask, sizeof consumed_mask); + } else { + memset(&consumed_mask, 0, sizeof consumed_mask); + } + + BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) == + sizeof key->udp_src); + if (add_set_flow_action(actions, &key->udp_src, + mask ? &mask->udp_src : NULL, + sizeof key->udp_src, + RTE_FLOW_ACTION_TYPE_SET_TP_SRC)) { + return -1; + } + memset(&consumed_mask.udp_src, 0, sizeof consumed_mask.udp_src); + + BUILD_ASSERT(sizeof(struct rte_flow_action_set_tp) == + sizeof key->udp_dst); + if (add_set_flow_action(actions, &key->udp_dst, + mask ? &mask->udp_dst : NULL, + sizeof key->udp_dst, + RTE_FLOW_ACTION_TYPE_SET_TP_DST)) { + return -1; + } + memset(&consumed_mask.udp_dst, 0, sizeof consumed_mask.udp_dst); + + if (!is_all_zeros(&consumed_mask, sizeof consumed_mask)) { + VLOG_DBG_RL(&error_rl, "Unsupported UDP set action"); + return -1; + } } else { VLOG_DBG_RL(&error_rl, "Unsupported set action type=%d", nl_attr_type(sa));