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 <[email protected]> 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 <[email protected]>
> >> ---
> >> 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
> >> [email protected]
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to