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.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?
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. ...
