On Wed, Oct 14, 2020 at 10:25 PM Xie He <xie.he.0...@gmail.com> wrote: > > On Wed, Oct 14, 2020 at 6:38 PM Xie He <xie.he.0...@gmail.com> wrote: > > > > On Wed, Oct 14, 2020 at 1:19 PM Willem de Bruijn > > <willemdebruijn.ker...@gmail.com> wrote: > > > > > > On Wed, Oct 14, 2020 at 3:48 PM Xie He <xie.he.0...@gmail.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.