On Wed, Oct 9, 2019 at 3:55 PM Simon Horman <simon.hor...@netronome.com> wrote:
>
> On Tue, Oct 08, 2019 at 11:16:12PM +0800, Xin Long wrote:
> > This patch is to add LWTUNNEL_IP_OPTS into lwtunnel_ip_t, by which
> > users will be able to set options for ip_tunnel_info by "ip route
> > encap" for erspan and vxlan's private metadata. Like one way to go
> > in iproute is:
> >
> > # ip route add 1.1.1.0/24 encap ip id 1 erspan ver 1 idx 123 \
> > dst 10.1.0.2 dev erspan1
> > # ip route show
> > 1.1.1.0/24 encap ip id 1 src 0.0.0.0 dst 10.1.0.2 ttl 0 \
> > tos 0 erspan ver 1 idx 123 dev erspan1 scope link
> >
> > Signed-off-by: Xin Long <lucien....@gmail.com>
>
> Hi Xin,
>
> thanks for your patch.
>
> While I have no objection to allowing options to be set, as per the sample
> ip commands above, I am concerned that the approach you have taken exposes
> to userspace the internal encoding used by the kernel for these options.
>
> This is the same concerned that was raised by others when I posed a patch
> to allow setting of Geneve options in a similar manner. I think what is
> called for here, as was the case in the Geneve work, is to expose netlink
> attributes for each option that may be set and have the kernel form
> these into the internal format (which appears to also be the wire format).
Understand.
Do you think if it's necessary to support for setting these options
by ip route command?
or if yes, should we introduce a global lwtun_option_ops_list where
geneve/vxlan/erspan could register their own option parsing functions?
>
> > ---
> > include/uapi/linux/lwtunnel.h | 1 +
> > net/ipv4/ip_tunnel_core.c | 30 ++++++++++++++++++++++++------
> > 2 files changed, 25 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/uapi/linux/lwtunnel.h b/include/uapi/linux/lwtunnel.h
> > index de696ca..93f2c05 100644
> > --- a/include/uapi/linux/lwtunnel.h
> > +++ b/include/uapi/linux/lwtunnel.h
> > @@ -27,6 +27,7 @@ enum lwtunnel_ip_t {
> > LWTUNNEL_IP_TOS,
> > LWTUNNEL_IP_FLAGS,
> > LWTUNNEL_IP_PAD,
> > + LWTUNNEL_IP_OPTS,
> > __LWTUNNEL_IP_MAX,
> > };
> >
> > diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
> > index 10f0848..d9b7188 100644
> > --- a/net/ipv4/ip_tunnel_core.c
> > +++ b/net/ipv4/ip_tunnel_core.c
> > @@ -218,6 +218,7 @@ static const struct nla_policy
> > ip_tun_policy[LWTUNNEL_IP_MAX + 1] = {
> > [LWTUNNEL_IP_TTL] = { .type = NLA_U8 },
> > [LWTUNNEL_IP_TOS] = { .type = NLA_U8 },
> > [LWTUNNEL_IP_FLAGS] = { .type = NLA_U16 },
> > + [LWTUNNEL_IP_OPTS] = { .type = NLA_BINARY },
> > };
>
> ...
