On Wed, Apr 13, 2016 at 01:11:12PM +0200, Simon Wunderlich wrote:

Hi Andrew,

On Tuesday 12 April 2016 22:42:59 Andrew Lunn wrote:

...

Hi Folks

Does anybody remember the history for the follow and can explain why the code is as it is?

The soft interface transmit function, batadv_interface_tx() calls batadv_skb_set_priority(skb, 0) to set the skb->priority based on the TOS bits or 801.p.

If the packet needs to be fragmented because of MTU issues, batadv_frag_create() is used to create the fragments. It overwrites the skb->priority in the original skb to TC_PRIO_CONTROL, and leaves the fragment skb with the default priority.

This seems a bit odd to me. I would of expected the priority to of been copied from the original into the fragment.

Hi Simon

I think this part could be improved. Right now, if I remember correctly, we set TC_PRIO_CONTROL by default and set another priority if we can parse the header (batadv_skb_set_priority()).

Yes, that is what i thought the intention was. The implementation is a bit different.

There are two cases:

1.) On the original sender, both fragments could adopt the priority as you suggest. The code probably doesn't take care of that yet, so that could be fixed.

I will cook up a patch for this.

2.) On routers on the way, the priority could only be set based on the first fragment, since the second fragment will not have a valid header to parse. And unless we remember the priority from the first fragment, we have no way to know to which priority we should set the second fragment.

I don't think remembering works. It looks like it fragments from the tail towards the head. So we are not going to receive the IP header until we get the last fragment.

I believe case 1 can be fixed easily, for case 2 I don't have an idea right now. :)

There is one reasonable option i can think of. batadv_skb_set_priority() extracts three bits of priority information, either from the TOS bits, or the 801.q header.

The fragment header is:

struct batadv_frag_packet { u8 packet_type; u8 version; /* batman version field */ u8 ttl; #if defined(__BIG_ENDIAN_BITFIELD) u8 no:4; u8 reserved:4; #elif defined(__LITTLE_ENDIAN_BITFIELD) u8 reserved:4; u8 no:4; #else #error "unknown bitfield endianness" #endif u8 dest[ETH_ALEN]; u8 orig[ETH_ALEN]; __be16 seqno; __be16 total_size; };

Place the priority information into 3 of the 4 reserved bits. The receiver can then set the skb->priority of the fragment before passing it to the hard interface.

Andrew