Re: [B.A.T.M.A.N.] [PATCH] batman-adv: change VID semantic in the BLA code

2013-04-07 Thread Simon Wunderlich
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 anto...@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?

  
  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 s...@hrz.tu-chemnitz.de
  Signed-off-by: Antonio Quartulli anto...@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.

   /* 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-Protocol

If there is a VID, it will send the frame in the respective VLAN.

The patch looks fine in generally, I have no objections.

Cheers,
Simon


signature.asc
Description: Digital signature


Re: [B.A.T.M.A.N.] [PATCH] batman-adv: change VID semantic in the BLA code

2013-04-03 Thread Antonio Quartulli
On Wed, Apr 03, 2013 at 11:17:14AM +0200, Antonio Quartulli wrote:
 From: Antonio Quartulli anto...@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.
 
 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 s...@hrz.tu-chemnitz.de
 Signed-off-by: Antonio Quartulli anto...@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),
 +};
 +
  /* 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.

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto Che Guevara


pgp2T2hhnUBdc.pgp
Description: PGP signature