[ovs-dev] [PATCH v2 14/22] flow: Make room after ct_state.
Jarno Rajahalme
jarno at ovn.org
Fri Mar 3 18:51:03 UTC 2017
> On Mar 2, 2017, at 6:41 PM, Joe Stringer <joe at ovn.org> wrote:
>
> On 28 February 2017 at 17:17, Jarno Rajahalme <jarno at ovn.org <mailto:jarno at ovn.org>> wrote:
>> 'ct_state' currently only needs 8 bits, so we can make room for a new
>> CT field introduced in the next patch.
>>
>> Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
>> ---
>> include/openvswitch/flow.h | 3 ++-
>> lib/flow.c | 3 ++-
>> lib/match.c | 8 ++++----
>> lib/packets.h | 2 +-
>> ofproto/ofproto-dpif.c | 2 +-
>> tests/ovs-ofctl.at | 2 +-
>> 6 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
>> index df80dfe..9169272 100644
>> --- a/include/openvswitch/flow.h
>> +++ b/include/openvswitch/flow.h
>> @@ -91,7 +91,8 @@ struct flow {
>> * computation is opaque to the user space. */
>> union flow_in_port in_port; /* Input port.*/
>> uint32_t recirc_id; /* Must be exact match. */
>> - uint16_t ct_state; /* Connection tracking state. */
>> + uint8_t ct_state; /* Connection tracking state. */
>> + uint8_t pad0;
>> uint16_t ct_zone; /* Connection tracking zone. */
>> uint32_t ct_mark; /* Connection mark.*/
>> uint8_t pad1[4]; /* Pad to 64 bits. */
>> diff --git a/lib/flow.c b/lib/flow.c
>> index fb7bfeb..0c95b75 100644
>> --- a/lib/flow.c
>> +++ b/lib/flow.c
>> @@ -593,7 +593,8 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
>> miniflow_push_uint32(mf, in_port, odp_to_u32(md->in_port.odp_port));
>> if (md->recirc_id || md->ct_state) {
>> miniflow_push_uint32(mf, recirc_id, md->recirc_id);
>> - miniflow_push_uint16(mf, ct_state, md->ct_state);
>> + miniflow_push_uint8(mf, ct_state, md->ct_state);
>> + miniflow_push_uint8(mf, pad0, 0);
>> miniflow_push_uint16(mf, ct_zone, md->ct_zone);
>> }
>>
>> diff --git a/lib/match.c b/lib/match.c
>> index 3fcaec5..882bf0c 100644
>> --- a/lib/match.c
>> +++ b/lib/match.c
>> @@ -340,8 +340,8 @@ match_set_ct_state(struct match *match, uint32_t ct_state)
>> void
>> match_set_ct_state_masked(struct match *match, uint32_t ct_state, uint32_t mask)
>> {
>> - match->flow.ct_state = ct_state & mask & UINT16_MAX;
>> - match->wc.masks.ct_state = mask & UINT16_MAX;
>> + match->flow.ct_state = ct_state & mask & UINT8_MAX;
>> + match->wc.masks.ct_state = mask & UINT8_MAX;
>> }
>>
>> void
>> @@ -1111,7 +1111,7 @@ match_format(const struct match *match, struct ds *s, int priority)
>> }
>>
>> if (wc->masks.ct_state) {
>> - if (wc->masks.ct_state == UINT16_MAX) {
>> + if (wc->masks.ct_state == UINT8_MAX) {
>> ds_put_format(s, "%sct_state=%s", colors.param, colors.end);
>> if (f->ct_state) {
>> format_flags(s, ct_state_to_string, f->ct_state, '|');
>> @@ -1120,7 +1120,7 @@ match_format(const struct match *match, struct ds *s, int priority)
>> }
>> } else {
>> format_flags_masked(s, "ct_state", ct_state_to_string,
>> - f->ct_state, wc->masks.ct_state, UINT16_MAX);
>> + f->ct_state, wc->masks.ct_state, UINT8_MAX);
>> }
>> ds_put_char(s, ',');
>> }
>> diff --git a/lib/packets.h b/lib/packets.h
>> index c4d3799..f7e1d82 100644
>> --- a/lib/packets.h
>> +++ b/lib/packets.h
>> @@ -99,7 +99,7 @@ struct pkt_metadata {
>> action. */
>> uint32_t skb_priority; /* Packet priority for QoS. */
>> uint32_t pkt_mark; /* Packet mark. */
>> - uint16_t ct_state; /* Connection state. */
>> + uint8_t ct_state; /* Connection state. */
>> uint16_t ct_zone; /* Connection zone. */
>> uint32_t ct_mark; /* Connection mark. */
>> ovs_u128 ct_label; /* Connection label. */
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index 89c7b7f..e595f3b 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -3994,7 +3994,7 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow)
>> uint32_t ct_mark;
>>
>> support = &ofproto->backer->support.odp;
>> - ct_state = MINIFLOW_GET_U16(flow, ct_state);
>> + ct_state = MINIFLOW_GET_U8(flow, ct_state);
>> if (support->ct_state && support->ct_zone && support->ct_mark
>> && support->ct_label && support->ct_state_nat) {
>> return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0;
>> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
>> index 7e26735..9c32788 100644
>> --- a/tests/ovs-ofctl.at
>> +++ b/tests/ovs-ofctl.at
>> @@ -1215,7 +1215,7 @@ NXM_NX_REG0(a0e0d050)
>> dnl
>> dnl When re-serialising, bits 16-31 are wildcarded, because current OVS userspace
>> dnl doesn't understand (or store) those bits.
>> -NXM_OF_ETH_TYPE(0800), NXM_NX_CT_STATE_W(00000020/0000ffff)
>> +NXM_OF_ETH_TYPE(0800), NXM_NX_CT_STATE_W(00000020/000000ff)
>> nx_pull_match() returned error OFPBMC_BAD_VALUE
>> NXM_OF_ETH_TYPE(0800), NXM_NX_CT_STATE_W(00000020/00000020)
>> NXM_OF_ETH_TYPE(0800), NXM_NX_CT_STATE_W(00000020/000000f0)
>
> Looks like the comment immediately above these lines needs updating.
>
Right.
> Is this changing controller-facing ct_state field size?
No, but maybe you double-check, as you are more familiar with that piece?
Jarno
More information about the dev
mailing list