[ovs-dev] [PATCH] match: Only print tp_src and tp_dst for TCP and UDP.

Justin Pettit jpettit at nicira.com
Fri Feb 22 08:58:59 UTC 2013


I'm not sure I understand your feedback.  Earlier in the code, "arp_[st]pa" and "arp_[st]ha" were printed, so without the commit you get something that looks like this:

-=-=-=-=-=-=-=-=-=-
2013-02-22T02:35:53Z|00244|dpif|DBG|system at ovs-system: execute 4 on packet arp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:01,dl_dst=ff:ff:ff:ff:ff:ff,arp_spa=192.168.0.1,arp_tpa=192.168.0.99,arp_op=1,arp_sha=50:54:00:00:00:01,arp_tha=00:00:00:00:00:00,tp_src=0,tp_dst=0
-=-=-=-=-=-=-=-=-=-

After this commit, the "tp_src" and "tp_dst" fields aren't there.  That function could use a cleanup, since it has redundant checks for the various protocol types, but I think this fixes a real problem.

Of course, let me know if I misunderstand.

--Justin


On Feb 21, 2013, at 10:20 PM, Ben Pfaff <blp at nicira.com> wrote:

> Based on the commit message it isn't clear to me why we shouldn't cover ARP here. (I don't have easy access to the full source code at the moment, so maybe this implies a source code revision and maybe a commit message revision and maybe I just misunderstand.)
> 
> On Feb 21, 2013 8:50 PM, "Justin Pettit" <jpettit at nicira.com> wrote:
> When printing a match, we would print "tp_src" and "tp_dst" if the
> packet wasn't ICMPv4 or ICMPv6.  Unfortunately, this doesn't cover ARP.
> This changes the check to only print those keys if the network protocol
> is TCP or UDP.
> 
> Signed-off-by: Justin Pettit <jpettit at nicira.com>
> ---
>  lib/match.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/match.c b/lib/match.c
> index f4b0a6c..2395fb4 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -1053,7 +1053,8 @@ match_format(const struct match *match, struct ds *s, unsigned int priority)
>                              &wc->masks.nd_target);
>          format_eth_masked(s, "nd_sll", f->arp_sha, wc->masks.arp_sha);
>          format_eth_masked(s, "nd_tll", f->arp_tha, wc->masks.arp_tha);
> -    } else {
> +    } else if (f->nw_proto == IPPROTO_TCP ||
> +               f->nw_proto == IPPROTO_UDP) {
>          format_be16_masked(s, "tp_src", f->tp_src, wc->masks.tp_src);
>          format_be16_masked(s, "tp_dst", f->tp_dst, wc->masks.tp_dst);
>      }
> --
> 1.7.5.4
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list