[ovs-dev] [PATCHv2 4/6] Add connection tracking label support.

Ben Pfaff blp at nicira.com
Fri Sep 18 17:49:03 UTC 2015


On Thu, Sep 17, 2015 at 04:04:26PM -0700, Joe Stringer wrote:
> This patch adds a new 128-bit metadata field to the connection tracking
> interface. When a label is specified as part of the ct action and the
> connection is committed, the value is saved with the current connection.
> Subsequent ct lookups with the table specified will expose this metadata
> as the "ct_label" field in the flow.
> 
> For example, to allow new connections from port 1->2 and only allow
> established connections from port 2->1, and to associate a label with
> those connections:
> 
>     priority=1,action=drop
>     priority=10,arp,action=normal
>     priority=10,icmp,action=normal
>     in_port=1,tcp,action=ct(commit,exec(set_field:1->ct_label)),2
>     in_port=2,ct_state=-trk,tcp,action=ct(table=1)
>     table=1,in_port=2,ct_state=+trk,ct_label=1,tcp,action=1
> 
> Signed-off-by: Joe Stringer <joestringer at nicira.com>
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
> ---
> v2: Address feedback from v1

MINIFLOW_GET_U128_PTR seems risky.  How you can be sure that both 64-bit
components of the u128 are present?

Is the formatting and parsing of ovs_u128 in lib/meta-flow.[ch]
consistent with what we're doing elsewhere?  It essentially treats the
bits in an ovs_u128 as if they were a (putative) ovs_be128; that is,
it's always big-endian.  I guess that's OK as long as we're consistent
throughout the code.

The wire format name though (in meta-flow.h etc) should probably not be
u128, because that implies that it's in host byte order.

I wonder whether ovs_u128 should actually be ovs_be128, with each of the
components defined as big-endian too.  Do we ever actually use an
ovs_u128 as an integer?  If not, then having it as big-endian would make
it a lot easier to think about (at least for me).

Thanks,

Ben.






More information about the dev mailing list