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:   Tue, 20 Oct 2020 11:56:27 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     David Ahern <dsahern@...il.com>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH bpf v2 1/3] bpf_redirect_neigh: Support supplying the
 nexthop as a helper parameter

On Tue, 20 Oct 2020 12:12:32 -0600 David Ahern wrote:
> On 10/20/20 10:30 AM, Jakub Kicinski wrote:
> > On Tue, 20 Oct 2020 12:51:02 +0200 Toke Høiland-Jørgensen wrote:  
> >> diff --git a/include/linux/filter.h b/include/linux/filter.h
> >> index 20fc24c9779a..ba9de7188cd0 100644
> >> --- a/include/linux/filter.h
> >> +++ b/include/linux/filter.h
> >> @@ -607,12 +607,21 @@ struct bpf_skb_data_end {
> >>  	void *data_end;
> >>  };
> >>  
> >> +struct bpf_nh_params {
> >> +	u8 nh_family;
> >> +	union {
> >> +		__u32 ipv4_nh;
> >> +		struct in6_addr ipv6_nh;
> >> +	};
> >> +};  
> >   
> >> @@ -4906,6 +4910,18 @@ struct bpf_fib_lookup {
> >>  	__u8	dmac[6];     /* ETH_ALEN */
> >>  };
> >>  
> >> +struct bpf_redir_neigh {
> >> +	/* network family for lookup (AF_INET, AF_INET6) */
> >> +	__u8 nh_family;
> >> +	 /* avoid hole in struct - must be set to 0 */
> >> +	__u8 unused[3];
> >> +	/* network address of nexthop; skips fib lookup to find gateway */
> >> +	union {
> >> +		__be32		ipv4_nh;
> >> +		__u32		ipv6_nh[4];  /* in6_addr; network order */
> >> +	};
> >> +};  
> > 
> > Isn't this backward? The hole could be named in the internal structure.
> > This is a bit of a gray area, but if you name this hole in uAPI and
> > programs start referring to it you will never be able to reuse it.
> > So you may as well not require it to be zeroed..
> 
> for uapi naming the holes, stating they are unused and requiring a 0
> value allows them to be used later if an api change needs to.

I'm not sure what you're saying, if the field is referenced it can't be
removed. But we could use a union, so I guess it's not a deal breaker.

Powered by blists - more mailing lists