On Tue, Dec 10, 2019 at 1:21 PM Eli Britstein <el...@mellanox.com> wrote: > > > On 12/10/2019 8:52 AM, Sathya Perla wrote: > > On Sun, Dec 8, 2019 at 6:53 PM Eli Britstein <el...@mellanox.com> wrote: > > ... > >> +static int > >> +parse_clone_actions(struct netdev *netdev, > >> + struct flow_actions *actions, > >> + const struct nlattr *clone_actions, > >> + const size_t clone_actions_len, > >> + struct offload_info *info) > >> +{ > >> + const struct nlattr *ca; > >> + unsigned int cleft; > >> + > >> + NL_ATTR_FOR_EACH_UNSAFE (ca, cleft, clone_actions, clone_actions_len) > >> { > >> + int clone_type = nl_attr_type(ca); > >> + > >> + if (clone_type == OVS_ACTION_ATTR_TUNNEL_PUSH) { > >> + const struct ovs_action_push_tnl *tnl_push = nl_attr_get(ca); > >> + struct rte_flow_action_raw_encap *raw_encap = > >> + xzalloc(sizeof *raw_encap); > >> + > >> + raw_encap->data = (uint8_t *)tnl_push->header; > >> + raw_encap->preserve = NULL; > >> + raw_encap->size = tnl_push->header_len; > >> + > >> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RAW_ENCAP, > >> + raw_encap); > > Hi, converting OVS_TUNNLE_PUSH into RTE_RAW_ENCAP loses the 'tnl_type' > > information provided by OVS. RAW_ENCAP provides the tunnel header just > > as a blob of bits which may not be ideal for NIC HW to offload. > > > > How about using tnl_push->tnl_type field to parse the header and > > compose specific tunnel encap actions like RTE_VXLAN_ENCAP, > > RTE_NVGRE_ENCAP etc. > > This would be also be more inline with how it's done with TC-offload. > > I see your point. However, struct ovs_action_push_tnl has the "header" > field just as a raw binary buffer, and not detailed like in TC. > "tnl_type" has a comment /* For logging. */. It is indeed used for > logging, as in lib/odp-util.c, in function format_odp_tnl_push_header.
This is not entirely true. Checkout propagate_tunnel_data_to_flow() where tnl_type is being used to figure out nw_proto. > > using VXLAN_ENCAP instead of RAW_ENCAP will make the push less generic, > as I will have to parse the header to build the vxlan_encap fields, just > so they can re-build as a raw header in the PMD level, so I don't see > the offload benefit of it. Why are you assuming that all PMDs will rebuild the tunnel header fields as a 'raw header'? What if the NIC HW needs tunnel specific headers to be programmed separately for each tunnel type? > > Another point is that this way it will support any header, not just > VXLAN (as an example). Given that VXLAN and NVGRE ENCAP and DECAP actions are already defined in rte_flow, how about we use them for vxlan and nvgre and use RAW for the rest of the tunnel types? We can support all tunnel headers even this way..... thanks! _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev