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

[Ben Pfaff]

OK. That makes sense. Thanks.
On Feb 22, 2013 2:59 AM, "Justin Pettit" <jpettit at nicira.com> wrote:

> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130222/2373c8d3/attachment-0003.html>