On Thu, Nov 29, 2018 at 5:56 AM Sabrina Dubroca <[email protected]> wrote:
>
> 2018-11-28, 14:27:57 -0800, Roopa Prabhu wrote:
> > From: Roopa Prabhu <[email protected]>
> >
> > We started very conservative when supporting changelink
> > especially because not all attribute changes could be
> > tested. This patch opens up a few more attributes for
> > changelink. The reason for choosing this set of attributes
> > is based on code references for these attributes. I have
> > tested TTL changes and did some changelink api testing
> > to sanity test the others.
> >
> > Signed-off-by: Roopa Prabhu <[email protected]>
> > ---
> > drivers/net/vxlan.c | 36 ++++--------------------------------
> > 1 file changed, 4 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> > index 9110662..73caa65 100644
> > --- a/drivers/net/vxlan.c
> > +++ b/drivers/net/vxlan.c
> > @@ -3438,11 +3438,8 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct
> > nlattr *data[],
> > if (data[IFLA_VXLAN_TTL])
> > conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
> >
> > - if (data[IFLA_VXLAN_TTL_INHERIT]) {
> > - if (changelink)
> > - return -EOPNOTSUPP;
> > + if (data[IFLA_VXLAN_TTL_INHERIT])
> > conf->flags |= VXLAN_F_TTL_INHERIT;
> > - }
>
> This doesn't give us an option to disable TTL_INHERIT after it was
> enabled once. Same thing with GBP, GPE, REMCSUM_NOPARTIAL.
that is provided by patch3, with the changelink patch. I can squash
them, if thats easier.
>
> > - if (data[IFLA_VXLAN_GBP]) {
> > - if (changelink)
> > - return -EOPNOTSUPP;
> > + if (data[IFLA_VXLAN_GBP])
> > conf->flags |= VXLAN_F_GBP;
> > - }
> >
> > - if (data[IFLA_VXLAN_GPE]) {
> > - if (changelink)
> > - return -EOPNOTSUPP;
> > + if (data[IFLA_VXLAN_GPE])
> > conf->flags |= VXLAN_F_GPE;
> > - }
>
> GPE implies running a different setup function (vxlan_raw_setup() vs
> vxlan_ether_setup()), that vxlan_config_apply() only calls for
> !changelink. I think this is incomplete.
>
> I think we'd also end up with mixed tunnel types (GPE/!GPE) on the
> same socket, I'm not sure how that would work. Normally, they each try
> to create a separate socket, and pass the GPE flag on to the
> associated vxlan_sock. I suspect that's also a problem with rx
> offload.
that is good to know. I will drop the change to GPE and also the rx
offload flag and let somebody else using it
open it up for changelink. thanks for the review
> > - if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
> > - if (changelink)
> > - return -EOPNOTSUPP;
> > + if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
> > conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
> > - }
> >
> > if (tb[IFLA_MTU]) {
> > if (changelink)
> > --
