| Message ID | 1438312222-7130-1-git-send-email-ast@plumgrid.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On 07/30/15 at 08:10pm, Alexei Starovoitov wrote: > Two vxlan driver flags FLOWBASED and COLLECT_METADATA need to be set to > make use of its new flow mode. The former already exposed. Expose the latter. > > Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> Since you are analyzing the program when you load it. Could you call ip_tunnel_need_metadata() when we load a program which accesses the new metadata field and ip_tunnel_unneed_metadata() when you unload it? This would hide the need to set this flag from the user. The con is that it enables metadata on all net_devices. I'm also fine with exposing this flag to the user though. I'll leave it up to you so feel free to add my ack if you want to go this route. Acked-by: Thomas Graf <tgraf@suug.ch> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7/31/15 1:00 AM, Thomas Graf wrote: > On 07/30/15 at 08:10pm, Alexei Starovoitov wrote: >> Two vxlan driver flags FLOWBASED and COLLECT_METADATA need to be set to >> make use of its new flow mode. The former already exposed. Expose the latter. >> >> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> > > Since you are analyzing the program when you load it. Could you call > ip_tunnel_need_metadata() when we load a program which accesses > the new metadata field and ip_tunnel_unneed_metadata() when you unload > it? This would hide the need to set this flag from the user. The con > is that it enables metadata on all net_devices. I thought about it, but the global static_key enabler via ip_tunnel_collect_metadata() is an overkill when we only want to enable it for one tunnel. I can imagine why you may need it for routing, but every tunnel type needs its own collect_metadata flag similar to vxlan. Also in bpf there is no program scan at unload time. To call ip_tunnel_unneed_metadata() cleanly, I would need to extra global refcnt just for that. which is ugly. > I'm also fine with exposing this flag to the user though. I'll leave it > up to you so feel free to add my ack if you want to go this route. > > Acked-by: Thomas Graf <tgraf@suug.ch> thanks. I think exposing collect_metadata for vxlan and in the future for other tunnel types is the clean enough way, though the other alternative would be to get rid of collect_metadata flag from the kernel and do it when flowmode flag is set. Thoughts? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Alexei Starovoitov <ast@plumgrid.com> Date: Thu, 30 Jul 2015 20:10:22 -0700 > Two vxlan driver flags FLOWBASED and COLLECT_METADATA need to be set to > make use of its new flow mode. The former already exposed. Expose the latter. > > Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 31, 2015 at 8:41 AM, Alexei Starovoitov <ast@plumgrid.com> wrote: > thanks. I think exposing collect_metadata for vxlan and in the future > for other tunnel types is the clean enough way, though the other > alternative would be to get rid of collect_metadata flag > from the kernel and do it when flowmode flag is set. Thoughts? This seems like a good idea to me - I'm not sure that flow based tunnels are all that useful without metadata collection enabled and the fewer interfaces that we have to create for each tunnel type, the better. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/03/15 at 02:14pm, Jesse Gross wrote: > On Fri, Jul 31, 2015 at 8:41 AM, Alexei Starovoitov <ast@plumgrid.com> wrote: > > thanks. I think exposing collect_metadata for vxlan and in the future > > for other tunnel types is the clean enough way, though the other > > alternative would be to get rid of collect_metadata flag > > from the kernel and do it when flowmode flag is set. Thoughts? > > This seems like a good idea to me - I'm not sure that flow based > tunnels are all that useful without metadata collection enabled and > the fewer interfaces that we have to create for each tunnel type, the > better. The use case at hand where the flag is useful if someone is only doing flow based TX which can be handy if the tunnel id remains unused. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 81f0f24b2cfb..1205729fcc56 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -2463,6 +2463,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = { [IFLA_VXLAN_L2MISS] = { .type = NLA_U8 }, [IFLA_VXLAN_L3MISS] = { .type = NLA_U8 }, [IFLA_VXLAN_FLOWBASED] = { .type = NLA_U8 }, + [IFLA_VXLAN_COLLECT_METADATA] = { .type = NLA_U8 }, [IFLA_VXLAN_PORT] = { .type = NLA_U16 }, [IFLA_VXLAN_UDP_CSUM] = { .type = NLA_U8 }, [IFLA_VXLAN_UDP_ZERO_CSUM6_TX] = { .type = NLA_U8 }, @@ -2817,6 +2818,10 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev, nla_get_u8(data[IFLA_VXLAN_FLOWBASED])) conf.flags |= VXLAN_F_FLOW_BASED; + if (data[IFLA_VXLAN_COLLECT_METADATA] && + nla_get_u8(data[IFLA_VXLAN_COLLECT_METADATA])) + conf.flags |= VXLAN_F_COLLECT_METADATA; + if (data[IFLA_VXLAN_PORT_RANGE]) { const struct ifla_vxlan_port_range *p = nla_data(data[IFLA_VXLAN_PORT_RANGE]); diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 9eeb5d9cf8f0..24e22cd4be79 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -383,6 +383,7 @@ enum { IFLA_VXLAN_GBP, IFLA_VXLAN_REMCSUM_NOPARTIAL, IFLA_VXLAN_FLOWBASED, + IFLA_VXLAN_COLLECT_METADATA, __IFLA_VXLAN_MAX }; #define IFLA_VXLAN_MAX (__IFLA_VXLAN_MAX - 1)
Two vxlan driver flags FLOWBASED and COLLECT_METADATA need to be set to make use of its new flow mode. The former already exposed. Expose the latter. Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> --- drivers/net/vxlan.c | 5 +++++ include/uapi/linux/if_link.h | 1 + 2 files changed, 6 insertions(+)