Hello Yifeng, thanks for the patch.
Yifeng Sun <[email protected]> writes: > Userspace conntrack doesn't support conntrack stats for packets and > bytes. This patch implements it. > > Signed-off-by: Yifeng Sun <[email protected]> > --- > lib/conntrack-private.h | 9 +++++++++ > lib/conntrack.c | 28 ++++++++++++++++++++++++++++ > tests/system-common-macros.at | 2 +- > tests/system-traffic.at | 30 ++++++++++++++++++++++++++++++ > 4 files changed, 68 insertions(+), 1 deletion(-) > > diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h > index dfdf4e676..7f21d3772 100644 > --- a/lib/conntrack-private.h > +++ b/lib/conntrack-private.h > @@ -91,6 +91,11 @@ enum OVS_PACKED_ENUM ct_conn_type { > CT_CONN_TYPE_UN_NAT, > }; > > +struct conn_counter { > + atomic_uint64_t packets; > + atomic_uint64_t bytes; > +}; > + > struct conn { > /* Immutable data. */ > struct conn_key key; > @@ -123,6 +128,10 @@ struct conn { > enum ct_conn_type conn_type; > > uint32_t tp_id; /* Timeout policy ID. */ > + > + /* Counters. */ > + struct conn_counter counters_orig; > + struct conn_counter counters_reply; > }; > > enum ct_update_res { > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 33a1a9295..177154cd8 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -1245,6 +1245,21 @@ conn_update_state_alg(struct conntrack *ct, struct > dp_packet *pkt, > return false; > } > > +static void > +conn_update_counters(struct conn *conn, > + const struct dp_packet *pkt, bool reply) > +{ > + if (conn) { > + struct conn_counter *counter = (reply > + ? &conn->counters_reply > + : &conn->counters_orig); > + uint64_t old; > + > + atomic_count_inc64(&counter->packets); > + atomic_add(&counter->bytes, dp_packet_size(pkt), &old); > + } > +} > + > static void > set_cached_conn(const struct nat_action_info_t *nat_action_info, > const struct conn_lookup_ctx *ctx, struct conn *conn, > @@ -1283,6 +1298,8 @@ process_one_fast(uint16_t zone, const uint32_t *setmark, > if (setlabel) { > set_label(pkt, conn, &setlabel[0], &setlabel[1]); > } > + > + conn_update_counters(conn, pkt, pkt->md.reply); > } > > static void > @@ -1420,6 +1437,8 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, > set_label(pkt, conn, &setlabel[0], &setlabel[1]); > } > > + conn_update_counters(conn, pkt, ctx->reply); > + > handle_alg_ctl(ct, ctx, pkt, ct_alg_ctl, conn, now, !!nat_action_info); > > set_cached_conn(nat_action_info, ctx, conn, pkt); > @@ -2641,6 +2660,15 @@ conn_to_ct_dpif_entry(const struct conn *conn, struct > ct_dpif_entry *entry, > } > ovs_mutex_unlock(&conn->lock); > > + entry->counters_orig.packets = atomic_count_get64( > + (atomic_uint64_t *)&conn->counters_orig.packets); > + entry->counters_orig.bytes = atomic_count_get64( > + (atomic_uint64_t *)&conn->counters_orig.bytes); > + entry->counters_reply.packets = atomic_count_get64( > + (atomic_uint64_t *)&conn->counters_reply.packets); > + entry->counters_reply.bytes = atomic_count_get64( > + (atomic_uint64_t *)&conn->counters_reply.bytes); > + > entry->timeout = (expiration > 0) ? expiration / 1000 : 0; > > if (conn->alg) { > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 19a0b125b..89cd7b83c 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -240,7 +240,7 @@ m4_define([STRIP_MONITOR_CSUM], [grep "csum:" | sed > 's/csum:.*/csum: <skip>/']) > # and limit the output to the rows containing 'ip-addr'. > # > m4_define([FORMAT_CT], > - [[grep "dst=$1" | sed -e 's/port=[0-9]*/port=<cleared>/g' -e > 's/id=[0-9]*/id=<cleared>/g' -e 's/state=[0-9_A-Z]*/state=<cleared>/g' | sort > | uniq]]) > + [[grep "dst=$1" | sed -e 's/port=[0-9]*/port=<cleared>/g' -e > 's/id=[0-9]*/id=<cleared>/g' -e 's/state=[0-9_A-Z]*/state=<cleared>/g' -e > 's/timeout=[0-9]*/timeout=<cleared>/g' | sort | uniq]]) > > # NETNS_DAEMONIZE([namespace], [command], [pidfile]) > # > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index f22d86e46..15b2c288c 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -6743,6 +6743,36 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | > OFPROTO_CLEAR_DURATION_IDLE > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([conntrack - stats]) nit: maybe we can use a more descriptive message in terms of what the test aims to check, like: "Stats with packets and bytes counters" anything else you think it would best describe the test will work. > +CHECK_CONNTRACK() > +OVS_TRAFFIC_VSWITCHD_START() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > + > +AT_DATA([flows.txt], [dnl > +priority=1,action=drop > +priority=10,arp,action=normal > +priority=100,in_port=1,icmp,action=ct(commit),2 > +priority=100,in_port=2,ct_state=-trk,icmp,action=ct(table=0) > +priority=100,in_port=2,ct_state=+trk+est-new,icmp,action=1 > +]) > + > +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) > + > +NS_CHECK_EXEC([at_ns0], [ping -s 500 -q -c 3 -i 0.3 -w 2 10.1.1.2 | > FORMAT_PING], [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +AT_CHECK([ovs-appctl dpctl/dump-conntrack -s | FORMAT_CT(10.1.1.2)], [0], > [dnl > +icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0,packets=3,bytes=1626),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0,packets=3,bytes=1626),timeout=<cleared> > +]) > + this would require: sysctl -w net.netfilter.nf_conntrack_acct=1 In my case it defaults to zero which in turn makes the test fail for kernel dp. I guess it would be ok to enable it as long as we restore it back to the old value. On top of that, after enabling it, the test still fails for a mismatch in the bytes counter (didn't check it, but guess it's a matter of mac len): icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=15137,type=8,code=0,packets=3,bytes=1584),reply=(src=10.1.1.2,dst=10.1.1.1,id=15137,type=0,code=0,packets=3,bytes=1584),timeout=29 > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > AT_BANNER([802.1ad]) > > AT_SETUP([802.1ad - vlan_limit]) > -- > 2.17.1 > > _______________________________________________ > 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
