diff mbox

[ovs-dev] xlate: Use OVS_CT_ATTR_EVENTMASK.

Message ID 1493071129-69189-1-git-send-email-jarno@ovn.org
State Superseded
Headers show

Commit Message

Jarno Rajahalme April 24, 2017, 9:58 p.m. UTC 
Specify the event mask with CT commit including bits for CT features
exposed at the OVS interface (mark and label changes in addition to
basic creation and destruction of conntrack entries).

VMware-BZ: #1837218
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
This patch depends on the following other patches currently in review:
- "datapath-windows: Add missing IPCT_LABEL."
- "datapath: Add eventmask support to CT action."

build-aux/extract-odp-netlink-h | 2 ++
 ofproto/ofproto-dpif-xlate.c    | 3 +++
 2 files changed, 5 insertions(+)

Comments

Ben Pfaff April 25, 2017, 12:28 a.m. UTC | #1 
What's the visible effect of this change?  I am not sure, based on the
patch description.  One or two more sentences of context would help.

Thanks,

Ben.

On Mon, Apr 24, 2017 at 02:58:49PM -0700, Jarno Rajahalme wrote:
> Specify the event mask with CT commit including bits for CT features
> exposed at the OVS interface (mark and label changes in addition to
> basic creation and destruction of conntrack entries).
> 
> VMware-BZ: #1837218
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> ---
> This patch depends on the following other patches currently in review:
> - "datapath-windows: Add missing IPCT_LABEL."
> - "datapath: Add eventmask support to CT action."
> 
> build-aux/extract-odp-netlink-h | 2 ++
>  ofproto/ofproto-dpif-xlate.c    | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/build-aux/extract-odp-netlink-h b/build-aux/extract-odp-netlink-h
> index 907a70a..7fb6ce8 100755
> --- a/build-aux/extract-odp-netlink-h
> +++ b/build-aux/extract-odp-netlink-h
> @@ -19,6 +19,8 @@ $i\
>  #ifdef _WIN32\
>  #include "OvsDpInterfaceExt.h"\
>  #include "OvsDpInterfaceCtExt.h"\
> +#else\
> +#include "linux/netfilter/nf_conntrack_common.h"\
>  #endif\
>  
>  # Use OVS's own struct eth_addr instead of a 6-byte char array.
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index d8c6a7c..21f2f7a 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -5351,6 +5351,9 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
>      if (ofc->flags & NX_CT_F_COMMIT) {
>          nl_msg_put_flag(ctx->odp_actions, ofc->flags & NX_CT_F_FORCE ?
>                          OVS_CT_ATTR_FORCE_COMMIT : OVS_CT_ATTR_COMMIT);
> +        nl_msg_put_u32(ctx->odp_actions, OVS_CT_ATTR_EVENTMASK,
> +                       1 << IPCT_NEW | 1 << IPCT_RELATED | 1 << IPCT_DESTROY |
> +                       1 << IPCT_MARK | 1 << IPCT_LABEL);
>      }
>      nl_msg_put_u16(ctx->odp_actions, OVS_CT_ATTR_ZONE, zone);
>      put_ct_mark(&ctx->xin->flow, ctx->odp_actions, ctx->wc);
> -- 
> 2.1.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Jarno Rajahalme April 25, 2017, 2 a.m. UTC | #2 
Ben,

The visible effect is that listeners of conntrack update events do not get L4 state transition events but still get the mark and label change notifications. A simple way of seeing the difference is to run “conntrack -E” while running a test case both before and after this change. For example:

System testsuite test:

 65: conntrack - FTP NAT postrecirc seqadj           ok


Before:

root@debian-jr:/home/jrajahalme# conntrack -E
    [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 [UNREPLIED] src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 helper=ftp
 [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 helper=ftp
 [UPDATE] tcp      6 432000 ESTABLISHED src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED] helper=ftp
    [NEW] tcp      6 120 SYN_SENT src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 [UNREPLIED] src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549
 [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549
 [UPDATE] tcp      6 432000 ESTABLISHED src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]
 [UPDATE] tcp      6 120 FIN_WAIT src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]
 [UPDATE] tcp      6 30 LAST_ACK src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]
 [UPDATE] tcp      6 120 TIME_WAIT src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]
 [UPDATE] tcp      6 120 FIN_WAIT src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED] helper=ftp
 [UPDATE] tcp      6 30 LAST_ACK src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED] helper=ftp
 [UPDATE] tcp      6 120 TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED] helper=ftp
[DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED]
[DESTROY] tcp      6 src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]

After:

    [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=40014 dport=21 [UNREPLIED] src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40014 helper=ftp
    [NEW] tcp      6 120 SYN_SENT src=10.1.1.2 dst=10.1.1.240 sport=53008 dport=50987 [UNREPLIED] src=10.1.1.1 dst=10.1.1.2 sport=50987 dport=53008
[DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=40014 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40014 [ASSURED]
[DESTROY] tcp      6 src=10.1.1.2 dst=10.1.1.240 sport=53008 dport=50987 src=10.1.1.1 dst=10.1.1.2 sport=50987 dport=53008 [ASSURED]


System testsuite tests (HTTP GET tests):

 27: conntrack - ct_mark bit-fiddling                ok
 30: conntrack - ct_label bit-fiddling               ok

Before:

    [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 [UNREPLIED] src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 mark=5
 [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 mark=3
 [UPDATE] tcp      6 432000 ESTABLISHED src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3
 [UPDATE] tcp      6 120 FIN_WAIT src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3
 [UPDATE] tcp      6 30 LAST_ACK src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3
 [UPDATE] tcp      6 120 TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3
[DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3


    [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 [UNREPLIED] src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 labels=0,2
 [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 labels=0,33
 [UPDATE] tcp      6 432000 ESTABLISHED src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED]
 [UPDATE] tcp      6 432000 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
 [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
 [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
 [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
 [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
 [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
 [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
 [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
 [UPDATE] tcp      6 120 FIN_WAIT src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
 [UPDATE] tcp      6 30 LAST_ACK src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED]
 [UPDATE] tcp      6 120 TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
[DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED]

*** NOTE: Duplicate events above are due to a missing backport of nf_connlabels_replace() function change. Linux 4.7 changed it to trigger the update event only if the labels actually changed. In this test case the labels are being set to the same values for multiple packets, hence the duplicate events above, one per packet! ***

After (with this patch and the nf_connlabels_replace() backport):

    [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=44272 dport=80 [UNREPLIED] src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44272 mark=5
 [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=44272 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44272 mark=3
[DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=44272 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44272 [ASSURED] mark=3


    [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=44273 dport=80 [UNREPLIED] src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44273 labels=0,2
 [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=44273 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44273 labels=0,33
[DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=44273 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44273 [ASSURED]

I’ll write a more descriptive commit message for v2.

  Jarno

> On Apr 24, 2017, at 5:28 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> What's the visible effect of this change?  I am not sure, based on the
> patch description.  One or two more sentences of context would help.
> 
> Thanks,
> 
> Ben.
> 
> On Mon, Apr 24, 2017 at 02:58:49PM -0700, Jarno Rajahalme wrote:
>> Specify the event mask with CT commit including bits for CT features
>> exposed at the OVS interface (mark and label changes in addition to
>> basic creation and destruction of conntrack entries).
>> 
>> VMware-BZ: #1837218
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>> ---
>> This patch depends on the following other patches currently in review:
>> - "datapath-windows: Add missing IPCT_LABEL."
>> - "datapath: Add eventmask support to CT action."
>> 
>> build-aux/extract-odp-netlink-h | 2 ++
>> ofproto/ofproto-dpif-xlate.c    | 3 +++
>> 2 files changed, 5 insertions(+)
>> 
>> diff --git a/build-aux/extract-odp-netlink-h b/build-aux/extract-odp-netlink-h
>> index 907a70a..7fb6ce8 100755
>> --- a/build-aux/extract-odp-netlink-h
>> +++ b/build-aux/extract-odp-netlink-h
>> @@ -19,6 +19,8 @@ $i\
>> #ifdef _WIN32\
>> #include "OvsDpInterfaceExt.h"\
>> #include "OvsDpInterfaceCtExt.h"\
>> +#else\
>> +#include "linux/netfilter/nf_conntrack_common.h"\
>> #endif\
>> 
>> # Use OVS's own struct eth_addr instead of a 6-byte char array.
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index d8c6a7c..21f2f7a 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -5351,6 +5351,9 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
>>     if (ofc->flags & NX_CT_F_COMMIT) {
>>         nl_msg_put_flag(ctx->odp_actions, ofc->flags & NX_CT_F_FORCE ?
>>                         OVS_CT_ATTR_FORCE_COMMIT : OVS_CT_ATTR_COMMIT);
>> +        nl_msg_put_u32(ctx->odp_actions, OVS_CT_ATTR_EVENTMASK,
>> +                       1 << IPCT_NEW | 1 << IPCT_RELATED | 1 << IPCT_DESTROY |
>> +                       1 << IPCT_MARK | 1 << IPCT_LABEL);
>>     }
>>     nl_msg_put_u16(ctx->odp_actions, OVS_CT_ATTR_ZONE, zone);
>>     put_ct_mark(&ctx->xin->flow, ctx->odp_actions, ctx->wc);
>> -- 
>> 2.1.4
>> 
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ben Pfaff April 25, 2017, 5:10 a.m. UTC | #3 
OK, I understand now.  Thank you!

On Mon, Apr 24, 2017 at 07:00:38PM -0700, Jarno Rajahalme wrote:
> Ben,
> 
> The visible effect is that listeners of conntrack update events do not get L4 state transition events but still get the mark and label change notifications. A simple way of seeing the difference is to run “conntrack -E” while running a test case both before and after this change. For example:
> 
> System testsuite test:
> 
>  65: conntrack - FTP NAT postrecirc seqadj           ok
> 
> 
> Before:
> 
> root@debian-jr:/home/jrajahalme# conntrack -E
>     [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 [UNREPLIED] src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 helper=ftp
>  [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 helper=ftp
>  [UPDATE] tcp      6 432000 ESTABLISHED src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED] helper=ftp
>     [NEW] tcp      6 120 SYN_SENT src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 [UNREPLIED] src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549
>  [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549
>  [UPDATE] tcp      6 432000 ESTABLISHED src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]
>  [UPDATE] tcp      6 120 FIN_WAIT src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]
>  [UPDATE] tcp      6 30 LAST_ACK src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]
>  [UPDATE] tcp      6 120 TIME_WAIT src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]
>  [UPDATE] tcp      6 120 FIN_WAIT src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED] helper=ftp
>  [UPDATE] tcp      6 30 LAST_ACK src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED] helper=ftp
>  [UPDATE] tcp      6 120 TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED] helper=ftp
> [DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=40013 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40013 [ASSURED]
> [DESTROY] tcp      6 src=10.1.1.2 dst=10.1.1.240 sport=52549 dport=48972 src=10.1.1.1 dst=10.1.1.2 sport=48972 dport=52549 [ASSURED]
> 
> After:
> 
>     [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=40014 dport=21 [UNREPLIED] src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40014 helper=ftp
>     [NEW] tcp      6 120 SYN_SENT src=10.1.1.2 dst=10.1.1.240 sport=53008 dport=50987 [UNREPLIED] src=10.1.1.1 dst=10.1.1.2 sport=50987 dport=53008
> [DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=40014 dport=21 src=10.1.1.2 dst=10.1.1.240 sport=21 dport=40014 [ASSURED]
> [DESTROY] tcp      6 src=10.1.1.2 dst=10.1.1.240 sport=53008 dport=50987 src=10.1.1.1 dst=10.1.1.2 sport=50987 dport=53008 [ASSURED]
> 
> 
> System testsuite tests (HTTP GET tests):
> 
>  27: conntrack - ct_mark bit-fiddling                ok
>  30: conntrack - ct_label bit-fiddling               ok
> 
> Before:
> 
>     [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 [UNREPLIED] src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 mark=5
>  [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 mark=3
>  [UPDATE] tcp      6 432000 ESTABLISHED src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3
>  [UPDATE] tcp      6 120 FIN_WAIT src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3
>  [UPDATE] tcp      6 30 LAST_ACK src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3
>  [UPDATE] tcp      6 120 TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3
> [DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=44270 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44270 [ASSURED] mark=3
> 
> 
>     [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 [UNREPLIED] src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 labels=0,2
>  [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 labels=0,33
>  [UPDATE] tcp      6 432000 ESTABLISHED src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED]
>  [UPDATE] tcp      6 432000 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
>  [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
>  [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
>  [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
>  [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
>  [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
>  [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
>  [UPDATE] tcp      6 300 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
>  [UPDATE] tcp      6 120 FIN_WAIT src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
>  [UPDATE] tcp      6 30 LAST_ACK src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED]
>  [UPDATE] tcp      6 120 TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED] labels=0,33
> [DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=44271 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44271 [ASSURED]
> 
> *** NOTE: Duplicate events above are due to a missing backport of nf_connlabels_replace() function change. Linux 4.7 changed it to trigger the update event only if the labels actually changed. In this test case the labels are being set to the same values for multiple packets, hence the duplicate events above, one per packet! ***
> 
> After (with this patch and the nf_connlabels_replace() backport):
> 
>     [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=44272 dport=80 [UNREPLIED] src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44272 mark=5
>  [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=44272 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44272 mark=3
> [DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=44272 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44272 [ASSURED] mark=3
> 
> 
>     [NEW] tcp      6 120 SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=44273 dport=80 [UNREPLIED] src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44273 labels=0,2
>  [UPDATE] tcp      6 60 SYN_RECV src=10.1.1.1 dst=10.1.1.2 sport=44273 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44273 labels=0,33
> [DESTROY] tcp      6 src=10.1.1.1 dst=10.1.1.2 sport=44273 dport=80 src=10.1.1.2 dst=10.1.1.1 sport=80 dport=44273 [ASSURED]
> 
> I’ll write a more descriptive commit message for v2.
> 
>   Jarno
> 
> > On Apr 24, 2017, at 5:28 PM, Ben Pfaff <blp@ovn.org> wrote:
> > 
> > What's the visible effect of this change?  I am not sure, based on the
> > patch description.  One or two more sentences of context would help.
> > 
> > Thanks,
> > 
> > Ben.
> > 
> > On Mon, Apr 24, 2017 at 02:58:49PM -0700, Jarno Rajahalme wrote:
> >> Specify the event mask with CT commit including bits for CT features
> >> exposed at the OVS interface (mark and label changes in addition to
> >> basic creation and destruction of conntrack entries).
> >> 
> >> VMware-BZ: #1837218
> >> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> >> ---
> >> This patch depends on the following other patches currently in review:
> >> - "datapath-windows: Add missing IPCT_LABEL."
> >> - "datapath: Add eventmask support to CT action."
> >> 
> >> build-aux/extract-odp-netlink-h | 2 ++
> >> ofproto/ofproto-dpif-xlate.c    | 3 +++
> >> 2 files changed, 5 insertions(+)
> >> 
> >> diff --git a/build-aux/extract-odp-netlink-h b/build-aux/extract-odp-netlink-h
> >> index 907a70a..7fb6ce8 100755
> >> --- a/build-aux/extract-odp-netlink-h
> >> +++ b/build-aux/extract-odp-netlink-h
> >> @@ -19,6 +19,8 @@ $i\
> >> #ifdef _WIN32\
> >> #include "OvsDpInterfaceExt.h"\
> >> #include "OvsDpInterfaceCtExt.h"\
> >> +#else\
> >> +#include "linux/netfilter/nf_conntrack_common.h"\
> >> #endif\
> >> 
> >> # Use OVS's own struct eth_addr instead of a 6-byte char array.
> >> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> >> index d8c6a7c..21f2f7a 100644
> >> --- a/ofproto/ofproto-dpif-xlate.c
> >> +++ b/ofproto/ofproto-dpif-xlate.c
> >> @@ -5351,6 +5351,9 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
> >>     if (ofc->flags & NX_CT_F_COMMIT) {
> >>         nl_msg_put_flag(ctx->odp_actions, ofc->flags & NX_CT_F_FORCE ?
> >>                         OVS_CT_ATTR_FORCE_COMMIT : OVS_CT_ATTR_COMMIT);
> >> +        nl_msg_put_u32(ctx->odp_actions, OVS_CT_ATTR_EVENTMASK,
> >> +                       1 << IPCT_NEW | 1 << IPCT_RELATED | 1 << IPCT_DESTROY |
> >> +                       1 << IPCT_MARK | 1 << IPCT_LABEL);
> >>     }
> >>     nl_msg_put_u16(ctx->odp_actions, OVS_CT_ATTR_ZONE, zone);
> >>     put_ct_mark(&ctx->xin->flow, ctx->odp_actions, ctx->wc);
> >> -- 
> >> 2.1.4
> >> 
> >> _______________________________________________
> >> dev mailing list
> >> dev@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox

Patch

diff --git a/build-aux/extract-odp-netlink-h b/build-aux/extract-odp-netlink-h
index 907a70a..7fb6ce8 100755
--- a/build-aux/extract-odp-netlink-h
+++ b/build-aux/extract-odp-netlink-h
@@ -19,6 +19,8 @@  $i\
 #ifdef _WIN32\
 #include "OvsDpInterfaceExt.h"\
 #include "OvsDpInterfaceCtExt.h"\
+#else\
+#include "linux/netfilter/nf_conntrack_common.h"\
 #endif\
 
 # Use OVS's own struct eth_addr instead of a 6-byte char array.
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d8c6a7c..21f2f7a 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5351,6 +5351,9 @@  compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
     if (ofc->flags & NX_CT_F_COMMIT) {
         nl_msg_put_flag(ctx->odp_actions, ofc->flags & NX_CT_F_FORCE ?
                         OVS_CT_ATTR_FORCE_COMMIT : OVS_CT_ATTR_COMMIT);
+        nl_msg_put_u32(ctx->odp_actions, OVS_CT_ATTR_EVENTMASK,
+                       1 << IPCT_NEW | 1 << IPCT_RELATED | 1 << IPCT_DESTROY |
+                       1 << IPCT_MARK | 1 << IPCT_LABEL);
     }
     nl_msg_put_u16(ctx->odp_actions, OVS_CT_ATTR_ZONE, zone);
     put_ct_mark(&ctx->xin->flow, ctx->odp_actions, ctx->wc);