Re: [B.A.T.M.A.N.] [PATCH] batctl: Validate translated mac addresses

Antonio Quartulli
2018-04-14T08:11:28Z

[-- Attachment #1.1: Type: text/plain, Size: 2383 bytes --]

On 14/04/18 15:10, Sven Eckelmann wrote:
> On Samstag, 14. April 2018 04:34:42 CEST Antonio Quartulli wrote:
>>
>> On 14/04/18 02:16, Sven Eckelmann wrote:
> [...]
>> We already handle the case of multiple originators handling the same MAC
>> address, no? In that case I think we pick the "best" originator.
>
> Yes, but this doesn't make a lot of sense for multicast and zero mac
> addresses. The translate layer of batctl is usually used to ping/traceroute to
> some originator. But multicast and zero mac addresses don't represent a
> "client" which can be used to identify some originator. So it doesn't seem to
> make sense to allow them here.

Right.

>
> Or even without the ping/traceroute stuff, the concept of calling `batctl
> translate` should give you an answer which you can understand. So it should
> tell you that batman-adv is very likely to transmit a unicast packet with this
> destination address to this originator. But this cannot work for multicast
> destination addresses because multiple answer should be given here - which is
> out of scope for this command. Which reminds me that I should propose a second
> patch which checks whether the input for translate_mac is "valid" before
> trying to translate it.
>
>> This case sounds more like a validity check because "a zero MAC should
>> not be in the translation table", or am I wrong?
>
> Partially, yes. I personally don't care (at the moment) whether there is a
> zero mac address in the translation table. The current translation table code
> (batadv_tt_local_add) doesn't check whether there is a zero mac address
> (is_zero_ether_addr). But Linus had some ideas when zero mac addresses can be
> useful - maybe he tell us whether it makes sense/problems to have them in the
> translation table.

Yeah, I agree that zero "may appear" in the table and therefore we have
to "check" what we are getting back and whether it makes sense for us in
this context.

Actually my comment was not about changing your approach, but just
making it more explicit in the commit and in the error message.

An error message like like "returned invalid all-zero mac address" (or
"multicast address") might help to distinguish similar "ambiguities" in
the future. No?

Cheers,

>
> Kind regards,
> Sven