VLAN driver question

VLAN driver question

[Jim Baxter]

I have a network card that has a single flag to indicate if a received
packet contains a vlan packet.

Can I use this to accelerate the kernels handling of the packet with
something like the following?

/* Handle received VLAN packets */
if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && ebdp &&
					(ebdp->cbd_esc &
					BD_ENET_RX_VLAN)) {
	/* Push and remove the vlan tag */
	struct vlan_hdr *vlan_header;
	u16 vlan_tag;
	vlan_header = (struct vlan_hdr *) skb->data;
	vlan_tag = ntohs(vlan_header->h_vlan_TCI);
	__vlan_hwaccel_put_tag(skb, vlan_tag);
	skb->len -= VLAN_HLEN;
	skb->data += VLAN_HLEN;
	vlan_set_encap_proto(skb, vlan_header);
}

napi_gro_receive(&fep->napi, skb);



Also I cannot find any documentation stating what the difference is
between NETIF_F_HW_VLAN_CTAG_RX and NETIF_F_HW_VLAN_CTAG_FILTER, can
anyone define it?
The FILTER word on NETIF_F_HW_VLAN_CTAG_FILTER suggests it may be
applicable to received packets.

Thank you for your help and comments.
Jim Baxter

Re: VLAN driver question

[Ben Hutchings]

On Mon, 2013-06-17 at 20:12 +0100, Jim Baxter wrote:
> I have a network card that has a single flag to indicate if a received
> packet contains a vlan packet.
> 
> Can I use this to accelerate the kernels handling of the packet with
> something like the following?
>
> /* Handle received VLAN packets */
> if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && ebdp &&
> 					(ebdp->cbd_esc &
> 					BD_ENET_RX_VLAN)) {
> 	/* Push and remove the vlan tag */
> 	struct vlan_hdr *vlan_header;
> 	u16 vlan_tag;
> 	vlan_header = (struct vlan_hdr *) skb->data;
> 	vlan_tag = ntohs(vlan_header->h_vlan_TCI);
> 	__vlan_hwaccel_put_tag(skb, vlan_tag);
> 	skb->len -= VLAN_HLEN;
> 	skb->data += VLAN_HLEN;
> 	vlan_set_encap_proto(skb, vlan_header);

You have to move the Ethernet addresses forward as well.  Imagine this
device is being bridged - the Ethernet header won't be regenerated, so
it has to be immediately before the network header.

> }
> 
> napi_gro_receive(&fep->napi, skb);

I've been meaning to add a core function to do
pull-out-VLAN-tag-and-GRO, although I'm more interested in
napi_gro_frags().  Perhaps you could add the generic wrapper for
napi_gro_receive()?

> Also I cannot find any documentation stating what the difference is
> between NETIF_F_HW_VLAN_CTAG_RX and NETIF_F_HW_VLAN_CTAG_FILTER, can
> anyone define it?
> The FILTER word on NETIF_F_HW_VLAN_CTAG_FILTER suggests it may be
> applicable to received packets.

NETIF_F_HW_VLAN_CTAG_RX means the device will extract VLAN tags so they
aren't received inline.

NETIF_F_HW_VLAN_CTAG_FILTER means the device can filter by VID.  The
driver's ndo_vlan_rx_{add,kill}_vid operations will be called to update
the filter.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: VLAN driver question

[Jim Baxter]

Thank you Ben, that is very helpful.

On 18/06/13 15:41, Ben Hutchings wrote:
> On Mon, 2013-06-17 at 20:12 +0100, Jim Baxter wrote:
>> I have a network card that has a single flag to indicate if a received
>> packet contains a vlan packet.
>>
>> Can I use this to accelerate the kernels handling of the packet with
>> something like the following?
>>
>> /* Handle received VLAN packets */
>> if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && ebdp &&
>> 					(ebdp->cbd_esc &
>> 					BD_ENET_RX_VLAN)) {
>> 	/* Push and remove the vlan tag */
>> 	struct vlan_hdr *vlan_header;
>> 	u16 vlan_tag;
>> 	vlan_header = (struct vlan_hdr *) skb->data;
>> 	vlan_tag = ntohs(vlan_header->h_vlan_TCI);
>> 	__vlan_hwaccel_put_tag(skb, vlan_tag);
>> 	skb->len -= VLAN_HLEN;
>> 	skb->data += VLAN_HLEN;
>> 	vlan_set_encap_proto(skb, vlan_header);
> 
> You have to move the Ethernet addresses forward as well.  Imagine this
> device is being bridged - the Ethernet header won't be regenerated, so
> it has to be immediately before the network header.
> 
Unless I have misunderstood, the Ethernet header is already copied
across to the skb by the Driver I am editing with the lines:
	skb_reserve(skb, NET_IP_ALIGN);
	skb_put(skb, pkt_len - 4);	/* Make room */
	skb_copy_to_linear_data(skb, data, pkt_len - 4);
	skb->protocol = eth_type_trans(skb, ndev);


Thinking about it though I should actually use the flag from the driver
to create an skb of the size (pkt_len - 4 - VLAN_HLEN) and extract the
vlan header from data so it is not copied to the skb, does this sound
sensible?

>> }
>>
>> napi_gro_receive(&fep->napi, skb);
> 
> I've been meaning to add a core function to do
> pull-out-VLAN-tag-and-GRO, although I'm more interested in
> napi_gro_frags().  Perhaps you could add the generic wrapper for
> napi_gro_receive()?
>
>> Also I cannot find any documentation stating what the difference is
>> between NETIF_F_HW_VLAN_CTAG_RX and NETIF_F_HW_VLAN_CTAG_FILTER, can
>> anyone define it?
>> The FILTER word on NETIF_F_HW_VLAN_CTAG_FILTER suggests it may be
>> applicable to received packets.
> 
> NETIF_F_HW_VLAN_CTAG_RX means the device will extract VLAN tags so they
> aren't received inline.
> 
> NETIF_F_HW_VLAN_CTAG_FILTER means the device can filter by VID.  The
> driver's ndo_vlan_rx_{add,kill}_vid operations will be called to update
> the filter.
> 
> Ben.
> 
Jim

Re: VLAN driver question

[Ben Hutchings]

On Wed, 2013-06-19 at 16:57 +0100, Jim Baxter wrote:
> Thank you Ben, that is very helpful.
> 
> On 18/06/13 15:41, Ben Hutchings wrote:
> > On Mon, 2013-06-17 at 20:12 +0100, Jim Baxter wrote:
> >> I have a network card that has a single flag to indicate if a received
> >> packet contains a vlan packet.
> >>
> >> Can I use this to accelerate the kernels handling of the packet with
> >> something like the following?
> >>
> >> /* Handle received VLAN packets */
> >> if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && ebdp &&
> >> 					(ebdp->cbd_esc &
> >> 					BD_ENET_RX_VLAN)) {
> >> 	/* Push and remove the vlan tag */
> >> 	struct vlan_hdr *vlan_header;
> >> 	u16 vlan_tag;
> >> 	vlan_header = (struct vlan_hdr *) skb->data;
> >> 	vlan_tag = ntohs(vlan_header->h_vlan_TCI);
> >> 	__vlan_hwaccel_put_tag(skb, vlan_tag);
> >> 	skb->len -= VLAN_HLEN;
> >> 	skb->data += VLAN_HLEN;
> >> 	vlan_set_encap_proto(skb, vlan_header);
> > 
> > You have to move the Ethernet addresses forward as well.  Imagine this
> > device is being bridged - the Ethernet header won't be regenerated, so
> > it has to be immediately before the network header.
> > 
> Unless I have misunderstood, the Ethernet header is already copied
> across to the skb by the Driver I am editing with the lines:
> 	skb_reserve(skb, NET_IP_ALIGN);
> 	skb_put(skb, pkt_len - 4);	/* Make room */
> 	skb_copy_to_linear_data(skb, data, pkt_len - 4);
> 	skb->protocol = eth_type_trans(skb, ndev);

Right, but the copy and ether_type_trans() result in an skb like this:

-14 -8  -2   0   2    4
|dst|src|8100|vid|type|payload...|
             ^

After extracting the VLAN tag the skb looks like:

-18 -12 -6   -4  -2   0
|dst|src|8100|vid|type|payload...|
                      ^

If this packet is bridged, the 14 bytes before skb->data will be treated
as the Ethernet header:

0 2   8    10  12   14
|x|src|8100|vid|type|payload...|
^

Then with the VLAN tag inserted;

0 2   8    10  12   14  16   18
|x|src|8100|vid|8100|vid|type|payload...|

> Thinking about it though I should actually use the flag from the driver
> to create an skb of the size (pkt_len - 4 - VLAN_HLEN) and extract the
> vlan header from data so it is not copied to the skb, does this sound
> sensible?
[...]

That would also work.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: VLAN driver question

[Jim Baxter]

On 19/06/13 17:33, Ben Hutchings wrote:
> On Wed, 2013-06-19 at 16:57 +0100, Jim Baxter wrote:
>> Thank you Ben, that is very helpful.
>>
>> On 18/06/13 15:41, Ben Hutchings wrote:
>>> On Mon, 2013-06-17 at 20:12 +0100, Jim Baxter wrote:
>>>> I have a network card that has a single flag to indicate if a received
>>>> packet contains a vlan packet.
>>>>
>>>> Can I use this to accelerate the kernels handling of the packet with
>>>> something like the following?
>>>>
>>>> /* Handle received VLAN packets */
>>>> if ((ndev->features & NETIF_F_HW_VLAN_CTAG_RX) && ebdp &&
>>>> 					(ebdp->cbd_esc &
>>>> 					BD_ENET_RX_VLAN)) {
>>>> 	/* Push and remove the vlan tag */
>>>> 	struct vlan_hdr *vlan_header;
>>>> 	u16 vlan_tag;
>>>> 	vlan_header = (struct vlan_hdr *) skb->data;
>>>> 	vlan_tag = ntohs(vlan_header->h_vlan_TCI);
>>>> 	__vlan_hwaccel_put_tag(skb, vlan_tag);
>>>> 	skb->len -= VLAN_HLEN;
>>>> 	skb->data += VLAN_HLEN;
>>>> 	vlan_set_encap_proto(skb, vlan_header);
>>>
>>> You have to move the Ethernet addresses forward as well.  Imagine this
>>> device is being bridged - the Ethernet header won't be regenerated, so
>>> it has to be immediately before the network header.
>>>
>> Unless I have misunderstood, the Ethernet header is already copied
>> across to the skb by the Driver I am editing with the lines:
>> 	skb_reserve(skb, NET_IP_ALIGN);
>> 	skb_put(skb, pkt_len - 4);	/* Make room */
>> 	skb_copy_to_linear_data(skb, data, pkt_len - 4);
>> 	skb->protocol = eth_type_trans(skb, ndev);
> 
> Right, but the copy and ether_type_trans() result in an skb like this:
> 
> -14 -8  -2   0   2    4
> |dst|src|8100|vid|type|payload...|
>              ^
> 
> After extracting the VLAN tag the skb looks like:
> 
> -18 -12 -6   -4  -2   0
> |dst|src|8100|vid|type|payload...|
>                       ^
> 
> If this packet is bridged, the 14 bytes before skb->data will be treated
> as the Ethernet header:
> 
> 0 2   8    10  12   14
> |x|src|8100|vid|type|payload...|
> ^
> 
> Then with the VLAN tag inserted;
> 
> 0 2   8    10  12   14  16   18
> |x|src|8100|vid|8100|vid|type|payload...|
> 
>> Thinking about it though I should actually use the flag from the driver
>> to create an skb of the size (pkt_len - 4 - VLAN_HLEN) and extract the
>> vlan header from data so it is not copied to the skb, does this sound
>> sensible?
> [...]
> 
> That would also work.
> 
> Ben.
> 

Thank you for that information, I have decided to extract the data
before copying the data into the skb->data structure.

This is the put of the packet showing my removal of the data correctly,
however the network is not receiving any data on the VLAN.

- data buffer from the Network card
ff ff ff ff ff ff 00 26 b9 d4 19 01 81 00 00 0a 08 06 00 01 08 00 06 04
00 01 00 26 b9 d4

- skb->data after I have only copied data to it and removed the vlan 4
octet header using skb_copy_to_linear_data and
skb_copy_to_linear_data_offset.
ff ff ff ff ff ff 00 26 b9 d4 19 01 08 06 00 01 08 00 06 04 00 01 00 26
b9 d4 19 01 c0 a8

- skb-data after eth_type_trans()
00 01 08 00 06 04 00 01 00 26 b9 d4 19 01 c0 a8 01 67 00 00 00 00 00 00
c0 a8 01 64 00 00

After this point I send the packet into the network stack with:
	__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);

	napi_gro_receive(&fep->napi, skb);

Am I missing a kernel call or doing something wrong to cause the packet
to be rejected in the kernel?

Thank you for any help,
Jim

Re: VLAN driver question

[Ben Hutchings]

On Thu, 2013-06-20 at 20:05 +0100, Jim Baxter wrote:
[...]
> Thank you for that information, I have decided to extract the data
> before copying the data into the skb->data structure.
> 
> This is the put of the packet showing my removal of the data correctly,
> however the network is not receiving any data on the VLAN.
> 
> - data buffer from the Network card
> ff ff ff ff ff ff 00 26 b9 d4 19 01 81 00 00 0a 08 06 00 01 08 00 06 04
> 00 01 00 26 b9 d4
> 
> - skb->data after I have only copied data to it and removed the vlan 4
> octet header using skb_copy_to_linear_data and
> skb_copy_to_linear_data_offset.
> ff ff ff ff ff ff 00 26 b9 d4 19 01 08 06 00 01 08 00 06 04 00 01 00 26
> b9 d4 19 01 c0 a8
> 
> - skb-data after eth_type_trans()
> 00 01 08 00 06 04 00 01 00 26 b9 d4 19 01 c0 a8 01 67 00 00 00 00 00 00
> c0 a8 01 64 00 00
> 
> After this point I send the packet into the network stack with:
> 	__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
> 
> 	napi_gro_receive(&fep->napi, skb);
> 
> Am I missing a kernel call or doing something wrong to cause the packet
> to be rejected in the kernel?

That looks about right to me, but perhaps vlan_tag has the wrong byte
order?  __vlan_hwaccel_put_tag() expects it in host byte order.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: VLAN driver question

[Jim Baxter]

On 21/06/13 16:42, Ben Hutchings wrote:
> On Thu, 2013-06-20 at 20:05 +0100, Jim Baxter wrote:
> [...]
>> Thank you for that information, I have decided to extract the data
>> before copying the data into the skb->data structure.
>>
>> This is the put of the packet showing my removal of the data correctly,
>> however the network is not receiving any data on the VLAN.
>>
>> - data buffer from the Network card
>> ff ff ff ff ff ff 00 26 b9 d4 19 01 81 00 00 0a 08 06 00 01 08 00 06 04
>> 00 01 00 26 b9 d4
>>
>> - skb->data after I have only copied data to it and removed the vlan 4
>> octet header using skb_copy_to_linear_data and
>> skb_copy_to_linear_data_offset.
>> ff ff ff ff ff ff 00 26 b9 d4 19 01 08 06 00 01 08 00 06 04 00 01 00 26
>> b9 d4 19 01 c0 a8
>>
>> - skb-data after eth_type_trans()
>> 00 01 08 00 06 04 00 01 00 26 b9 d4 19 01 c0 a8 01 67 00 00 00 00 00 00
>> c0 a8 01 64 00 00
>>
>> After this point I send the packet into the network stack with:
>> 	__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
>>
>> 	napi_gro_receive(&fep->napi, skb);
>>
>> Am I missing a kernel call or doing something wrong to cause the packet
>> to be rejected in the kernel?
> 
> That looks about right to me, but perhaps vlan_tag has the wrong byte
> order?  __vlan_hwaccel_put_tag() expects it in host byte order.
> 
> Ben.
> 

Hi, Thank you, I have got it working. The problem was that when I had
changed the code to remove the VLAN before putting the data into the
skb, I was not moving the pointer past the TPID.

It now all works with ping, ssh and scp'ing small files but when I scp a
large file it stalls. I suspect it is because it is not processing
packets that are at the MTU of 1518 bytes (1522 bytes with the VLAN
header) in size.

Jim