Re: [B.A.T.M.A.N.] [RFCv2 5/6] batman-adv: consider outgoing interface in OGM sending

Simon Wunderlich
2013-10-03T22:11:52Z

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

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]