[ovs-dev] [PATCH 5/8] ofproto: Match on IP ToS/DSCP bits (OpenFlow 1.0)

[Ben Pfaff]

On Fri, Jan 22, 2010 at 01:09:10AM -0800, Justin Pettit wrote:
> OpenFlow 1.0 adds support for matching on IP ToS/DSCP bits.
> 
> NOTE: OVS at this point is not wire-compatible with OpenFlow 1.0 until
> the final commit in this OpenFlow 1.0 set.

This patch wouldn't apply for me:

    Applying: ofproto: Match on IP ToS/DSCP bits (OpenFlow 1.0)
    /home/blp/nicira/openflow/.git/rebase-apply/patch:394: trailing whitespace.
    Matches IP ToS/DSCP field \fItos\fR, which is specified as a decimal 
    error: patch failed: datapath/datapath.c:859
    error: datapath/datapath.c: patch does not apply
    error: patch failed: include/openvswitch/datapath-protocol.h:163
    error: include/openvswitch/datapath-protocol.h: patch does not apply
    error: patch failed: lib/classifier.c:57
    error: lib/classifier.c: patch does not apply
    error: patch failed: lib/dpif-netdev.c:664
    error: lib/dpif-netdev.c: patch does not apply
    error: patch failed: lib/flow.c:276
    error: lib/flow.c: patch does not apply
    error: patch failed: tests/flowgen.pl:203
    error: tests/flowgen.pl: patch does not apply
    error: patch failed: tests/test-classifier.c:366
    error: tests/test-classifier.c: patch does not apply
    Patch failed at 0001 ofproto: Match on IP ToS/DSCP bits (OpenFlow 1.0)
    When you have resolved this problem run "git am --resolved".
    If you would prefer to skip this patch, instead run "git am --skip".
    To restore the original branch and stop patching run "git am --abort".

I don't know why; I applied all of the previous patches on top of the
openflow-0.9 branch.

So this review (and the ones for the following patches too, presumably)
will be just from looking at the diff.

> @@ -244,6 +244,7 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct odp_flow_key *key)
>  		int th_ofs = nh_ofs + nh->ihl * 4;
>  		key->nw_src = nh->saddr;
>  		key->nw_dst = nh->daddr;
> +		key->nw_tos = nh->tos & 0xfc;
>  		key->nw_proto = nh->protocol;
>  		skb_set_transport_header(skb, th_ofs);

I guess I always assumed that the TOS bits would be in the least
significant 6 bits of the nw_tos field, i.e. nh->tos >> 2.  I hope that
the OpenFlow specification is clear about this one way or another?

> @@ -291,11 +295,11 @@ void
>  flow_format(struct ds *ds, const flow_t *flow)
>  {
>      ds_put_format(ds, "in_port%04x:vlan%d:pcp%d mac"ETH_ADDR_FMT
> -                  "->"ETH_ADDR_FMT" type%04x proto%"PRId8" ip"IP_FMT
> +                  "->"ETH_ADDR_FMT" type%04x proto%"PRId8" tos%d ip"IP_FMT

__u8 should be printed with PRIu8.

>                    "->"IP_FMT" port%d->%d",
>                    flow->in_port, ntohs(flow->dl_vlan), flow->dl_vlan_pcp,
>                    ETH_ADDR_ARGS(flow->dl_src), ETH_ADDR_ARGS(flow->dl_dst),
> -                  ntohs(flow->dl_type), flow->nw_proto,
> +                  ntohs(flow->dl_type), flow->nw_proto, flow->nw_tos,
>                    IP_ARGS(&flow->nw_src), IP_ARGS(&flow->nw_dst),
>                    ntohs(flow->tp_src), ntohs(flow->tp_dst));