On Wednesday, August 5, 2015, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
>
> > On Aug 3, 2015, at 6:04 PM, Jesse Gross <je...@nicira.com <javascript:;>>
> wrote:
> > On Mon, Aug 3, 2015 at 3:45 PM, Jarno Rajahalme <jrajaha...@nicira.com
> <javascript:;>> wrote:
> >>> static inline uint32_t
> >>> diff --git a/lib/flow.c b/lib/flow.c
> >>> index 352e9b8..d3d25e4 100644
> >>> --- a/lib/flow.c
> >>> +++ b/lib/flow.c
> >>> @@ -462,9 +462,22 @@ miniflow_extract(struct dp_packet *packet, struct
> miniflow *dst)
> >>> miniflow_push_words(mf, tunnel, &md->tunnel,
> >>> offsetof(struct flow_tnl, metadata) /
> >>> sizeof(uint64_t));
> >>> - if (md->tunnel.metadata.opt_map) {
> >>> - miniflow_push_words(mf, tunnel.metadata,
> &md->tunnel.metadata,
> >>> - sizeof md->tunnel.metadata /
> sizeof(uint64_t));
> >>> +
> >>> + if (!(md->tunnel.flags & FLOW_TNL_F_UDPIF)) {
> >>> + if (md->tunnel.metadata.present.map) {
> >>> + miniflow_push_words(mf, tunnel.metadata,
> &md->tunnel.metadata,
> >>> + sizeof md->tunnel.metadata /
> >>> + sizeof(uint64_t));
> >>> + }
> >>> + } else {
> >>> + if (md->tunnel.metadata.present.len) {
> >>> + miniflow_push_words(mf, tunnel.metadata.present,
> >>> + &md->tunnel.metadata.present, 1);
> >>> + miniflow_push_words(mf, tunnel.metadata.opts.gnv,
> >>> + md->tunnel.metadata.opts.gnv,
> >>> +
> DIV_ROUND_UP(md->tunnel.metadata.present.len,
> >>> + sizeof(uint64_t)));
> >>
> >> Wrong indentation here.
> >
> > Sorry, which part? It's not jumping out at me.
>
> sizeof is the argument of DIV_ROUND_UP, so it should be indented after the
> opening parenthesis of “DIV_ROUND_UP(“
Thanks, fixed now.
> >>> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
> >>> index bd95c8e..0f1724a 100644
> >>> --- a/tests/tunnel-push-pop.at
> >>> +++ b/tests/tunnel-push-pop.at
> >>> @@ -132,7 +132,7 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep
> 'port 5'], [0], [dnl
> >>> port 5: rx pkts=1, bytes=98, drop=0, errs=0, frame=0, over=0, crc=0
> >>> ])
> >>> AT_CHECK([ovs-appctl dpif/dump-flows int-br], [0], [dnl
> >>>
> -tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,ttl=64,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}),flags(-df-csum+key)),skb_mark(0),recirc_id(0),in_port(6081),eth_type(0x0800),ipv4(frag=no),
> packets:0, bytes:0, used:never, actions:drop
> >>>
> +tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,ttl=64,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),skb_mark(0),recirc_id(0),in_port(6081),eth_type(0x0800),ipv4(frag=no),
> packets:0, bytes:0, used:never, actions:drop
> >>
> >> Why this changed?
> >
> > This is actually a nice side benefit of the change. The input packet
> > actually has two options, the second of which was unknown. Previously,
> > the unknown one didn't show up in the installed flow since it was
> > skipped over by the userspace packet parsing code. Now that we always
> > extract the full set of options, the unknown option is visible to the
> > unit test and we can validation the processing associated with them.
>
> Ok, I think you mentioned this somehow in the commit message, but maybe
> you could add explicitly that also the unknown options are now shown by
> dump-flows?
>
Sure, I can some text to commit message. By the way, this is only new
behavior for the user space datapath - the kernel always behaved this way,
so it is nice to have parity.
I'll fix these up and apply the patch to master in a little bit.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
