Add ETHER_ALIGN support to ng_device(4).

[manu]

Summary

This adds a new ng_device command, NGM_DEVICE_ETHERALIGN, which has no
associated args. After the command arrives, the device begins adjusting all
packets sent out its hook to have ETHER_ALIGN bytes of padding at the
beginning of the packet. The ETHER_ALIGN padding is added only when
running on an architecture that requires strict alignment of IP headers
(based on the __NO_STRICT_ALIGNMENT macro, which is only #define'd on
x86 as of this writing).

This also adds ascii <-> binary command translation to ng_device, both for
the existing NGM_DEVICE_GET_DEVNAME and the new ETHERALIGN command.

This also gives a name to every ng_device node when it is constructed, using
the cdev device name (ngd0, ngd1, etc). This makes it easier to address
command msgs to the device using ngctl(8).

Test Plan

Patch is from ian@ and tested by him at work.

[donner]

I welcome this extension to other architectures. Thank you.

But for my personal clarification, I'd like to know, why this is not applied by default.
Something like this:

#ifndef __NO_STRICT_ALIGNMENT
#define NG_DEVICE_ALIGN   ETHER_ALIGN
#else
#define NG_DEVICE_ALIGN   0
#endif
         m = m_uiotombuf(uio, M_NOWAIT, 0, NG_DEVICE_ALIGN, M_PKTHDR);

and forget about all the extra glue?

[manu]

In D32905#742890, @donner wrote:

I welcome this extension to other architectures. Thank you.

But for my personal clarification, I'd like to know, why this is not applied by default.
Something like this:

#ifndef __NO_STRICT_ALIGNMENT
#define NG_DEVICE_ALIGN   ETHER_ALIGN
#else
#define NG_DEVICE_ALIGN   0
#endif
         m = m_uiotombuf(uio, M_NOWAIT, 0, NG_DEVICE_ALIGN, M_PKTHDR);

and forget about all the extra glue?

I'm just proxying for ian@ as he forgot to take care of this patch before giving his commitbit back so here is his answer :
16:56 <DamnHippi> I can create one if needed, but the answer to his question is pretty simple:
16:57 <DamnHippi> The ng_device node doesn't know what kind of downstream node its packets will be sent to. ETHER_ALIGN must

only be applied if the packets will be going to an ng_eiface node.  So whether to add the alignment has to 
be configured by whoever is configuring node connections.

16:58 <DamnHippi> Oops, I think that line was too long?
16:58 <DamnHippi> The ng_device node doesn't know what kind of downstream node its packets will be sent to. ETHER_ALIGN must

only be applied if the packets will be going to an ng_eiface node.

16:58 <DamnHippi> So whether to add the alignment has to be configured by whoever is configuring node connections.

[afedorov]

I don't understand what the alignment does in this case.
Does it fix a bug or is it an optimization?
What part of the code requires this alignment, is it the TCP/IP stack?
Because not only ng_eiface(4) can inject Ethernet packets into the network stack.

Just some other examples of unalligned packets with netgraph:
ng_device(4) -> ng_ether::upper(4) -> TCP/IP stack
ng_socket(4) -> ng_eiface -> TCP/IP stack

As I can see, ETHER_ALIGN is only used by network drivers. Is there any alignment requirement for network drivers or TCP/IP stack?

Other changes, I think you can commit:

1. Support of the "getdevname".
2. Automatically give the node name same as device name.

[• ian]

In D32905#742921, @afedorov wrote:

I don't understand what the alignment does in this case.
Does it fix a bug or is it an optimization?
What part of the code requires this alignment, is it the TCP/IP stack?
Because not only ng_eiface(4) can inject Ethernet packets into the network stack.

Just some other examples of unalligned packets with netgraph:
ng_device(4) -> ng_ether::upper(4) -> TCP/IP stack
ng_socket(4) -> ng_eiface -> TCP/IP stack

As I can see, ETHER_ALIGN is only used by network drivers. Is there any alignment requirement for network drivers or TCP/IP stack?

Other changes, I think you can commit:

1. Support of the "getdevname".
2. Automatically give the node name same as device name.

The IP headers of packets injected into the network stack must be aligned on a 32-bit boundary on architectures that require alignment. Ethernet packets have a 6-byte header, making the IP header unaligned to 32 bits. If a packet is going to be injected via ether_input(), the two-byte offset on the mbuf is required so that the 6-byte ether header follows, then the IP header after that will be 32-bit aligned.

When packets are injected at other network stack ingress points, the IP header still must be 32-bit aligned. The thing that makes ethernet packets different is that because they start with a 6-byte header, drivers typically receive the incoming data into an mbuf that has a leading 2-byte offset, so that there is no need to copy the mbuf from the original receive buffer to a properly-aligned bufer.

It could be argued here that ng_eiface is the place in the netgraph code that needs to ensure that the IP portion of the packet is 32-bit aligned. But the only way to do that would be to have ng_eiface handle packets that come in on its ng hook by allocating a new mbuf and copy-aligning the data to it.

I implemented it this way so that the alignment could be done as the packet is originally received without needing to re-copy every packet. Other netgraph ingress points, such as ng_socket, could also support the ether_align option like I added to ng_device, and indeed if someone needs to inject packets at those points on non-x86 architecture, that code would have to be added to those nodes. But that's the beyond the scope of this change.

After a lot of analysis I concluded that this change -- receiving the data directly into a pre-aligned buffer when necessary -- was most in line with the design goals of netgraph, especially the part about avoiding the cost of re-copying data over and over as it moves between hooks.

[donner]

If I understand correctly, the requirement is to align the IP header unconditional.
So there is no need for an extra message, but all other nodes might be urged to add this, too.

[• ian]

In D32905#743481, @donner wrote:

If I understand correctly, the requirement is to align the IP header unconditional.
So there is no need for an extra message, but all other nodes might be urged to add this, too.

The requirement is only that an ETHERNET packet which still contains the 6-byte ethernet header on it must start the ethernet header at a two-byte offset to force the IP header to be aligned.

That means the alignment must happen if the downstream node is ng_eiface, and must NOT happen for any other type of downstream node.

The ng_device node can't know ahead of time whether its output will be connected to the input hook of ng_eiface versus some other type of downstream node. Only the entity wiring the nodes together from the outside knows that, so only that outside entity can say whether the alignment is needed or not, so it's controlled via an option that can be set from the outside.

[adrian]

I think it's fine. This only /really/ works with non-VLAN packets (ask me how I know due to MIPS, sigh) as the VLAN header ends up making the ethernet header size 2 bytes larger. It brings it inline with the rest of the system behaviour in this case.