On Fri, Sep 14, 2018 at 05:19:24PM +0200, Lorenzo Bianconi wrote:
> Add buffering support for IPv4 packets that will be processed
> by arp {} action when L2 address is not discovered yet since
> otherwise the packet will be substituted with an ARP frame and
> this will result in the lost of the first packet of the connection
>
> Signed-off-by: Lorenzo Bianconi <[email protected]>
Thanks for working on this! I believe that users will find it useful.
A buffer queue depth of 64 sounds big to me. Linux appears to default
to a depth of 3 (see unres_qlen in arp(7)).
A 30-second timeout seems long to me. After that long, the sender will
have retransmitted the packet, probably multiple times. I wasn't able
to figure out how long Linux buffers packets.
Do you think it makes sense to use the same timestamp for all of the
packets buffered for a single IP? I believe that, currently, no packets
will ever expire from a buffer unless either the buffer limit is
exceeded or no new packets have been queued for the timeout interval. I
think that would mean that there could be a packet buffered for a total
of 30 + 64 seconds.
It seems odd to me to convert the IP address to a string then use that
as the key. Why not the binary form of the IP address? I see that the
following patch adds IPv6 support, which could be one reason, but in
many places in OVS we use IPv6-mapped IPv4 addresses in places where
IPv4 and IPv6 are both possible (see in6_addr_mapped_ipv4() and
in6_addr_get_mapped_ipv4(), for example).
In the buffered_send_packets() prototype, I would use struct eth_addr
instead of an unsigned char *.
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
