| Message ID | 20191208132304.15521-17-elibr@mellanox.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | netdev datapath actions offload | expand |
On 12/8/2019 3:23 PM, Eli Britstein wrote: > Signed-off-by: Eli Britstein <elibr@mellanox.com> > Reviewed-by: Oz Shlomo <ozsh@mellanox.com> > --- > Documentation/howto/dpdk.rst | 1 + > NEWS | 3 +- > lib/netdev-dpdk.c | 15 ++++++ > lib/netdev-offload-dpdk.c | 107 +++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 125 insertions(+), 1 deletion(-) > > diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst > index 6cedd7f63..d228493e9 100644 > --- a/Documentation/howto/dpdk.rst > +++ b/Documentation/howto/dpdk.rst > @@ -392,6 +392,7 @@ Supported actions for hardware offload are: > > - Output (a single output, as the last action). > - Drop. > +- Modification of Ethernet (mod_dl_src/mod_dl_dst). > > Further Reading > --------------- > diff --git a/NEWS b/NEWS > index d019e066f..3ade86d49 100644 > --- a/NEWS > +++ b/NEWS > @@ -26,7 +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 and drop actions (experimental). > + * Add hardware offload support for output, drop and set MAC actions > + (experimental). > > v2.12.0 - 03 Sep 2019 > --------------------- > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 872a45e75..e67a3dd76 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -4723,6 +4723,21 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) > } > } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { > ds_put_cstr(s, "rte flow drop action\n"); > + } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC || > + actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST) { > + const struct rte_flow_action_set_mac *set_mac = actions->conf; > + > + char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST > + ? "dst" : "src"; > + > + ds_put_format(s, "rte flow set-mac-%s action:\n", dirstr); > + if (set_mac) { > + ds_put_format(s, > + " Set-mac-%s: "ETH_ADDR_FMT"\n", > + dirstr, ETH_ADDR_BYTES_ARGS(set_mac->mac_addr)); > + } else { > + ds_put_format(s, " Set-mac-%s = null\n", dirstr); > + } > } else { > ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); > } > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index bffb69cad..07f5d4687 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -511,6 +511,103 @@ add_output_action(struct netdev *netdev, > return ret; > } > > +struct set_action_info { > + const uint8_t *value, *mask; > + const uint8_t size; > + uint8_t *spec; > + const int attr; > +}; > + > +static int > +add_set_flow_action(struct flow_actions *actions, > + struct set_action_info *sa_info_arr, > + size_t sa_info_arr_size) > +{ > + int field, i; > + > + for (field = 0; field < sa_info_arr_size; field++) { > + if (sa_info_arr[field].mask) { > + /* DPDK does not support partially masked set actions. In such > + * case, fail the offload. > + */ > + if (sa_info_arr[field].mask[0] != 0x00 && > + sa_info_arr[field].mask[0] != 0xFF) { > + VLOG_DBG_RL(&error_rl, > + "Partial mask is not supported"); > + return -1; > + } > + > + for (i = 1; i < sa_info_arr[field].size; i++) { > + if (sa_info_arr[field].mask[i] != > + sa_info_arr[field].mask[i - 1]) { > + VLOG_DBG_RL(&error_rl, > + "Partial mask is not supported"); > + return -1; > + } > + } > + > + if (sa_info_arr[field].mask[0] == 0x00) { > + /* mask bytes are all 0 - no rewrite action required */ > + continue; > + } > + } > + > + memcpy(sa_info_arr[field].spec, sa_info_arr[field].value, > + sa_info_arr[field].size); > + add_flow_action(actions, sa_info_arr[field].attr, > + sa_info_arr[field].spec); > + } > + > + return 0; > +} > + > +/* Mask is at the midpoint of the data. */ > +#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1) > + > +#define SA_INFO(_field, _spec, _attr) { \ > + .value = (uint8_t *)&key->_field, \ > + .mask = (masked) ? (uint8_t *)&mask->_field : NULL, \ > + .size = sizeof key->_field, \ > + .spec = (uint8_t *)&_spec, \ > + .attr = _attr } > + > +static int > +parse_set_actions(struct flow_actions *actions, > + const struct nlattr *set_actions, > + const size_t set_actions_len, > + bool masked) > +{ > + const struct nlattr *sa; > + unsigned int sleft; > + > + 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 rte_flow_action_set_mac *src = xzalloc(sizeof *src); > + struct rte_flow_action_set_mac *dst = xzalloc(sizeof *dst); In case they are not used, there is a memory leak. will be fixed in v4. > + struct set_action_info sa_info_arr[] = { > + SA_INFO(eth_src, src->mac_addr[0], > + RTE_FLOW_ACTION_TYPE_SET_MAC_SRC), > + SA_INFO(eth_dst, dst->mac_addr[0], > + RTE_FLOW_ACTION_TYPE_SET_MAC_DST), > + }; > + > + if (add_set_flow_action(actions, sa_info_arr, > + ARRAY_SIZE(sa_info_arr))) { > + return -1; > + } > + } else { > + VLOG_DBG_RL(&error_rl, > + "Unsupported set action type=%d", nl_attr_type(sa)); > + return -1; > + } > + } > + > + return 0; > +} > + > static int > parse_flow_actions(struct netdev *netdev, > struct flow_actions *actions, > @@ -528,6 +625,16 @@ parse_flow_actions(struct netdev *netdev, > add_output_action(netdev, actions, nla, info )) { > return -1; > } > + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SET || > + nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED) { > + const struct nlattr *set_actions = nl_attr_get(nla); > + const size_t set_actions_len = nl_attr_get_size(nla); > + bool masked = nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED; > + > + if (parse_set_actions(actions, set_actions, set_actions_len, > + masked)) { > + return -1; > + } > } else { > VLOG_DBG_RL(&error_rl, > "Unsupported action type %d", nl_attr_type(nla));
On 12/8/19 4:23 PM, Eli Britstein wrote: > Signed-off-by: Eli Britstein <elibr@mellanox.com> > Reviewed-by: Oz Shlomo <ozsh@mellanox.com> > --- > Documentation/howto/dpdk.rst | 1 + > NEWS | 3 +- > lib/netdev-dpdk.c | 15 ++++++ > lib/netdev-offload-dpdk.c | 107 +++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 125 insertions(+), 1 deletion(-) > > diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst > index 6cedd7f63..d228493e9 100644 > --- a/Documentation/howto/dpdk.rst > +++ b/Documentation/howto/dpdk.rst > @@ -392,6 +392,7 @@ Supported actions for hardware offload are: > > - Output (a single output, as the last action). > - Drop. > +- Modification of Ethernet (mod_dl_src/mod_dl_dst). > > Further Reading > --------------- > diff --git a/NEWS b/NEWS > index d019e066f..3ade86d49 100644 > --- a/NEWS > +++ b/NEWS > @@ -26,7 +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 and drop actions (experimental). > + * Add hardware offload support for output, drop and set MAC actions > + (experimental). > > v2.12.0 - 03 Sep 2019 > --------------------- > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 872a45e75..e67a3dd76 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -4723,6 +4723,21 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) > } > } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { > ds_put_cstr(s, "rte flow drop action\n"); > + } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC || > + actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST) { > + const struct rte_flow_action_set_mac *set_mac = actions->conf; > + > + char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST > + ? "dst" : "src"; > + > + ds_put_format(s, "rte flow set-mac-%s action:\n", dirstr); > + if (set_mac) { > + ds_put_format(s, > + " Set-mac-%s: "ETH_ADDR_FMT"\n", > + dirstr, ETH_ADDR_BYTES_ARGS(set_mac->mac_addr)); > + } else { > + ds_put_format(s, " Set-mac-%s = null\n", dirstr); > + } > } else { > ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); > } > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index bffb69cad..07f5d4687 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -511,6 +511,103 @@ add_output_action(struct netdev *netdev, > return ret; > } > > +struct set_action_info { > + const uint8_t *value, *mask; > + const uint8_t size; > + uint8_t *spec; > + const int attr; > +}; > + > +static int > +add_set_flow_action(struct flow_actions *actions, > + struct set_action_info *sa_info_arr, > + size_t sa_info_arr_size) > +{ > + int field, i; > + > + for (field = 0; field < sa_info_arr_size; field++) { > + if (sa_info_arr[field].mask) { > + /* DPDK does not support partially masked set actions. In such > + * case, fail the offload. > + */ > + if (sa_info_arr[field].mask[0] != 0x00 && > + sa_info_arr[field].mask[0] != 0xFF) { > + VLOG_DBG_RL(&error_rl, > + "Partial mask is not supported"); > + return -1; > + } > + > + for (i = 1; i < sa_info_arr[field].size; i++) { > + if (sa_info_arr[field].mask[i] != > + sa_info_arr[field].mask[i - 1]) { > + VLOG_DBG_RL(&error_rl, > + "Partial mask is not supported"); > + return -1; > + } > + } > + > + if (sa_info_arr[field].mask[0] == 0x00) { > + /* mask bytes are all 0 - no rewrite action required */ > + continue; > + } > + } > + > + memcpy(sa_info_arr[field].spec, sa_info_arr[field].value, > + sa_info_arr[field].size); > + add_flow_action(actions, sa_info_arr[field].attr, > + sa_info_arr[field].spec); > + } > + > + return 0; > +} > + > +/* Mask is at the midpoint of the data. */ > +#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1) > + > +#define SA_INFO(_field, _spec, _attr) { \ > + .value = (uint8_t *)&key->_field, \ > + .mask = (masked) ? (uint8_t *)&mask->_field : NULL, \ > + .size = sizeof key->_field, \ > + .spec = (uint8_t *)&_spec, \ > + .attr = _attr } > + > +static int > +parse_set_actions(struct flow_actions *actions, > + const struct nlattr *set_actions, > + const size_t set_actions_len, > + bool masked) > +{ > + const struct nlattr *sa; > + unsigned int sleft; > + > + 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 rte_flow_action_set_mac *src = xzalloc(sizeof *src); > + struct rte_flow_action_set_mac *dst = xzalloc(sizeof *dst); I've failed to find where memory allocated above is freed. > + struct set_action_info sa_info_arr[] = { > + SA_INFO(eth_src, src->mac_addr[0], > + RTE_FLOW_ACTION_TYPE_SET_MAC_SRC), > + SA_INFO(eth_dst, dst->mac_addr[0], > + RTE_FLOW_ACTION_TYPE_SET_MAC_DST), > + }; > + > + if (add_set_flow_action(actions, sa_info_arr, > + ARRAY_SIZE(sa_info_arr))) { > + return -1; > + } > + } else { > + VLOG_DBG_RL(&error_rl, > + "Unsupported set action type=%d", nl_attr_type(sa)); > + return -1; > + } > + } > + > + return 0; > +} > + > static int > parse_flow_actions(struct netdev *netdev, > struct flow_actions *actions, > @@ -528,6 +625,16 @@ parse_flow_actions(struct netdev *netdev, > add_output_action(netdev, actions, nla, info )) { > return -1; > } > + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SET || > + nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED) { > + const struct nlattr *set_actions = nl_attr_get(nla); > + const size_t set_actions_len = nl_attr_get_size(nla); > + bool masked = nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED; > + > + if (parse_set_actions(actions, set_actions, set_actions_len, > + masked)) { > + return -1; > + } > } else { > VLOG_DBG_RL(&error_rl, > "Unsupported action type %d", nl_attr_type(nla)); >
On 12/10/2019 4:12 PM, Andrew Rybchenko wrote: > On 12/8/19 4:23 PM, Eli Britstein wrote: >> Signed-off-by: Eli Britstein <elibr@mellanox.com> >> Reviewed-by: Oz Shlomo <ozsh@mellanox.com> >> --- >> Documentation/howto/dpdk.rst | 1 + >> NEWS | 3 +- >> lib/netdev-dpdk.c | 15 ++++++ >> lib/netdev-offload-dpdk.c | 107 +++++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 125 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst >> index 6cedd7f63..d228493e9 100644 >> --- a/Documentation/howto/dpdk.rst >> +++ b/Documentation/howto/dpdk.rst >> @@ -392,6 +392,7 @@ Supported actions for hardware offload are: >> >> - Output (a single output, as the last action). >> - Drop. >> +- Modification of Ethernet (mod_dl_src/mod_dl_dst). >> >> Further Reading >> --------------- >> diff --git a/NEWS b/NEWS >> index d019e066f..3ade86d49 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -26,7 +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 and drop actions (experimental). >> + * Add hardware offload support for output, drop and set MAC actions >> + (experimental). >> >> v2.12.0 - 03 Sep 2019 >> --------------------- >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index 872a45e75..e67a3dd76 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -4723,6 +4723,21 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) >> } >> } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { >> ds_put_cstr(s, "rte flow drop action\n"); >> + } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC || >> + actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST) { >> + const struct rte_flow_action_set_mac *set_mac = actions->conf; >> + >> + char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST >> + ? "dst" : "src"; >> + >> + ds_put_format(s, "rte flow set-mac-%s action:\n", dirstr); >> + if (set_mac) { >> + ds_put_format(s, >> + " Set-mac-%s: "ETH_ADDR_FMT"\n", >> + dirstr, ETH_ADDR_BYTES_ARGS(set_mac->mac_addr)); >> + } else { >> + ds_put_format(s, " Set-mac-%s = null\n", dirstr); >> + } >> } else { >> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); >> } >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >> index bffb69cad..07f5d4687 100644 >> --- a/lib/netdev-offload-dpdk.c >> +++ b/lib/netdev-offload-dpdk.c >> @@ -511,6 +511,103 @@ add_output_action(struct netdev *netdev, >> return ret; >> } >> >> +struct set_action_info { >> + const uint8_t *value, *mask; >> + const uint8_t size; >> + uint8_t *spec; >> + const int attr; >> +}; >> + >> +static int >> +add_set_flow_action(struct flow_actions *actions, >> + struct set_action_info *sa_info_arr, >> + size_t sa_info_arr_size) >> +{ >> + int field, i; >> + >> + for (field = 0; field < sa_info_arr_size; field++) { >> + if (sa_info_arr[field].mask) { >> + /* DPDK does not support partially masked set actions. In such >> + * case, fail the offload. >> + */ >> + if (sa_info_arr[field].mask[0] != 0x00 && >> + sa_info_arr[field].mask[0] != 0xFF) { >> + VLOG_DBG_RL(&error_rl, >> + "Partial mask is not supported"); >> + return -1; >> + } >> + >> + for (i = 1; i < sa_info_arr[field].size; i++) { >> + if (sa_info_arr[field].mask[i] != >> + sa_info_arr[field].mask[i - 1]) { >> + VLOG_DBG_RL(&error_rl, >> + "Partial mask is not supported"); >> + return -1; >> + } >> + } >> + >> + if (sa_info_arr[field].mask[0] == 0x00) { >> + /* mask bytes are all 0 - no rewrite action required */ >> + continue; >> + } >> + } >> + >> + memcpy(sa_info_arr[field].spec, sa_info_arr[field].value, >> + sa_info_arr[field].size); >> + add_flow_action(actions, sa_info_arr[field].attr, >> + sa_info_arr[field].spec); >> + } >> + >> + return 0; >> +} >> + >> +/* Mask is at the midpoint of the data. */ >> +#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1) >> + >> +#define SA_INFO(_field, _spec, _attr) { \ >> + .value = (uint8_t *)&key->_field, \ >> + .mask = (masked) ? (uint8_t *)&mask->_field : NULL, \ >> + .size = sizeof key->_field, \ >> + .spec = (uint8_t *)&_spec, \ >> + .attr = _attr } >> + >> +static int >> +parse_set_actions(struct flow_actions *actions, >> + const struct nlattr *set_actions, >> + const size_t set_actions_len, >> + bool masked) >> +{ >> + const struct nlattr *sa; >> + unsigned int sleft; >> + >> + 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 rte_flow_action_set_mac *src = xzalloc(sizeof *src); >> + struct rte_flow_action_set_mac *dst = xzalloc(sizeof *dst); > I've failed to find where memory allocated above is freed. Right. I saw it too after submission. See https://patchwork.ozlabs.org/patch/1205673/ > >> + struct set_action_info sa_info_arr[] = { >> + SA_INFO(eth_src, src->mac_addr[0], >> + RTE_FLOW_ACTION_TYPE_SET_MAC_SRC), >> + SA_INFO(eth_dst, dst->mac_addr[0], >> + RTE_FLOW_ACTION_TYPE_SET_MAC_DST), >> + }; >> + >> + if (add_set_flow_action(actions, sa_info_arr, >> + ARRAY_SIZE(sa_info_arr))) { >> + return -1; >> + } >> + } else { >> + VLOG_DBG_RL(&error_rl, >> + "Unsupported set action type=%d", nl_attr_type(sa)); >> + return -1; >> + } >> + } >> + >> + return 0; >> +} >> + >> static int >> parse_flow_actions(struct netdev *netdev, >> struct flow_actions *actions, >> @@ -528,6 +625,16 @@ parse_flow_actions(struct netdev *netdev, >> add_output_action(netdev, actions, nla, info )) { >> return -1; >> } >> + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SET || >> + nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED) { >> + const struct nlattr *set_actions = nl_attr_get(nla); >> + const size_t set_actions_len = nl_attr_get_size(nla); >> + bool masked = nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED; >> + >> + if (parse_set_actions(actions, set_actions, set_actions_len, >> + masked)) { >> + return -1; >> + } >> } else { >> VLOG_DBG_RL(&error_rl, >> "Unsupported action type %d", nl_attr_type(nla)); >>
On 12/10/19 5:14 PM, Eli Britstein wrote: > > On 12/10/2019 4:12 PM, Andrew Rybchenko wrote: >> On 12/8/19 4:23 PM, Eli Britstein wrote: >>> Signed-off-by: Eli Britstein <elibr@mellanox.com> >>> Reviewed-by: Oz Shlomo <ozsh@mellanox.com> >>> --- >>> Documentation/howto/dpdk.rst | 1 + >>> NEWS | 3 +- >>> lib/netdev-dpdk.c | 15 ++++++ >>> lib/netdev-offload-dpdk.c | 107 +++++++++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 125 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst >>> index 6cedd7f63..d228493e9 100644 >>> --- a/Documentation/howto/dpdk.rst >>> +++ b/Documentation/howto/dpdk.rst >>> @@ -392,6 +392,7 @@ Supported actions for hardware offload are: >>> >>> - Output (a single output, as the last action). >>> - Drop. >>> +- Modification of Ethernet (mod_dl_src/mod_dl_dst). >>> >>> Further Reading >>> --------------- >>> diff --git a/NEWS b/NEWS >>> index d019e066f..3ade86d49 100644 >>> --- a/NEWS >>> +++ b/NEWS >>> @@ -26,7 +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 and drop actions (experimental). >>> + * Add hardware offload support for output, drop and set MAC actions >>> + (experimental). >>> >>> v2.12.0 - 03 Sep 2019 >>> --------------------- >>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >>> index 872a45e75..e67a3dd76 100644 >>> --- a/lib/netdev-dpdk.c >>> +++ b/lib/netdev-dpdk.c >>> @@ -4723,6 +4723,21 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) >>> } >>> } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { >>> ds_put_cstr(s, "rte flow drop action\n"); >>> + } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC || >>> + actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST) { >>> + const struct rte_flow_action_set_mac *set_mac = actions->conf; >>> + >>> + char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST >>> + ? "dst" : "src"; >>> + >>> + ds_put_format(s, "rte flow set-mac-%s action:\n", dirstr); >>> + if (set_mac) { >>> + ds_put_format(s, >>> + " Set-mac-%s: "ETH_ADDR_FMT"\n", >>> + dirstr, ETH_ADDR_BYTES_ARGS(set_mac->mac_addr)); >>> + } else { >>> + ds_put_format(s, " Set-mac-%s = null\n", dirstr); >>> + } >>> } else { >>> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); >>> } >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >>> index bffb69cad..07f5d4687 100644 >>> --- a/lib/netdev-offload-dpdk.c >>> +++ b/lib/netdev-offload-dpdk.c >>> @@ -511,6 +511,103 @@ add_output_action(struct netdev *netdev, >>> return ret; >>> } >>> >>> +struct set_action_info { >>> + const uint8_t *value, *mask; >>> + const uint8_t size; >>> + uint8_t *spec; >>> + const int attr; >>> +}; >>> + >>> +static int >>> +add_set_flow_action(struct flow_actions *actions, >>> + struct set_action_info *sa_info_arr, >>> + size_t sa_info_arr_size) >>> +{ >>> + int field, i; >>> + >>> + for (field = 0; field < sa_info_arr_size; field++) { >>> + if (sa_info_arr[field].mask) { >>> + /* DPDK does not support partially masked set actions. In such >>> + * case, fail the offload. >>> + */ >>> + if (sa_info_arr[field].mask[0] != 0x00 && >>> + sa_info_arr[field].mask[0] != 0xFF) { >>> + VLOG_DBG_RL(&error_rl, >>> + "Partial mask is not supported"); >>> + return -1; >>> + } >>> + >>> + for (i = 1; i < sa_info_arr[field].size; i++) { >>> + if (sa_info_arr[field].mask[i] != >>> + sa_info_arr[field].mask[i - 1]) { >>> + VLOG_DBG_RL(&error_rl, >>> + "Partial mask is not supported"); >>> + return -1; >>> + } >>> + } >>> + >>> + if (sa_info_arr[field].mask[0] == 0x00) { >>> + /* mask bytes are all 0 - no rewrite action required */ >>> + continue; >>> + } >>> + } >>> + >>> + memcpy(sa_info_arr[field].spec, sa_info_arr[field].value, >>> + sa_info_arr[field].size); >>> + add_flow_action(actions, sa_info_arr[field].attr, >>> + sa_info_arr[field].spec); >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +/* Mask is at the midpoint of the data. */ >>> +#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1) >>> + >>> +#define SA_INFO(_field, _spec, _attr) { \ >>> + .value = (uint8_t *)&key->_field, \ >>> + .mask = (masked) ? (uint8_t *)&mask->_field : NULL, \ >>> + .size = sizeof key->_field, \ >>> + .spec = (uint8_t *)&_spec, \ >>> + .attr = _attr } >>> + >>> +static int >>> +parse_set_actions(struct flow_actions *actions, >>> + const struct nlattr *set_actions, >>> + const size_t set_actions_len, >>> + bool masked) >>> +{ >>> + const struct nlattr *sa; >>> + unsigned int sleft; >>> + >>> + 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 rte_flow_action_set_mac *src = xzalloc(sizeof *src); >>> + struct rte_flow_action_set_mac *dst = xzalloc(sizeof *dst); >> I've failed to find where memory allocated above is freed. > Right. I saw it too after submission. See > https://patchwork.ozlabs.org/patch/1205673/ Yes, I'm sorry for spam, I should take a look at available follow ups first. >> >>> + struct set_action_info sa_info_arr[] = { >>> + SA_INFO(eth_src, src->mac_addr[0], >>> + RTE_FLOW_ACTION_TYPE_SET_MAC_SRC), >>> + SA_INFO(eth_dst, dst->mac_addr[0], >>> + RTE_FLOW_ACTION_TYPE_SET_MAC_DST), >>> + }; >>> + >>> + if (add_set_flow_action(actions, sa_info_arr, >>> + ARRAY_SIZE(sa_info_arr))) { >>> + return -1; >>> + } >>> + } else { >>> + VLOG_DBG_RL(&error_rl, >>> + "Unsupported set action type=%d", nl_attr_type(sa)); >>> + return -1; >>> + } >>> + } >>> + >>> + return 0; >>> +} >>> + >>> static int >>> parse_flow_actions(struct netdev *netdev, >>> struct flow_actions *actions, >>> @@ -528,6 +625,16 @@ parse_flow_actions(struct netdev *netdev, >>> add_output_action(netdev, actions, nla, info )) { >>> return -1; >>> } >>> + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SET || >>> + nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED) { >>> + const struct nlattr *set_actions = nl_attr_get(nla); >>> + const size_t set_actions_len = nl_attr_get_size(nla); >>> + bool masked = nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED; >>> + >>> + if (parse_set_actions(actions, set_actions, set_actions_len, >>> + masked)) { >>> + return -1; >>> + } >>> } else { >>> VLOG_DBG_RL(&error_rl, >>> "Unsupported action type %d", nl_attr_type(nla)); >>>
On 08.12.2019 14:23, Eli Britstein wrote: > Signed-off-by: Eli Britstein <elibr@mellanox.com> > Reviewed-by: Oz Shlomo <ozsh@mellanox.com> > --- > Documentation/howto/dpdk.rst | 1 + > NEWS | 3 +- > lib/netdev-dpdk.c | 15 ++++++ > lib/netdev-offload-dpdk.c | 107 +++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 125 insertions(+), 1 deletion(-) > > diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst > index 6cedd7f63..d228493e9 100644 > --- a/Documentation/howto/dpdk.rst > +++ b/Documentation/howto/dpdk.rst > @@ -392,6 +392,7 @@ Supported actions for hardware offload are: > > - Output (a single output, as the last action). > - Drop. > +- Modification of Ethernet (mod_dl_src/mod_dl_dst). > > Further Reading > --------------- > diff --git a/NEWS b/NEWS > index d019e066f..3ade86d49 100644 > --- a/NEWS > +++ b/NEWS > @@ -26,7 +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 and drop actions (experimental). > + * Add hardware offload support for output, drop and set MAC actions > + (experimental). > > v2.12.0 - 03 Sep 2019 > --------------------- > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 872a45e75..e67a3dd76 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -4723,6 +4723,21 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) > } > } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { > ds_put_cstr(s, "rte flow drop action\n"); > + } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC || > + actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST) { > + const struct rte_flow_action_set_mac *set_mac = actions->conf; > + > + char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST > + ? "dst" : "src"; > + > + ds_put_format(s, "rte flow set-mac-%s action:\n", dirstr); > + if (set_mac) { > + ds_put_format(s, > + " Set-mac-%s: "ETH_ADDR_FMT"\n", > + dirstr, ETH_ADDR_BYTES_ARGS(set_mac->mac_addr)); > + } else { > + ds_put_format(s, " Set-mac-%s = null\n", dirstr); > + } > } else { > ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); > } > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index bffb69cad..07f5d4687 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -511,6 +511,103 @@ add_output_action(struct netdev *netdev, > return ret; > } > > +struct set_action_info { > + const uint8_t *value, *mask; > + const uint8_t size; > + uint8_t *spec; > + const int attr; > +}; > + > +static int > +add_set_flow_action(struct flow_actions *actions, > + struct set_action_info *sa_info_arr, > + size_t sa_info_arr_size) > +{ > + int field, i; > + > + for (field = 0; field < sa_info_arr_size; field++) { > + if (sa_info_arr[field].mask) { > + /* DPDK does not support partially masked set actions. In such > + * case, fail the offload. > + */ Could you, please, just use is_all_zeros/ones() functions? > + if (sa_info_arr[field].mask[0] != 0x00 && > + sa_info_arr[field].mask[0] != 0xFF) { > + VLOG_DBG_RL(&error_rl, > + "Partial mask is not supported"); > + return -1; > + } > + > + for (i = 1; i < sa_info_arr[field].size; i++) { > + if (sa_info_arr[field].mask[i] != > + sa_info_arr[field].mask[i - 1]) { > + VLOG_DBG_RL(&error_rl, > + "Partial mask is not supported"); > + return -1; > + } > + } > + > + if (sa_info_arr[field].mask[0] == 0x00) { > + /* mask bytes are all 0 - no rewrite action required */ > + continue; > + } > + } > + > + memcpy(sa_info_arr[field].spec, sa_info_arr[field].value, > + sa_info_arr[field].size); > + add_flow_action(actions, sa_info_arr[field].attr, > + sa_info_arr[field].spec); > + } > + > + return 0; > +} > + > +/* Mask is at the midpoint of the data. */ > +#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1) > + > +#define SA_INFO(_field, _spec, _attr) { \ > + .value = (uint8_t *)&key->_field, \ > + .mask = (masked) ? (uint8_t *)&mask->_field : NULL, \ > + .size = sizeof key->_field, \ > + .spec = (uint8_t *)&_spec, \ > + .attr = _attr } All of this doesn't look good and not easy to read. I think it might be much better if you'll just pass required things to add_set_flow_action() and call this function once for each action instead of looping inside. > + > +static int > +parse_set_actions(struct flow_actions *actions, > + const struct nlattr *set_actions, > + const size_t set_actions_len, > + bool masked) > +{ > + const struct nlattr *sa; > + unsigned int sleft; > + > + 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 rte_flow_action_set_mac *src = xzalloc(sizeof *src); > + struct rte_flow_action_set_mac *dst = xzalloc(sizeof *dst); > + struct set_action_info sa_info_arr[] = { > + SA_INFO(eth_src, src->mac_addr[0], > + RTE_FLOW_ACTION_TYPE_SET_MAC_SRC), > + SA_INFO(eth_dst, dst->mac_addr[0], > + RTE_FLOW_ACTION_TYPE_SET_MAC_DST), > + }; > + > + if (add_set_flow_action(actions, sa_info_arr, > + ARRAY_SIZE(sa_info_arr))) { > + return -1; > + } > + } else { > + VLOG_DBG_RL(&error_rl, > + "Unsupported set action type=%d", nl_attr_type(sa)); > + return -1; > + } > + } > + > + return 0; > +} > + > static int > parse_flow_actions(struct netdev *netdev, > struct flow_actions *actions, > @@ -528,6 +625,16 @@ parse_flow_actions(struct netdev *netdev, > add_output_action(netdev, actions, nla, info )) { > return -1; > } > + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SET || > + nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED) { > + const struct nlattr *set_actions = nl_attr_get(nla); > + const size_t set_actions_len = nl_attr_get_size(nla); > + bool masked = nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED; > + > + if (parse_set_actions(actions, set_actions, set_actions_len, > + masked)) { > + return -1; > + } > } else { > VLOG_DBG_RL(&error_rl, > "Unsupported action type %d", nl_attr_type(nla)); >
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst index 6cedd7f63..d228493e9 100644 --- a/Documentation/howto/dpdk.rst +++ b/Documentation/howto/dpdk.rst @@ -392,6 +392,7 @@ Supported actions for hardware offload are: - Output (a single output, as the last action). - Drop. +- Modification of Ethernet (mod_dl_src/mod_dl_dst). Further Reading --------------- diff --git a/NEWS b/NEWS index d019e066f..3ade86d49 100644 --- a/NEWS +++ b/NEWS @@ -26,7 +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 and drop actions (experimental). + * Add hardware offload support for output, drop and set MAC actions + (experimental). v2.12.0 - 03 Sep 2019 --------------------- diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 872a45e75..e67a3dd76 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -4723,6 +4723,21 @@ ds_put_flow_action(struct ds *s, const struct rte_flow_action *actions) } } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) { ds_put_cstr(s, "rte flow drop action\n"); + } else if (actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_SRC || + actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST) { + const struct rte_flow_action_set_mac *set_mac = actions->conf; + + char *dirstr = actions->type == RTE_FLOW_ACTION_TYPE_SET_MAC_DST + ? "dst" : "src"; + + ds_put_format(s, "rte flow set-mac-%s action:\n", dirstr); + if (set_mac) { + ds_put_format(s, + " Set-mac-%s: "ETH_ADDR_FMT"\n", + dirstr, ETH_ADDR_BYTES_ARGS(set_mac->mac_addr)); + } else { + ds_put_format(s, " Set-mac-%s = null\n", dirstr); + } } else { ds_put_format(s, "unknown rte flow action (%d)\n", actions->type); } diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index bffb69cad..07f5d4687 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -511,6 +511,103 @@ add_output_action(struct netdev *netdev, return ret; } +struct set_action_info { + const uint8_t *value, *mask; + const uint8_t size; + uint8_t *spec; + const int attr; +}; + +static int +add_set_flow_action(struct flow_actions *actions, + struct set_action_info *sa_info_arr, + size_t sa_info_arr_size) +{ + int field, i; + + for (field = 0; field < sa_info_arr_size; field++) { + if (sa_info_arr[field].mask) { + /* DPDK does not support partially masked set actions. In such + * case, fail the offload. + */ + if (sa_info_arr[field].mask[0] != 0x00 && + sa_info_arr[field].mask[0] != 0xFF) { + VLOG_DBG_RL(&error_rl, + "Partial mask is not supported"); + return -1; + } + + for (i = 1; i < sa_info_arr[field].size; i++) { + if (sa_info_arr[field].mask[i] != + sa_info_arr[field].mask[i - 1]) { + VLOG_DBG_RL(&error_rl, + "Partial mask is not supported"); + return -1; + } + } + + if (sa_info_arr[field].mask[0] == 0x00) { + /* mask bytes are all 0 - no rewrite action required */ + continue; + } + } + + memcpy(sa_info_arr[field].spec, sa_info_arr[field].value, + sa_info_arr[field].size); + add_flow_action(actions, sa_info_arr[field].attr, + sa_info_arr[field].spec); + } + + return 0; +} + +/* Mask is at the midpoint of the data. */ +#define get_mask(a, type) ((const type *)(const void *)(a + 1) + 1) + +#define SA_INFO(_field, _spec, _attr) { \ + .value = (uint8_t *)&key->_field, \ + .mask = (masked) ? (uint8_t *)&mask->_field : NULL, \ + .size = sizeof key->_field, \ + .spec = (uint8_t *)&_spec, \ + .attr = _attr } + +static int +parse_set_actions(struct flow_actions *actions, + const struct nlattr *set_actions, + const size_t set_actions_len, + bool masked) +{ + const struct nlattr *sa; + unsigned int sleft; + + 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 rte_flow_action_set_mac *src = xzalloc(sizeof *src); + struct rte_flow_action_set_mac *dst = xzalloc(sizeof *dst); + struct set_action_info sa_info_arr[] = { + SA_INFO(eth_src, src->mac_addr[0], + RTE_FLOW_ACTION_TYPE_SET_MAC_SRC), + SA_INFO(eth_dst, dst->mac_addr[0], + RTE_FLOW_ACTION_TYPE_SET_MAC_DST), + }; + + if (add_set_flow_action(actions, sa_info_arr, + ARRAY_SIZE(sa_info_arr))) { + return -1; + } + } else { + VLOG_DBG_RL(&error_rl, + "Unsupported set action type=%d", nl_attr_type(sa)); + return -1; + } + } + + return 0; +} + static int parse_flow_actions(struct netdev *netdev, struct flow_actions *actions, @@ -528,6 +625,16 @@ parse_flow_actions(struct netdev *netdev, add_output_action(netdev, actions, nla, info )) { return -1; } + } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SET || + nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED) { + const struct nlattr *set_actions = nl_attr_get(nla); + const size_t set_actions_len = nl_attr_get_size(nla); + bool masked = nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED; + + if (parse_set_actions(actions, set_actions, set_actions_len, + masked)) { + return -1; + } } else { VLOG_DBG_RL(&error_rl, "Unsupported action type %d", nl_attr_type(nla));