All 4 patches look good to me.
Acked-by: Andy Zhou <az...@nicira.com>
On Tue, Aug 6, 2013 at 12:57 PM, Jesse Gross <je...@nicira.com> wrote:
> The Linux kernel datapath enables matching and setting the skb mark
> but this functionality is currently used only internally by
> ovs-vswitchd. This exposes it through NXM to enable external
> controllers to interact with other kernel subsystems. Although this
> is simply exporting the skb mark, the intention is that this is a
> platform independent mechanism to access some system metadata and
> therefore may have different implementations on various systems.
>
> Bug #17855
>
> Signed-off-by: Jesse Gross <je...@nicira.com>
> ---
> NEWS | 3 +++
> include/openflow/nicira-ext.h | 17 +++++++++++++++++
> lib/flow.c | 1 +
> lib/flow.h | 1 +
> lib/match.c | 21 +++++++++++++++++----
> lib/match.h | 1 +
> lib/meta-flow.c | 14 +++++++++-----
> lib/nx-match.c | 4 ++++
> lib/ofp-print.c | 4 ++++
> lib/ofp-util.c | 15 +++++++++++++--
> tests/ofproto.at | 4 ++--
> tests/ovs-ofctl.at | 8 +++++---
> utilities/ovs-ofctl.8.in | 7 +++++++
> 13 files changed, 84 insertions(+), 16 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index f9953ab..0165eff 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,9 @@ v1.12.0 - xx xxx xxxx
> * New support for matching outer source and destination IP address
> of tunneled packets, for tunnel ports configured with the newly
> added "remote_ip=flow" and "local_ip=flow" options.
> + * Support for matching on metadata 'pkt_mark' for interacting with
> + other system components. On Linux this corresponds to the skb
> + mark.
> - The Interface table in the database has a new "ifindex" column to
> report the interface's OS-assigned ifindex.
> - New "check-oftest" Makefile target for running OFTest against Open
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index 6319f4e..de5ff6a 100644
> --- a/include/openflow/nicira-ext.h
> +++ b/include/openflow/nicira-ext.h
> @@ -477,6 +477,7 @@ OFP_ASSERT(sizeof(struct nx_action_pop_queue) == 16);
> * - NXM_NX_ND_SLL
> * - NXM_NX_ND_TLL
> * - NXM_NX_REG(idx) for idx in the switch's accepted range.
> + * - NXM_NX_PKT_MARK
> * - NXM_NX_TUN_IPV4_SRC
> * - NXM_NX_TUN_IPV4_DST
> *
> @@ -498,6 +499,8 @@ OFP_ASSERT(sizeof(struct nx_action_pop_queue) == 16);
> *
> * - NXM_NX_REG(idx) for idx in the switch's accepted range.
> *
> + * - NXM_NX_PKT_MARK
> + *
> * - NXM_OF_VLAN_TCI. Modifying this field's value has side effects on
> the
> * packet's 802.1Q header. Setting a value with CFI=0 removes the
> 802.1Q
> * header (if any), ignoring the other bits. Setting a value with
> CFI=1
> @@ -1766,6 +1769,20 @@ OFP_ASSERT(sizeof(struct nx_action_output_reg) ==
> 24);
> #define NXM_NX_TUN_IPV4_DST NXM_HEADER (0x0001, 32, 4)
> #define NXM_NX_TUN_IPV4_DST_W NXM_HEADER_W(0x0001, 32, 4)
>
> +/* Metadata marked onto the packet in a system-dependent manner.
> + *
> + * The packet mark may be used to carry contextual information
> + * to other parts of the system outside of Open vSwitch. As a
> + * result, the semantics depend on system in use.
> + *
> + * Prereqs: None.
> + *
> + * Format: 32-bit integer in network byte order.
> + *
> + * Masking: Fully maskable. */
> +#define NXM_NX_PKT_MARK NXM_HEADER (0x0001, 33, 4)
> +#define NXM_NX_PKT_MARK_W NXM_HEADER_W(0x0001, 33, 4)
> +
> /* ## --------------------- ## */
> /* ## Requests and replies. ## */
> /* ## --------------------- ## */
> diff --git a/lib/flow.c b/lib/flow.c
> index a60965e..3e29aa1 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -500,6 +500,7 @@ flow_get_metadata(const struct flow *flow, struct
> flow_metadata *fmd)
> fmd->tun_dst = flow->tunnel.ip_dst;
> fmd->metadata = flow->metadata;
> memcpy(fmd->regs, flow->regs, sizeof fmd->regs);
> + fmd->pkt_mark = flow->pkt_mark;
> fmd->in_port = flow->in_port.ofp_port;
> }
>
> diff --git a/lib/flow.h b/lib/flow.h
> index 4c5fc03..8164d9c 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -128,6 +128,7 @@ struct flow_metadata {
> ovs_be32 tun_dst; /* Tunnel outer IPv4 dst addr */
> ovs_be64 metadata; /* OpenFlow 1.1+ metadata field. */
> uint32_t regs[FLOW_N_REGS]; /* Registers. */
> + uint32_t pkt_mark; /* Packet mark. */
> ofp_port_t in_port; /* OpenFlow port or zero. */
> };
>
> diff --git a/lib/match.c b/lib/match.c
> index db1553b..e97b0b1 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -138,7 +138,6 @@ match_init_exact(struct match *match, const struct
> flow *flow)
> {
> match->flow = *flow;
> match->flow.skb_priority = 0;
> - match->flow.pkt_mark = 0;
> flow_wildcards_init_exact(&match->wc);
> }
>
> @@ -288,8 +287,14 @@ match_set_skb_priority(struct match *match, uint32_t
> skb_priority)
> void
> match_set_pkt_mark(struct match *match, uint32_t pkt_mark)
> {
> - match->wc.masks.pkt_mark = UINT32_MAX;
> - match->flow.pkt_mark = pkt_mark;
> + match_set_pkt_mark_masked(match, pkt_mark, UINT32_MAX);
> +}
> +
> +void
> +match_set_pkt_mark_masked(struct match *match, uint32_t pkt_mark,
> uint32_t mask)
> +{
> + match->flow.pkt_mark = pkt_mark & mask;
> + match->wc.masks.pkt_mark = mask;
> }
>
> void
> @@ -836,8 +841,16 @@ match_format(const struct match *match, struct ds *s,
> unsigned int priority)
> ds_put_format(s, "priority=%u,", priority);
> }
>
> - if (wc->masks.pkt_mark) {
> + switch (wc->masks.pkt_mark) {
> + case 0:
> + break;
> + case UINT32_MAX:
> ds_put_format(s, "pkt_mark=%#"PRIx32",", f->pkt_mark);
> + break;
> + default:
> + ds_put_format(s, "pkt_mark=%#"PRIx32"/%#"PRIx32",",
> + f->pkt_mark, wc->masks.pkt_mark);
> + break;
> }
>
> if (wc->masks.skb_priority) {
> diff --git a/lib/match.h b/lib/match.h
> index f6b9868..5788721 100644
> --- a/lib/match.h
> +++ b/lib/match.h
> @@ -62,6 +62,7 @@ void match_set_tun_flags(struct match *match, uint16_t
> flags);
> void match_set_tun_flags_masked(struct match *match, uint16_t flags,
> uint16_t mask);
> void match_set_in_port(struct match *, ofp_port_t ofp_port);
> void match_set_pkt_mark(struct match *, uint32_t pkt_mark);
> +void match_set_pkt_mark_masked(struct match *, uint32_t pkt_mark,
> uint32_t mask);
> void match_set_skb_priority(struct match *, uint32_t skb_priority);
> void match_set_dl_type(struct match *, ovs_be16);
> void match_set_dl_src(struct match *, const uint8_t[6]);
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index 92849eb..ce061a3 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -139,12 +139,12 @@ static const struct mf_field mf_fields[MFF_N_IDS] = {
> }, {
> MFF_PKT_MARK, "pkt_mark", NULL,
> MF_FIELD_SIZES(be32),
> - MFM_NONE,
> + MFM_FULLY,
> MFS_HEXADECIMAL,
> MFP_NONE,
> - false,
> - 0, NULL,
> - 0, NULL,
> + true,
> + NXM_NX_PKT_MARK, "NXM_NX_PKT_MARK",
> + NXM_NX_PKT_MARK, "NXM_NX_PKT_MARK",
> },
>
> #define REGISTER(IDX) \
> @@ -1780,7 +1780,6 @@ mf_set(const struct mf_field *mf,
> switch (mf->id) {
> case MFF_IN_PORT:
> case MFF_IN_PORT_OXM:
> - case MFF_PKT_MARK:
> case MFF_SKB_PRIORITY:
> case MFF_ETH_TYPE:
> case MFF_DL_VLAN:
> @@ -1829,6 +1828,11 @@ mf_set(const struct mf_field *mf,
> ntohl(value->be32), ntohl(mask->be32));
> break;
>
> + case MFF_PKT_MARK:
> + match_set_pkt_mark_masked(match, ntohl(value->be32),
> + ntohl(mask->be32));
> + break;
> +
> case MFF_ETH_DST:
> match_set_dl_dst_masked(match, value->mac, mask->mac);
> break;
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 940dd9a..09f7f54 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -693,6 +693,10 @@ nx_put_raw(struct ofpbuf *b, bool oxm, const struct
> match *match,
> htonl(flow->regs[i]), htonl(match->wc.masks.regs[i]));
> }
>
> + /* Mark. */
> + nxm_put_32m(b, NXM_NX_PKT_MARK, htonl(flow->pkt_mark),
> + htonl(match->wc.masks.pkt_mark));
> +
> /* OpenFlow 1.1+ Metadata. */
> nxm_put_64m(b, OXM_OF_METADATA, flow->metadata,
> match->wc.masks.metadata);
>
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index 1a4dd9c..21989a9 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -130,6 +130,10 @@ ofp_print_packet_in(struct ds *string, const struct
> ofp_header *oh,
> }
> }
>
> + if (pin.fmd.pkt_mark != 0) {
> + ds_put_format(string, " pkt_mark=0x%"PRIx32, pin.fmd.pkt_mark);
> + }
> +
> ds_put_format(string, " (via %s)",
> ofputil_packet_in_reason_to_string(pin.reason,
> reasonbuf,
> sizeof reasonbuf));
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 6ae67a9..45ff0a1 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -1134,11 +1134,17 @@ ofputil_usable_protocols(const struct match *match)
> return OFPUTIL_P_NONE;
> }
>
> - /* pkt_mark and skb_priority can't be sent in a flow_mod */
> - if (wc->masks.pkt_mark || wc->masks.skb_priority) {
> + /* skb_priority can't be sent in a flow_mod */
> + if (wc->masks.skb_priority) {
> return OFPUTIL_P_NONE;
> }
>
> + /* NXM and OXM support pkt_mark */
> + if (wc->masks.pkt_mark) {
> + return OFPUTIL_P_OF10_NXM_ANY | OFPUTIL_P_OF12_OXM
> + | OFPUTIL_P_OF13_OXM;
> + }
> +
> /* NXM, OXM, and OF1.1 support bitwise matching on ethernet
> addresses. */
> if (!eth_mask_is_exact(wc->masks.dl_src)
> && !eth_addr_is_zero(wc->masks.dl_src)) {
> @@ -2917,6 +2923,7 @@ ofputil_decode_packet_in_finish(struct
> ofputil_packet_in *pin,
> pin->fmd.tun_dst = match->flow.tunnel.ip_dst;
> pin->fmd.metadata = match->flow.metadata;
> memcpy(pin->fmd.regs, match->flow.regs, sizeof pin->fmd.regs);
> + pin->fmd.pkt_mark = match->flow.pkt_mark;
> }
>
> enum ofperr
> @@ -3031,6 +3038,10 @@ ofputil_packet_in_to_match(const struct
> ofputil_packet_in *pin,
> }
> }
>
> + if (pin->fmd.pkt_mark != 0) {
> + match_set_pkt_mark(match, pin->fmd.pkt_mark);
> + }
> +
> match_set_in_port(match, pin->fmd.in_port);
> }
>
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index e2e6f1b..38bfb02 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -1563,14 +1563,14 @@ ovs-appctl -t ovs-ofctl ofctl/set-output-file
> monitor.log
> AT_CAPTURE_FILE([monitor.log])
>
> # Send a packet-out with a load action to set some metadata, and forward
> to controller
> -AT_CHECK([ovs-ofctl packet-out br0 controller
> 'load(0xfafafafa5a5a5a5a->OXM_OF_METADATA[[0..63]]), controller'
> '0001020304050010203040501234'])
> +AT_CHECK([ovs-ofctl packet-out br0 controller
> 'load(0xfafafafa5a5a5a5a->OXM_OF_METADATA[[0..63]]),
> load(0xaa->NXM_NX_PKT_MARK[[]]), controller'
> '0001020304050010203040501234'])
>
> # Stop the monitor and check its output.
> ovs-appctl -t ovs-ofctl ofctl/barrier
> ovs-appctl -t ovs-ofctl exit
>
> AT_CHECK([sed 's/ (xid=0x[[0-9a-fA-F]]*)//' monitor.log], [0], [dnl
> -NXT_PACKET_IN: total_len=14 in_port=CONTROLLER
> metadata=0xfafafafa5a5a5a5a (via action) data_len=14 (unbuffered)
> +NXT_PACKET_IN: total_len=14 in_port=CONTROLLER
> metadata=0xfafafafa5a5a5a5a pkt_mark=0xaa (via action) data_len=14
> (unbuffered)
>
>
> metadata=0,in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
> OFPT_BARRIER_REPLY:
> ])
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 5e3441d..996ea06 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -12,7 +12,7 @@ for test_case in \
> 'metadata=0 NXM,OXM' \
> 'in_port=1 any' \
> 'skb_priority=0 none' \
> - 'pkt_mark=1 none' \
> + 'pkt_mark=1 NXM,OXM' \
> 'reg0=0 NXM,OXM' \
> 'reg1=1 NXM,OXM' \
> 'reg2=2 NXM,OXM' \
> @@ -180,9 +180,9 @@ AT_CHECK([ovs-ofctl parse-flows flows.txt
> AT_CLEANUP
>
>
> -AT_SETUP([ovs-ofctl parse-flows (pkt_mark and skb_priority)])
> +AT_SETUP([ovs-ofctl parse-flows (skb_priority)])
> AT_DATA([flows.txt], [[
> -pkt_mark=0x12345678,skb_priority=0x12341234,tcp,tp_src=123,actions=flood
> +skb_priority=0x12341234,tcp,tp_src=123,actions=flood
> ]])
>
> AT_CHECK([ovs-ofctl parse-flows flows.txt
> @@ -197,6 +197,7 @@ AT_DATA([flows.txt], [[
> # comment
> tcp,tp_src=123,actions=flood
> in_port=LOCAL dl_vlan=9 dl_src=00:0A:E4:25:6B:B0 actions=drop
> +pkt_mark=0xbb,actions=set_field:0xaa->pkt_mark
> udp dl_vlan_pcp=7 idle_timeout=5 actions=strip_vlan output:0
> tcp,nw_src=192.168.0.3,tp_dst=80 actions=set_queue:37,output:1
> udp,nw_src=192.168.0.3,tp_dst=53 actions=pop_queue,output:1
> @@ -232,6 +233,7 @@ AT_CHECK([[sed 's/ (xid=0x[0-9a-fA-F]*)//' stdout]],
> [0],
> chosen protocol: NXM+table_id
> NXT_FLOW_MOD: ADD table:255 tcp,tp_src=123 actions=FLOOD
> NXT_FLOW_MOD: ADD table:255
> in_port=LOCAL,dl_vlan=9,dl_src=00:0a:e4:25:6b:b0 actions=drop
> +NXT_FLOW_MOD: ADD table:255 pkt_mark=0xbb
> actions=load:0xaa->NXM_NX_PKT_MARK[]
> NXT_FLOW_MOD: ADD table:255 udp,dl_vlan_pcp=7 idle:5
> actions=strip_vlan,output:0
> NXT_FLOW_MOD: ADD table:255 tcp,nw_src=192.168.0.3,tp_dst=80
> actions=set_queue:37,output:1
> NXT_FLOW_MOD: ADD table:255 udp,nw_src=192.168.0.3,tp_dst=53
> actions=pop_queue,output:1
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index 3e6c7fe..447d71c 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -806,6 +806,13 @@ exactly, and a 0-bit wildcards that bit.
> When a packet enters an OpenFlow switch, all of the registers are set
> to 0. Only explicit Nicira extension actions change register values.
> .
> +.IP \fBpkt_mark=\fIvalue\fR[\fB/\fImask\fR]
> +Matches packet metadata mark \fIvalue\fR either exactly or with optional
> +\fImask\fR. The mark is associated data that may be passed into other
> +system components in order to facilitate interaction between subsystems.
> +On Linux this corresponds to the skb mark but the exact implementation is
> +platform-dependent.
> +.
> .PP
> Defining IPv6 flows (those with \fBdl_type\fR equal to 0x86dd) requires
> support for NXM. The following shorthand notations are available for
> --
> 1.8.1.2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
