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  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:   Thu, 18 Jun 2020 15:03:47 +0200
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     Stefano Brivio <sbrivio@...hat.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net] geneve: allow changing DF behavior after creation

2020-06-18, 12:26:29 +0200, Stefano Brivio wrote:
> On Thu, 18 Jun 2020 12:13:22 +0200
> Sabrina Dubroca <sd@...asysnail.net> wrote:
> 
> > Currently, trying to change the DF parameter of a geneve device does
> > nothing:
> > 
> >     # ip -d link show geneve1
> >     14: geneve1: <snip>
> >         link/ether <snip>
> >         geneve id 1 remote 10.0.0.1 ttl auto df set dstport 6081 <snip>
> >     # ip link set geneve1 type geneve id 1 df unset
> >     # ip -d link show geneve1
> >     14: geneve1: <snip>
> >         link/ether <snip>
> >         geneve id 1 remote 10.0.0.1 ttl auto df set dstport 6081 <snip>
> > 
> > We just need to update the value in geneve_changelink.
> > 
> > Fixes: a025fb5f49ad ("geneve: Allow configuration of DF behaviour")
> > Signed-off-by: Sabrina Dubroca <sd@...asysnail.net>
> > ---
> >  drivers/net/geneve.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> > index 75266580b586..4661ef865807 100644
> > --- a/drivers/net/geneve.c
> > +++ b/drivers/net/geneve.c
> > @@ -1649,6 +1649,7 @@ static int geneve_changelink(struct net_device *dev, struct nlattr *tb[],
> >  	geneve->collect_md = metadata;
> >  	geneve->use_udp6_rx_checksums = use_udp6_rx_checksums;
> >  	geneve->ttl_inherit = ttl_inherit;
> > +	geneve->df = df;
> 
> I introduced this bug as I didn't notice the asymmetry with VXLAN,
> where vxlan_nl2conf() takes care of this for both new links and link
> changes.

Yeah, I didn't notice either :/

> Here, this block is duplicated in geneve_configure(), which,
> somewhat surprisingly given the name, is not called from
> geneve_changelink(). Did you consider factoring out (at least) this
> block to have it shared?

Then I'd have to introduce another lovely function with an absurdly
long argument list. I'd rather clean that up in all of geneve and
introduce something like struct vxlan_config, but it's a bit much for
net. I'll do that once this fix finds its way into net-next.

> 
> Either way,
> 
> Reviewed-by: Stefano Brivio <sbrivio@...hat.com>

Thanks.

-- 
Sabrina

Powered by blists - more mailing lists