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@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 <[email protected]> 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" <[email protected]> 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 <[email protected]> > --- > 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 > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
