Re: [PATCH bpf-next] bpf: Add document for 'dst_port' of 'struct bpf_sock'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 25, 2022 at 2:45 PM Martin KaFai Lau <kafai@xxxxxx> wrote:
>
> On Tue, Jan 25, 2022 at 08:24:27PM +0100, Jakub Sitnicki wrote:
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index b0383d371b9a..891a182a749a 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -5500,7 +5500,11 @@ struct bpf_sock {
> > >     __u32 src_ip4;
> > >     __u32 src_ip6[4];
> > >     __u32 src_port;         /* host byte order */
> > > -   __u32 dst_port;         /* network byte order */
> > > +   __u32 dst_port;         /* low 16-bits are in network byte order,
> > > +                            * and high 16-bits are filled by 0.
> > > +                            * So the real port in host byte order is
> > > +                            * bpf_ntohs((__u16)dst_port).
> > > +                            */
> > >     __u32 dst_ip4;
> > >     __u32 dst_ip6[4];
> > >     __u32 state;
> >
> > I'm probably missing something obvious, but is there anything stopping
> > us from splitting the field, so that dst_ports is 16-bit wide?
> >
> > I gave a quick check to the change below and it seems to pass verifier
> > checks and sock_field tests.
> >
> > IDK, just an idea. Didn't give it a deeper thought.
> >
> > --8<--
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 4a2f7041ebae..344d62ccafba 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -5574,7 +5574,8 @@ struct bpf_sock {
> >       __u32 src_ip4;
> >       __u32 src_ip6[4];
> >       __u32 src_port;         /* host byte order */
> > -     __u32 dst_port;         /* network byte order */
> > +     __u16 unused;
> > +     __u16 dst_port;         /* network byte order */
> This will break the existing bpf prog.

I think Jakub's idea is partially expressed:
+       case offsetof(struct bpf_sock, dst_port):
+               bpf_ctx_record_field_size(info, sizeof(__u16));
+               return bpf_ctx_narrow_access_ok(off, size, sizeof(__u16));

Either 'unused' needs to be after dst_port or
bpf_sock_is_valid_access() needs to allow offset at 'unused'
and at 'dst_port'.
And allow u32 access though the size is actually u16.
Then the existing bpf progs (without recompiling) should work?



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux