Re: [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: move and restructure batadv_v_ogm_forward

[Simon Wunderlich <sw@simonwunderlich.de>]

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

On Wednesday 20 January 2016 22:34:25 Antonio Quartulli wrote:
> On 20/01/16 21:32, Simon Wunderlich wrote:
> >  static void batadv_v_ogm_forward(struct batadv_priv *bat_priv,
> >  
> >  				 const struct batadv_ogm2_packet *ogm_received,
> > 
> > -				 u32 throughput,
> > +				 struct batadv_neigh_node *neigh_node,
> > 
> >  				 struct batadv_hard_iface *if_incoming,
> >  				 struct batadv_hard_iface *if_outgoing)
> >  
> >  {
> > 
> > +	struct batadv_neigh_ifinfo *neigh_ifinfo = NULL;
> > +	struct batadv_orig_ifinfo *orig_ifinfo = NULL;
> > +	struct batadv_neigh_node *router = NULL;
> > 
> >  	struct batadv_ogm2_packet *ogm_forward;
> >  	unsigned char *skb_buff;
> >  	struct sk_buff *skb;
> >  	size_t packet_len;
> >  	u16 tvlv_len;
> > 
> > +	/* only forward for specific interfaces, not for the default one. */
> > +	if (if_outgoing != BATADV_IF_DEFAULT)
> > +		goto out;
> > +
> 
> personally I'd prefer the forward function to do what it says: forward
> the OGM. The checks whether or not we should do this should stay in
> ogm_process_per_outif().
> 

Hm, yeah we discussed to keep these changes in route_update, but actually 
those checks decide whether we forward or not (i.e. directly involving 
forwarding). Therefore I thought they would fit better in the forward function 
and also move the corresponding description [1] in the OGMv2 wiki page from 
route update to forward. We don't have a matching "general" description in the 
OGMv2 wiki page which corresponds to ogm_process_per_outif() after all.

Any other opinions?

[1] From the OGMv2 wiki page: If the OGMv2 has been received by the (now) 
selected router, the OGM is forwarded on the considered outgoing interface 
(except for the default interface). However, the OGMv2 is not forwarded if 
another OGMv2 has been forwarded with the same sequence number.
> > +	orig_ifinfo = batadv_orig_ifinfo_new(orig_node, if_outgoing);
> > +	if (!orig_ifinfo)
> > +		goto out;
> > +	/* acquire possibly updated router */
> > +	router = batadv_orig_router_get(orig_node, if_outgoing);
> > +
> > +	/* strict rule: forward packets coming from the best next hop only */
> > +	if (neigh_node != router)
> > +		goto out;
> > +
> 
> this is changing the behaviour.
> here now we get a router which potentially was elected during the
> previous update_route() call while processing this very OGM. We are
> still discussing if we want to do this or not, but this patch should be
> just a style change, while this is not.

No, this is already in the code which is merged into master - we already 
acquire the updated router (see bat_v_ogm.c:547, function 
batadv_v_ogm_route_update()).

Cheers,
    Simon

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