On 11/22/2019 6:19 PM, Ilya Maximets wrote:
> On 20.11.2019 16:28, Eli Britstein wrote:
>> Signed-off-by: Eli Britstein <el...@mellanox.com>
>> Reviewed-by: Oz Shlomo <o...@mellanox.com>
>> ---
>> NEWS | 2 +
>> lib/netdev-offload-dpdk-flow.c | 87
>> ++++++++++++++++++++++++++++++++++++++++--
>> 2 files changed, 85 insertions(+), 4 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 88b818948..ca9c2b230 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -10,6 +10,8 @@ Post-v2.12.0
>> if supported by libbpf.
>> * Add option to enable, disable and query TCP sequence checking in
>> conntrack.
>> + - DPDK:
>> + * Add hardware offload support for output actions.
>>
>> v2.12.0 - 03 Sep 2019
>> ---------------------
>> diff --git a/lib/netdev-offload-dpdk-flow.c b/lib/netdev-offload-dpdk-flow.c
>> index dbbc45e99..6e7efb315 100644
>> --- a/lib/netdev-offload-dpdk-flow.c
>> +++ b/lib/netdev-offload-dpdk-flow.c
>> @@ -274,6 +274,28 @@ ds_put_flow_action(struct ds *s, const struct
>> rte_flow_action *actions)
>> } else {
>> ds_put_cstr(s, " RSS = null\n");
>> }
>> + } else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
>> + const struct rte_flow_action_count *count = actions->conf;
>> +
>> + ds_put_cstr(s, "rte flow count action:\n");
>> + if (count) {
>> + ds_put_format(s,
>> + " Count: shared=%d, id=%d\n",
>> + count->shared, count->id);
>> + } else {
>> + ds_put_cstr(s, " Count = null\n");
>> + }
>> + } else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
>> + const struct rte_flow_action_port_id *port_id = actions->conf;
>> +
>> + ds_put_cstr(s, "rte flow port-id action:\n");
>> + if (port_id) {
>> + ds_put_format(s,
>> + " Port-id: original=%d, id=%d\n",
>> + port_id->original, port_id->id);
>> + } else {
>> + ds_put_cstr(s, " Port-id = null\n");
>> + }
>> } else {
>> ds_put_format(s, "unknown rte flow action (%d)\n", actions->type);
>> }
>> @@ -531,19 +553,76 @@ netdev_dpdk_flow_patterns_add(struct flow_patterns
>> *patterns,
>> return 0;
>> }
>>
>> +static void
>> +netdev_dpdk_flow_add_count_action(struct flow_actions *actions)
>> +{
>> + struct rte_flow_action_count *count = xzalloc(sizeof *count);
>> +
>> + count->shared = 0;
>> + /* Each flow has a single count action, so no need of id */
>> + count->id = 0;
>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_COUNT, count);
>> +}
>> +
>> +static void
>> +netdev_dpdk_flow_add_port_id_action(struct flow_actions *actions,
>> + struct netdev *outdev)
>> +{
>> + struct rte_flow_action_port_id *port_id = xzalloc(sizeof *port_id);
>> +
>> + port_id->id = netdev_dpdk_get_port_id(outdev);
>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
>> +}
>> +
>> +static int
>> +netdev_dpdk_flow_add_output_action(struct flow_actions *actions,
>> + const struct nlattr *nla,
>> + struct offload_info *info)
>> +{
>> + struct netdev *outdev;
>> + odp_port_t port;
>> +
>> + port = nl_attr_get_odp_port(nla);
>> + outdev = netdev_ports_get(port, info->dpif_class);
>> + if (outdev == NULL) {
>> + VLOG_DBG_RL(&error_rl,
>> + "Cannot find netdev for odp port %d", port);
>> + return -1;
>> + }
>> + if (!netdev_dpdk_flow_api_supported(outdev)) {
> This doesn't look correct. I mean, maybe we need to check if port is
> really the port on a same piece of hardware. Will the flow setup fail
> in this case? Will code fallback to the MARK+RSS?
We cannot check if the port is on the same HW, and it is also wrong from
the application point of view. If the operation cannot be supported, it
is in the driver (DPDK) scope to fail it.
You are right about the fallback. I'll fix it in v2.
>
>> + VLOG_DBG_RL(&error_rl,
>> + "Output to %s cannot be offloaded",
>> + netdev_get_name(outdev));
>> + return -1;
>> + }
>> +
>> + netdev_dpdk_flow_add_count_action(actions);
>> + netdev_dpdk_flow_add_port_id_action(actions, outdev);
>> + netdev_close(outdev);
>> +
>> + return 0;
>> +}
>> +
>> int
>> netdev_dpdk_flow_actions_add(struct flow_actions *actions,
>> struct nlattr *nl_actions,
>> size_t nl_actions_len,
>> - struct offload_info *info OVS_UNUSED)
>> + struct offload_info *info)
>> {
>> struct nlattr *nla;
>> size_t left;
>>
>> NL_ATTR_FOR_EACH_UNSAFE (nla, left, nl_actions, nl_actions_len) {
>> - VLOG_DBG_RL(&error_rl,
>> - "Unsupported action type %d", nl_attr_type(nla));
>> - return -1;
>> + if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT &&
>> + left <= NLA_ALIGN(nla->nla_len)) {
>> + if (netdev_dpdk_flow_add_output_action(actions, nla, info )) {
>> + return -1;
>> + }
>> + } else {
>> + VLOG_DBG_RL(&error_rl,
>> + "Unsupported action type %d", nl_attr_type(nla));
>> + return -1;
>> + }
>> }
>>
>> add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
>>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
