netdev - Re: [Patch net v2] ip_gre: set dev->hard_header_len and dev->needed

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:   Thu, 15 Oct 2020 09:42:08 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Xie He <xie.he.0141@...il.com>
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        Network Development <netdev@...r.kernel.org>,
        syzbot <syzbot+4a2c52677a8a1aa283cb@...kaller.appspotmail.com>,
        William Tu <u9012063@...il.com>
Subject: Re: [Patch net v2] ip_gre: set dev->hard_header_len and
 dev->needed_headroom properly

On Wed, Oct 14, 2020 at 10:25 PM Xie He <xie.he.0141@...il.com> wrote:
>
> On Wed, Oct 14, 2020 at 6:38 PM Xie He <xie.he.0141@...il.com> wrote:
> >
> > On Wed, Oct 14, 2020 at 1:19 PM Willem de Bruijn
> > <willemdebruijn.kernel@...il.com> wrote:
> > >
> > > On Wed, Oct 14, 2020 at 3:48 PM Xie He <xie.he.0141@...il.com> wrote:
> > > >
> > > > I thought we agreed that ideally GRE devices would not have
> > > > header_ops. Currently GRE devices (in normal situations) indeed do not
> > > > use header_ops (and use ARHPHRD_IPGRE as dev->type). I think we should
> > > > keep this behavior.
> > > >
> > > > To solve the problem of the same dev->type having different
> > > > hard_header_len values which you mentioned. I think we should create a
> > > > new dev->type (ARPHRD_IPGRE_SPECIAL) for GRE devices that use
> > > > header_ops.
> > > >
> > > > Also, for collect_md, I think we should use ARHPHRD_IPGRE. I see no
> > > > reason to use ARPHRD_NONE.
> > >
> > > What does ARPHRD_IPGRE define beyond ARPHRD_NONE? And same for
> > > ARPHRD_TUNNEL variants. If they are indistinguishable, they are the
> > > same and might as well have the same label.
> >
> > It is indeed reasonable to keep devices indistinguishable to each
> > other having the same dev->type label. But I see a lot of devices in
> > the kernel without header_ops having different dev->type labels. For
> > example, ARPHRD_SLIP should be the same as ARPHRD_RAWIP. One feature
> > distinguishing these devices might be their dev->mtu.
> >
> > GRE devices may have their special dev->mtu determined by the maximum
> > IP packet size and the GRE header length.
> >
> > For ARPHRD_TUNNEL, it may also have its own dev->mtu. I also see it
> > has header_ops->parse_protocol (but it doesn't have
> > header_ops->create).
>
> Actually I think dev->type can be seen from user space. For example,
> when you type "ip link", it will display the link type for you. So I
> think it is useful to keep different dev->type labels without merging
> them even if they appear to have no difference.

Ah, indeed. These constants are matched in iproute2 in lib/ll_types.c
to string representations.

Good catch. Yes, then they have to stay as is.