Re: [B.A.T.M.A.N.] [PATCHv2 1/7] batman-adv: Return reason for failure in batadv_check_unicast_packet()

Antonio Quartulli
2012-12-15T12:39:35Z

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

On Sat, Dec 15, 2012 at 04:10:17PM +0800, Marek Lindner wrote:
> On Friday, December 14, 2012 14:32:46 Martin Hundebøll wrote:
> > static int batadv_check_unicast_packet(struct sk_buff *skb, int hdr_size)
> > {
> > struct ethhdr *ethhdr;
> > @@ -564,15 +574,15 @@ static int batadv_check_unicast_packet(struct sk_buff
> > *skb, int hdr_size)
> >
> > /* packet with unicast indication but broadcast recipient */
> > if (is_broadcast_ether_addr(ethhdr->h_dest))
> > - return -1;
> > + return -2;
> >
> > /* packet with broadcast sender address */
> > if (is_broadcast_ether_addr(ethhdr->h_source))
> > - return -1;
> > + return -3;
> >
> > /* not for me */
> > if (!batadv_is_my_mac(ethhdr->h_dest))
> > - return -1;
> > + return -4;
> >
> > return 0;
> > }
>
> Returning meaningless numbers isn't the way to go. Either we make our own
> defines or we re-use our E***** friends.

If I am not mistaken you only use "-4" in your network coding code. Maybe you
want to reduce this distinction to two cases only?

e.g.
* -EINVAL : incorrect header or bad fields (-1, -2, -3)..the packet is not valid
in the end
* something else for your case..what about -EREMOTE ?

>
> By the way, I suggest to send this patch separately. It increases the chance
> of getting merged faster and reduces your patchset.

I agree on this. It would simplify your daily rebasing task. And when/if you do
so, remember to change the commit message: networking does not exist yet.

Cheers,

--
Antonio Quartulli

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

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