Thanks for the review, I pushed the series to master.
On Tue, Aug 13, 2013 at 1:48 PM, Andy Zhou <az...@nicira.com> wrote:
> 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
>
>
X-CudaMail-Whitelist-To: dev@openvswitch.org
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
