Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Fix broadcast packet CRC calculation

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 3476 bytes --]

On Wednesday 17 October 2012 15:50:50 Sven Eckelmann wrote:
> On Wednesday 17 October 2012 13:38:30 Linus Lüssing wrote:
> > So far the crc16 checksum for a batman-adv broadcast data packet, received
> > on a batman-adv hard interface, was calculated over zero bytes of its
> > content leading to many incoming broadcast data packets wrongly being
> > dropped.
> > 
> > This patch fixes this issue by calculating the crc16 over the actual,
> > complete broadcast payload.
> > 
> > The issue is a regrission introduced by
> > e5cf86d30a9b32feccb6b866a3f23b0e0483a3b0.
> > 
> > Signed-off-by: Linus Lüssing <linus.luessing@web.de>
> > ---
> > It led to about 60-80% broadcast packet loss to a direct neighbor
> > with a ping6 with a 1s interval in our scenario, but about no
> > packet loss with an interval smaller than 0.2s.
> > 
> > Also see: https://projects.universe-factory.net/issues/65 (German)
> > 
> >  bridge_loop_avoidance.c |    8 ++++----
> >  routing.c               |    8 +++++++-
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/bridge_loop_avoidance.c b/bridge_loop_avoidance.c
> > index 6705d35..e7b5777 100644
> > --- a/bridge_loop_avoidance.c
> > +++ b/bridge_loop_avoidance.c
> > @@ -1205,8 +1205,8 @@ int batadv_bla_init(struct batadv_priv *bat_priv)
> > 
> >  /**
> >  
> >   * batadv_bla_check_bcast_duplist
> >   * @bat_priv: the bat priv with all the soft interface information
> > 
> > - * @bcast_packet: originator mac address
> > - * @hdr_size: maximum length of the frame
> > + * @bcast_packet: encapsulated broadcast frame plus batman header
> > + * @bcast_packet_len: length of encapsulated broadcast frame plus batman
> > header *
> > 
> >   * check if it is on our broadcast list. Another gateway might
> >   * have sent the same packet because it is connected to the same
> >   backbone,
> > 
> > @@ -1219,14 +1219,14 @@ int batadv_bla_init(struct batadv_priv *bat_priv)
> > 
> >   */
> >  
> >  int batadv_bla_check_bcast_duplist(struct batadv_priv *bat_priv,
> >  
> >  				   struct batadv_bcast_packet *bcast_packet,
> > 
> > -				   int hdr_size)
> > +				   int bcast_packet_len)
> > 
> >  {
> >  
> >  	int i, length, curr;
> >  	uint8_t *content;
> >  	uint16_t crc;
> >  	struct batadv_bcast_duplist_entry *entry;
> > 
> > -	length = hdr_size - sizeof(*bcast_packet);
> > +	length = bcast_packet_len - sizeof(*bcast_packet);
> > 
> >  	content = (uint8_t *)bcast_packet;
> >  	content += sizeof(*bcast_packet);
> > 
> > diff --git a/routing.c b/routing.c
> > index bc2b88b..f861b7c 100644
> > --- a/routing.c
> > +++ b/routing.c
> > @@ -1136,8 +1136,14 @@ int batadv_recv_bcast_packet(struct sk_buff *skb,
> > 
> >  	spin_unlock_bh(&orig_node->bcast_seqno_lock);
> > 
> > +	/* keep skb linear for crc calculation */
> > +	if (skb_linearize(skb) < 0)
> > +		goto out;
> > +
> > +	bcast_packet = (struct batadv_bcast_packet *)skb->data;
> > +
> > 
> >  	/* check whether this has been sent by another originator before */
> > 
> > -	if (batadv_bla_check_bcast_duplist(bat_priv, bcast_packet, hdr_size))
> > +	if (batadv_bla_check_bcast_duplist(bat_priv, bcast_packet, skb->len))
> > 
> >  		goto out;
> >  	
> >  	/* rebroadcast packet */
> 
> Btw. why should renaming the hdr_size to bcast_packet_len help to get a
> different value than 0?

Ok, Marek just noticed the skb->len :D

> Kind regards,
> 	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]