[ovs-dev] [PATCH] Add table_id to NXM flow_removed messages.

[Ben Pfaff]

The things we sacrifice for compatibility!

I'll apply this to master and branch-1.10 in a minute.

On Fri, Mar 08, 2013 at 10:11:58AM -0800, Justin Pettit wrote:
> I'll hold my nose on the "table_id + 1", but otherwise looks good.  :-)
> 
> --Justin
> 
> 
> On Mar 6, 2013, at 9:13 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > Feature #15466.
> > Requested-by: Ronghua Zhang <rzhang at vmware.com>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > NEWS                          |    2 ++
> > include/openflow/nicira-ext.h |   10 ++++++++--
> > lib/ofp-util.c                |    3 ++-
> > tests/ofp-print.at            |    4 ++--
> > 4 files changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/NEWS b/NEWS
> > index 7fb0932..a33c14a 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -9,6 +9,8 @@ post-v1.10.0
> >     - OpenFlow:
> >       * The "dec_mpls_ttl" and "set_mpls_ttl" actions from OpenFlow
> >         1.1 and later are now implemented.
> > +      * The NXM flow_removed message now reports the OpenFlow table ID
> > +        from which the flow was removed.
> > 
> > 
> > v1.10.0 - xx xxx xxxx
> > diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> > index cdb1952..a1e5b58 100644
> > --- a/include/openflow/nicira-ext.h
> > +++ b/include/openflow/nicira-ext.h
> > @@ -1770,12 +1770,18 @@ struct nx_flow_mod {
> > };
> > OFP_ASSERT(sizeof(struct nx_flow_mod) == 32);
> > 
> > -/* NXT_FLOW_REMOVED (analogous to OFPT_FLOW_REMOVED). */
> > +/* NXT_FLOW_REMOVED (analogous to OFPT_FLOW_REMOVED).
> > + *
> > + * 'table_id' is present only in Open vSwitch 1.11 and later.  In earlier
> > + * versions of Open vSwitch, this is a padding byte that is always zeroed.
> > + * Therefore, a 'table_id' value of 0 indicates that the table ID is not known,
> > + * and other values may be interpreted as one more than the flow's former table
> > + * ID. */
> > struct nx_flow_removed {
> >     ovs_be64 cookie;          /* Opaque controller-issued identifier. */
> >     ovs_be16 priority;        /* Priority level of flow entry. */
> >     uint8_t reason;           /* One of OFPRR_*. */
> > -    uint8_t pad[1];           /* Align to 32-bits. */
> > +    uint8_t table_id;         /* Flow's former table ID, plus one. */
> >     ovs_be32 duration_sec;    /* Time flow was alive in seconds. */
> >     ovs_be32 duration_nsec;   /* Time flow was alive in nanoseconds beyond
> >                                  duration_sec. */
> > diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> > index dd6eed8..06e149a 100644
> > --- a/lib/ofp-util.c
> > +++ b/lib/ofp-util.c
> > @@ -2345,7 +2345,7 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed *fr,
> >         fr->priority = ntohs(nfr->priority);
> >         fr->cookie = nfr->cookie;
> >         fr->reason = nfr->reason;
> > -        fr->table_id = 255;
> > +        fr->table_id = nfr->table_id ? nfr->table_id - 1 : 255;
> >         fr->duration_sec = ntohl(nfr->duration_sec);
> >         fr->duration_nsec = ntohl(nfr->duration_nsec);
> >         fr->idle_timeout = ntohs(nfr->idle_timeout);
> > @@ -2424,6 +2424,7 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr,
> >         nfr->cookie = fr->cookie;
> >         nfr->priority = htons(fr->priority);
> >         nfr->reason = fr->reason;
> > +        nfr->table_id = fr->table_id + 1;
> >         nfr->duration_sec = htonl(fr->duration_sec);
> >         nfr->duration_nsec = htonl(fr->duration_nsec);
> >         nfr->idle_timeout = htons(fr->idle_timeout);
> > diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> > index 2939125..35f599c 100644
> > --- a/tests/ofp-print.at
> > +++ b/tests/ofp-print.at
> > @@ -1699,7 +1699,7 @@ AT_SETUP([NXT_FLOW_REMOVED])
> > AT_KEYWORDS([ofp-print])
> > AT_CHECK([ovs-ofctl ofp-print "\
> > 01 04 00 78 00 00 00 00 00 00 23 20 00 00 00 0e \
> > -00 00 00 00 00 00 00 00 ff ff 00 00 00 00 00 06 \
> > +00 00 00 00 00 00 00 00 ff ff 00 02 00 00 00 06 \
> > 01 6e 36 00 00 05 00 3c 00 00 00 00 00 00 00 01 \
> > 00 00 00 00 00 00 00 3c 00 00 00 02 00 03 00 00 \
> > 02 06 50 54 00 00 00 06 00 00 04 06 50 54 00 00 \
> > @@ -1707,7 +1707,7 @@ AT_CHECK([ovs-ofctl ofp-print "\
> > 1e 02 00 02 00 00 20 04 c0 a8 00 01 00 00 22 04 \
> > c0 a8 00 02 00 00 00 00 \
> > "], [0], [dnl
> > -NXT_FLOW_REMOVED (xid=0x0): priority=65535,arp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,arp_spa=192.168.0.1,arp_tpa=192.168.0.2,arp_op=2 reason=idle duration6.024s idle5 pkts1 bytes60
> > +NXT_FLOW_REMOVED (xid=0x0): priority=65535,arp,in_port=3,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:06,arp_spa=192.168.0.1,arp_tpa=192.168.0.2,arp_op=2 reason=idle table_id=1 duration6.024s idle5 pkts1 bytes60
> > ])
> > AT_CLEANUP
> > 
> > -- 
> > 1.7.10.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>