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
signature.asc
Description: This is a digitally signed message part.
