Hi Simon,
thanks for reviewing this patch.
On Sun, Apr 07, 2013 at 04:20:53PM +0200, Simon Wunderlich wrote:
Hey Antonio,
On Wed, Apr 03, 2013 at 11:23:59AM +0200, Antonio Quartulli wrote:
On Wed, Apr 03, 2013 at 11:17:14AM +0200, Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
In order to make batman-adv fully vlan aware later, the semantic used for variables storing the VLAN ID values has to be changed in order to be adapted to the new one which will be used batman-adv wide.
That is for the TT change later I guess? Was confused first, because batman-adv is already pretty VLAN aware ... maybe add this as a comment?
well, the fact that it works does not make it aware, no? :) Actually batman-adv does not take any action and does not recognise the event of creating a new VLAN on top of it.
But from your comment I realise that "being aware" is probably not the correct term. So I will try to improve it.
In particular, the VID has to be an "_unsigned_ short int" and its 4 MSB will be used as a flag bitfield, while the remaining 12 bits are used to store the real VID value
Cc: Simon Wunderlich siwu@hrz.tu-chemnitz.de Signed-off-by: Antonio Quartulli antonio@open-mesh.com
[cut..]
diff --git a/packet.h b/packet.h index a51ccfc..d5464f6 100644 --- a/packet.h +++ b/packet.h @@ -105,6 +105,14 @@ enum batadv_tt_client_flags { BATADV_TT_CLIENT_PENDING = BIT(10), };
+/**
- batadv_vlan_flags - flags for the four MSB of any vlan ID field
- @BATADV_VLAN_HAS_TAG: whether the field contains a valid vlan tag or not
- */
+enum batadv_vlan_flags {
- BATADV_VLAN_HAS_TAG = BIT(15),
+};
Please put this into main.h or somewhere else as long as it is not sent over the wire.
Ok. I put it there because I'll be sending it over the wire later. But better putting it into main.h for now.
/* claim frame types for the bridge loop avoidance */ enum batadv_bla_claimframe { BATADV_CLAIM_TYPE_CLAIM = 0x00, diff --git a/soft-interface.c b/soft-interface.c index 403b8c4..34597a2 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -154,7 +154,7 @@ static int batadv_interface_tx(struct sk_buff *skb, 0x00, 0x00}; unsigned int header_len = 0; int data_len = skb->len, ret;
- short vid __maybe_unused = -1;
- unsigned short vid __maybe_unused = BATADV_NO_FLAGS; bool do_bcast = false; uint32_t seqno; unsigned long brd_delay = 1;
@@ -303,7 +303,7 @@ void batadv_interface_rx(struct net_device *soft_iface, struct ethhdr *ethhdr; struct vlan_ethhdr *vhdr; struct batadv_header *batadv_header = (struct batadv_header *)skb->data;
- short vid __maybe_unused = -1;
- unsigned short vid __maybe_unused = BATADV_NO_FLAGS; __be16 ethertype = __constant_htons(ETH_P_BATMAN); bool is_bcast;
I just realised that this change is going to break compatibility because we change the menaing of the value that BLA sends over the wire.
We must postpone this change to the next (BIG) compat bump.
Actually no, it just uses the VID internally, so this is not a problem. See: http://www.open-mesh.org/projects/batman-adv/wiki/Bridge-loop-avoidance-Prot...
If there is a VID, it will send the frame in the respective VLAN.
My bad, I looked at the patch stats and I misinterpreted _my own_ change to packet.h :)
The patch looks fine in generally, I have no objections.
Thanks a lot! I'll send v2.
Cheers,