[ovs-dev] [PATCH v2 05/16] ofp-util: Abstract table miss configuration and fix related bugs.

Ben Pfaff blp at nicira.com
Mon Aug 11 18:31:11 UTC 2014


On Fri, Aug 08, 2014 at 11:32:40AM -0700, Jarno Rajahalme wrote:
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> Two small nits and a comment error (?) spotted below,
> 
>   Jarno
> 
> On Aug 7, 2014, at 4:13 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > +
> > +static struct mf_bitmap
> > +mf_bitmap_from_of10(ovs_be32 wc10)
> > +{
> > +    struct mf_bitmap fields = MF_BITMAP_INITIALIZER;
> > +    const struct ofp10_wc_map *p;
> > +
> > +    for (p = ofp10_wc_map; p < &ofp10_wc_map[ARRAY_SIZE(ofp10_wc_map)]; p++) {
> > +        if (wc10 & htonl(p->wc10)) {
> 
> Maybe ntohl(wc10) once outside the loop?

Done.

> > +static struct mf_bitmap
> > +mf_bitmap_from_of11(ovs_be32 wc11)
> > +{
> > +    struct mf_bitmap fields = MF_BITMAP_INITIALIZER;
> > +    const struct ofp11_wc_map *p;
> > +
> > +    for (p = ofp11_wc_map; p < &ofp11_wc_map[ARRAY_SIZE(ofp11_wc_map)]; p++) {
> > +        if (wc11 & htonl(p->wc11)) {
> 
> Ditto.

Done.

> > diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> > index 6c7559a..b9d2ae8 100644
> > --- a/lib/ofp-util.h
> > +++ b/lib/ofp-util.h
> > @@ -578,10 +578,38 @@ enum ofperr ofputil_decode_port_mod(const struct ofp_header *,
> > struct ofpbuf *ofputil_encode_port_mod(const struct ofputil_port_mod *,
> >                                        enum ofputil_protocol);
> > 
> > +/* Abstract version of OFPTC11_TABLE_MISS_*.
> > + *
> > + * OpenFlow 1.0 always sends packets that miss to the controller.
> > + *
> > + * OpenFlow 1.1 and 1.2 can configure table miss behavior via a "table-mod"
> > + * that specifies "send to controller", "miss", or "drop".
> > + *
> > + * OpenFlow 1.3 and later never sends packets that miss to the controller.
> > + */
> > +enum ofputil_table_miss {
> > +    /* Protocol-specific default behavior.  On OpenFlow 1.0 through 1.2
> > +     * connections, the packet is sent to the controller, and on OpenFlow 1.3
> > +     * and later connections, the packet is dropped.
> > +     *
> > +     * This is also used as a result of decoding OpenFlow 1.3+ "config" values
> > +     * in table-mods, to indicate that no table-miss was specified. */
> > +    OFPUTIL_TABLE_MISS_DEFAULT,    /* Protocol default behavior. */
> > +
> > +    /* These constants have the same meanings as those in OpenFlow with the
> > +     * same names. */
> > +    OFPUTIL_TABLE_MISS_CONTROLLER, /* Send to controller. */
> > +    OFPUTIL_TABLE_MISS_CONTINUE,   /* Go to next table, like OF1.0. */
> 
> The comment here does not seem to match the one above (?OpenFlow 1.0
> always sends packets that miss to the controller.?)

The OF1.0 situation is more nuanced than I documented here.  In an
OF1.0 switch, a packet that misses in table 0 goes on to table 1, and
so on until it misses in the last table, when it goes to the
controller.  (I claim that OVS complies with this on the basis that
OF1.0 provides no way to specify what table a flows should go into,
and OVS always puts OF1.0 flows into table 0, so that if you're using
OF1.0 with OVS you always see compliant behavior.)

Anyway, I clarified the comments:

/* Abstract version of OFPTC11_TABLE_MISS_*.
 *
 * OpenFlow 1.0 always sends packets that miss to the next flow table, or to
 * the controller if they miss in the last flow table.

...

    OFPUTIL_TABLE_MISS_CONTROLLER, /* Send to controller. */
    OFPUTIL_TABLE_MISS_CONTINUE,   /* Go to next table. */
    OFPUTIL_TABLE_MISS_DROP,       /* Drop the packet. */

I'll run the unit tests and apply this, then.



More information about the dev mailing list