netdev - Re: [PATCHv2 net-next 2/6] lwtunnel: add LWTUNNEL_IP_OPTS support for lwtunnel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Oct 2019 07:31:02 +0200
From:   Simon Horman <simon.horman@...ronome.com>
To:     Xin Long <lucien.xin@...il.com>
Cc:     network dev <netdev@...r.kernel.org>, davem <davem@...emloft.net>,
        Jiri Benc <jbenc@...hat.com>, Thomas Graf <tgraf@...g.ch>,
        William Tu <u9012063@...il.com>
Subject: Re: [PATCHv2 net-next 2/6] lwtunnel: add LWTUNNEL_IP_OPTS support
 for lwtunnel_ip

On Thu, Oct 10, 2019 at 05:45:14PM +0800, Xin Long wrote:
> On Wed, Oct 9, 2019 at 3:55 PM Simon Horman <simon.horman@...ronome.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.xin@...il.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?

Hi Xin,

In the case of Geneve the options are (now) exposed via tc cls_flower and
act_tunnel rather than ip route. The approach taken was to create an
ENC_OPTS attribute with a ENC_OPTS_GENEVE sub-attribute which in turn has
sub-attributes that allow options to be described.

The idea behind this design, as I recall, was to allow other options, say
for VXLAN, to be added via a new sub-attribute of ENC_OPTS.  Without
examining the details beyond your patch I think a similar approach would
work well for options supplied via ip route.

...