On Tue, Feb 13, 2018 at 03:43:51PM +0100, Lorenzo Bianconi wrote: > icmp4 action is used to replace the IPv4 packet been processed with > an ICMPv4 packet initialized based on incoming IPv4 one. > Ethernet and IPv4 fields not listed are not changed: > - ip.proto = 1 (ICMPv4) > - ip.frag = 0 (not a fragment) > - icmp4.type = 3 (destination unreachable) > - icmp4.code = 1 (host unreachable) > Prerequisite: ip4 > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
Thanks for the patch! It looks really good. I'm appending a few changes I suggest folding in: usually we write hex numbers preceded by 0x (which the # flag does), and because the two lines that get deleted aren't really needed. There's some inconsistency in naming between "icmp" and "icmp4". I guess I like icmp4 better because it seems likely that we'll eventually add an icmp6. Did you consider the tos value passed to packet_set_ipv4()? I'd be somewhat inclined to use ip_flow->nw_tos. The documentation says that IPv4 values not listed are not changed, so either we should update the documentation or the code regarding TTL and TOS. I'd add some parsing tests to the "ovn -- action parsing" test in tests/ovn.at. Thanks, Ben. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
