[ovs-dev] [PATCH] odp-util: Fix a bug in parse_flag().

Alex Wang alexw at nicira.com
Sat May 2 22:41:07 UTC 2015


This is really helpful~!

Post v2~

Thanks,
Alex Wang,

On Sat, May 2, 2015 at 2:29 PM, Ben Pfaff <blp at nicira.com> wrote:

> OK, I understand the question now.
>
> When + or - is used, it's supposed to work as documented for tcp_flags
> in ovs-ofctl(8), quoted below.  We don't really document the odp format
> anywhere; maybe we should.
>
>        tcp_flags=flags/mask
>        tcp_flags=[+flag...][-flag...]
>               Bitwise  match  on TCP flags.  The flags and mask are 16-bit
> num‐
>               bers written in decimal or in hexadecimal prefixed by  0x.
>  Each
>               1-bit  in  mask requires that the corresponding bit in flags
> must
>               match.  Each 0-bit in mask causes the  corresponding  bit
> to  be
>               ignored.
>
>               Alternatively, the flags can be specified by their symbolic
> names
>               (listed below), each preceded by either + for a flag that
> must be
>               set, or - for a flag that must be unset, without any other
> delim‐
>               iters between the flags.  Flags  not  mentioned  are
> wildcarded.
>               For example, tcp,tcp_flags=+syn-ack matches TCP SYNs that
> are not
>               ACKs.
>
>               TCP protocol currently defines 9 flag bits, and additional 3
> bits
>               are  reserved  (must be transmitted as zero), see RFCs 793,
> 3168,
>               and 3540.  The flag bits are, numbering from the  least
> signifi‐
>               cant bit:
>
>               0: fin No more data from sender.
>
>               1: syn Synchronize sequence numbers.
>
>               2: rst Reset the connection.
>
>               3: psh Push function.
>
>               4: ack Acknowledgement field significant.
>
>               5: urg Urgent pointer field significant.
>
>               6: ece ECN Echo.
>
>               7: cwr Congestion Windows Reduced.
>
>               8: ns  Nonce Sum.
>
>               9-11:  Reserved.
>
>               12-15: Not matchable, must be zero.
>
>
> On Sat, May 02, 2015 at 02:15:48PM -0700, Alex Wang wrote:
> > in the parse_flag() function in odp_util, it seems that we can mask the
> > flag.
> > Also, the parsing is different for masked flag and unmasked flag.
> > (e.g., the masked flag parsing uses + and - as delimiter, while the
> unmasked
> > flag parsing use , as delimiter)
> >
> > Thanks,
> > Alex Wang,
> >
> > On Sat, May 2, 2015 at 11:47 AM, Ben Pfaff <blp at nicira.com> wrote:
> >
> > > I don't understand the question yet, can you rephrase it?
> > >
> > > Thanks,
> > >
> > > Ben.
> > >
> > > On Sat, May 02, 2015 at 11:21:13AM -0700, Alex Wang wrote:
> > > > Thx for pointing me to the example~
> > > >
> > > > After check the tests, want to ask what is the difference between
> masked
> > > > flag and unmasked flag?  especially for tunnel,
> > > >
> > > > Thanks,
> > > > Alex Wang,
> > > >
> > > > On Sat, May 2, 2015 at 10:55 AM, Ben Pfaff <blp at nicira.com> wrote:
> > > >
> > > > > Can we just add an example to tests/odp.at?
> > > > >
> > > > > On Sat, May 02, 2015 at 10:51:30AM -0700, Alex Wang wrote:
> > > > > > Thx, I'll apply this first, trying to think of a good way to
> test it~
> > > > > >
> > > > > > On Sat, May 2, 2015 at 10:13 AM, Ben Pfaff <blp at nicira.com>
> wrote:
> > > > > >
> > > > > > > On Fri, May 01, 2015 at 08:58:57PM -0700, Alex Wang wrote:
> > > > > > > > This commit fixes a bug in the parse_flag() function which
> causes
> > > > > > > > failure of parsing tunnel flags like:
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > >
> tunnel(tun_id=0x0,src=1.2.3.4,dst=1.2.3.5,tos=0,ttl=64,flags(-df+csum+key))
> > > > > > > >
> > > > > > > > Reported-by: Jacob Cherkas <jcherkas at nicira.com>
> > > > > > > > Signed-off-by: Alex Wang <alexw at nicira.com>
> > > > > > >
> > > > > > > Fantastic, thank you--I noticed the same problem a few days ago
> > > testing
> > > > > > > OVN but hadn't had time to follow up yet.
> > > > > > >
> > > > > > > Can you add a test?
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Ben.
> > > > > > >
> > > > >
> > >
>



More information about the dev mailing list