On Tuesday 15 October 2013 21:20:23 Antonio Quartulli wrote:
Besides, if we make everything generic batadv_socket_packet->icmp_packet should not be hard-coded to batadv_icmp_packet_rr but the largest available ICMP packet type ?
or we dynamically allocate a buffer of size 'len'? In this way we don't need to change icmp_packet each time (hopefully not so many but still..) the "largest available ICMP packet type" changes.
Allocating a dynamic buffer does not solve the underlying issue. At some point we would want to check the packet size - either through sizeof(socket_packet-
icmp_packet) or a macro or whatever.
Take a look at batadv_max_header_len() for an idea how to address the matter.
Wouldn't it be better to dump unkown icmp types for us instead of copying everything to user space ?
dump == drop ? in that case I agree. Delivering unknown packets to batctl may also be dangerous.
Yes, that is what I was talking about.
Same is true for batadv_socket_write(). We should use the icmp header and not assume icmp echo.
do you mean using the ICMP header to understand what packet it is and then behave accordingly? Also in this case I agree with you :)
Yap.
Moving to a packet type based "check" was also part of the original idea of this generalisation (if I remember correctly), but not really needed when looking at avoiding further compatibility breakage (this should be the reason why this "feature" is not part of this patch).
I agree - it is not strictly needed but would fit since the rest of the patch works into the same direction.
Cheers, Marek