On Tue, Oct 01, 2013 at 11:00:09AM +0200, Antonio Quartulli wrote: > On Wed, Sep 04, 2013 at 06:27:39PM +0200, Simon Wunderlich wrote: > > @@ -574,6 +562,13 @@ batadv_iv_ogm_can_aggregate(const struct > > batadv_ogm_packet *new_bat_ogm_packet, > > if (!primary_if) > > goto out; > > > > + /* packet is not leaving on the same interface. > > + * TODO: some other parts here could be reworked as the > > + * outgoing interface is specified now. > > + */ > > What does this TODO exactly mean? What could be reworked? >
There might be some more places where we can optimize/save a few cycles. The forw_packet was scheduled for all interfaces instead of individual interfaces before. Therefore direct link flag handling might be simplified. TBH I'm not sure and the aggregation code is not too easy to understand, so I preferred to not break it and put that TODO. :P > > @@ -1449,6 +1489,10 @@ batadv_iv_ogm_process_per_outif(const struct ethhdr > > *ethhdr, > > if_outgoing, tt_buff, dup_status); > > rcu_read_unlock(); > > > > + /* don't forward packet if no outgoing interface was specified */ > > + if (!if_outgoing) > > + goto out_neigh; > > can this really happen? or does it mean we have a BUG? > No, this is one of the main cases actually. batadv_iv_ogm_process_per_outif() is called: * first for the out interface NULL * then for all interfaces However the OGM is not forwarded for the NULL interface, only for the hard interfaces according to the respective rules. The if_outgoing = NULL case is only for locally generated packets. > > @@ -539,9 +542,12 @@ void batadv_send_outstanding_bat_ogm_packet(struct > > work_struct *work) > > > > /* we have to have at least one packet in the queue > > * to determine the queues wake up time unless we are > > - * shutting down > > + * shutting down. > > + * > > + * only re-schedule if this is the "original" copy. > > */ > > - if (forw_packet->own) > > + if (forw_packet->own && > > + forw_packet->if_incoming == forw_packet->if_outgoing) > > batadv_schedule_bat_ogm(forw_packet->if_incoming); > > What do you mean with "original" copy? Can an "own" packet have if_incoming != > if_outgoing? Yes, for the forw_packet of the primary interface we have multiple copies now for each outgoing interface (secondary interfaces). However we want to reschedule the OGM only once per period. This is why the check is neccesary. I'll rework the comment to make this more clear ... > > > Cheers, > > -- > Antonio Quartulli
signature.asc
Description: Digital signature
