Hi Sairam,
You're right, that should be tcp_peer_to_protoinfo_flags(&conn->peer[1])
Thanks for pointing that out!
Daniele
On 16/06/2016 15:57, "Sairam Venugopal" <vsai...@vmware.com> wrote:
>Hi Daniele,
>
>I was trying to add conntrack dump support based on this patch and found a
>discrepancy in
>conntrack-tcp.c.
>
>Can you confirm if this is intended? I have inlined the comment.
>
>Thanks,
>Sairam
>
>
>
>On 6/10/16, 3:47 PM, "Daniele Di Proietto" <diproiet...@vmware.com> wrote:
>
>>New functions are implemented in the conntrack module to support this.
>>
>>Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
>>Acked-by: Flavio Leitner <f...@sysclose.org>
>>---
>> lib/conntrack-private.h | 3 ++
>> lib/conntrack-tcp.c | 34 +++++++++++++
>> lib/conntrack.c | 123
>>++++++++++++++++++++++++++++++++++++++++++++++++
>> lib/conntrack.h | 15 ++++++
>> lib/dpif-netdev.c | 60 +++++++++++++++++++++--
>> 5 files changed, 232 insertions(+), 3 deletions(-)
>>
>>diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
>>index 4743dc6..d5d5e52 100644
>>--- a/lib/conntrack-private.h
>>+++ b/lib/conntrack-private.h
>>@@ -22,6 +22,7 @@
>> #include <netinet/ip6.h>
>>
>> #include "conntrack.h"
>>+#include "ct-dpif.h"
>> #include "hmap.h"
>> #include "openvswitch/list.h"
>> #include "openvswitch/types.h"
>>@@ -75,6 +76,8 @@ struct ct_l4_proto {
>> struct conntrack_bucket *,
>> struct dp_packet *pkt, bool reply,
>> long long now);
>>+ void (*conn_get_protoinfo)(const struct conn *,
>>+ struct ct_dpif_protoinfo *);
>> };
>>
>> extern struct ct_l4_proto ct_proto_tcp;
>>diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
>>index 71eadc1..947e1c9 100644
>>--- a/lib/conntrack-tcp.c
>>+++ b/lib/conntrack-tcp.c
>>@@ -457,8 +457,42 @@ tcp_new_conn(struct conntrack_bucket *ctb, struct
>>dp_packet *pkt,
>> return &newconn->up;
>> }
>>
>>+static uint8_t
>>+tcp_peer_to_protoinfo_flags(const struct tcp_peer *peer)
>>+{
>>+ uint8_t res = 0;
>>+
>>+ if (peer->wscale & CT_WSCALE_FLAG) {
>>+ res |= CT_DPIF_TCPF_WINDOW_SCALE;
>>+ }
>>+
>>+ if (peer->wscale & CT_WSCALE_UNKNOWN) {
>>+ res |= CT_DPIF_TCPF_BE_LIBERAL;
>>+ }
>>+
>>+ return res;
>>+}
>>+
>>+static void
>>+tcp_conn_get_protoinfo(const struct conn *conn_,
>>+ struct ct_dpif_protoinfo *protoinfo)
>>+{
>>+ const struct conn_tcp *conn = conn_tcp_cast(conn_);
>>+
>>+ protoinfo->proto = IPPROTO_TCP;
>>+ protoinfo->tcp.state_orig = conn->peer[0].state;
>>+ protoinfo->tcp.state_reply = conn->peer[1].state;
>>+
>>+ protoinfo->tcp.wscale_orig = conn->peer[0].wscale & CT_WSCALE_MASK;
>>+ protoinfo->tcp.wscale_reply = conn->peer[1].wscale & CT_WSCALE_MASK;
>>+
>>+ protoinfo->tcp.flags_orig =
>>tcp_peer_to_protoinfo_flags(&conn->peer[0]);
>>+ protoinfo->tcp.flags_reply =
>>tcp_peer_to_protoinfo_flags(&conn->peer[0]);
>
>Sai: Shouldn¹t the reply be tcp_peer_to_protoinfo_flags(&conn->peer[1]);
>
>
>>+}
>>+
>> struct ct_l4_proto ct_proto_tcp = {
>> .new_conn = tcp_new_conn,
>> .valid_new = tcp_valid_new,
>> .conn_update = tcp_conn_update,
>>+ .conn_get_protoinfo = tcp_conn_get_protoinfo,
>> };
>>diff --git a/lib/conntrack.c b/lib/conntrack.c
>>index 5376550..6e59375 100644
>>--- a/lib/conntrack.c
>>+++ b/lib/conntrack.c
>>@@ -26,6 +26,7 @@
>> #include "conntrack-private.h"
>> #include "coverage.h"
>> #include "csum.h"
>>+#include "ct-dpif.h"
>> #include "dp-packet.h"
>> #include "flow.h"
>> #include "hmap.h"
>>@@ -1048,3 +1049,125 @@ delete_conn(struct conn *conn)
>> {
>> free(conn);
>> }
>>+?
>>+static void
>>+ct_endpoint_to_ct_dpif_inet_addr(const struct ct_addr *a,
>>+ union ct_dpif_inet_addr *b,
>>+ ovs_be16 dl_type)
>>+{
>>+ if (dl_type == htons(ETH_TYPE_IP)) {
>>+ b->ip = a->ipv4_aligned;
>>+ } else if (dl_type == htons(ETH_TYPE_IPV6)){
>>+ b->in6 = a->ipv6_aligned;
>>+ }
>>+}
>>+
>>+static void
>>+conn_key_to_tuple(const struct conn_key *key, struct ct_dpif_tuple
>>*tuple)
>>+{
>>+ if (key->dl_type == htons(ETH_TYPE_IP)) {
>>+ tuple->l3_type = AF_INET;
>>+ } else if (key->dl_type == htons(ETH_TYPE_IPV6)) {
>>+ tuple->l3_type = AF_INET6;
>>+ }
>>+ tuple->ip_proto = key->nw_proto;
>>+ ct_endpoint_to_ct_dpif_inet_addr(&key->src.addr, &tuple->src,
>>+ key->dl_type);
>>+ ct_endpoint_to_ct_dpif_inet_addr(&key->dst.addr, &tuple->dst,
>>+ key->dl_type);
>>+
>>+ if (key->nw_proto == IPPROTO_ICMP || key->nw_proto ==
>>IPPROTO_ICMPV6) {
>>+ tuple->icmp_id = key->src.port;
>>+ /* ICMP type and code are not tracked */
>>+ tuple->icmp_type = 0;
>>+ tuple->icmp_code = 0;
>>+ } else {
>>+ tuple->src_port = key->src.port;
>>+ tuple->dst_port = key->dst.port;
>>+ }
>>+}
>>+
>>+static void
>>+conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry
>>*entry,
>>+ long long now)
>>+{
>>+ struct ct_l4_proto *class;
>>+ long long expiration;
>>+ memset(entry, 0, sizeof *entry);
>>+ conn_key_to_tuple(&conn->key, &entry->tuple_orig);
>>+ conn_key_to_tuple(&conn->rev_key, &entry->tuple_reply);
>>+
>>+ entry->zone = conn->key.zone;
>>+ entry->mark = conn->mark;
>>+
>>+ memcpy(&entry->labels, &conn->label, sizeof(entry->labels));
>>+ /* Not implemented yet */
>>+ entry->timestamp.start = 0;
>>+ entry->timestamp.stop = 0;
>>+
>>+ expiration = conn->expiration - now;
>>+ entry->timeout = (expiration > 0) ? expiration / 1000 : 0;
>>+
>>+ class = l4_protos[conn->key.nw_proto];
>>+ if (class->conn_get_protoinfo) {
>>+ class->conn_get_protoinfo(conn, &entry->protoinfo);
>>+ }
>>+}
>>+
>>+int
>>+conntrack_dump_start(struct conntrack *ct, struct conntrack_dump *dump,
>>+ const uint16_t *pzone)
>>+{
>>+ memset(dump, 0, sizeof(*dump));
>>+ if (pzone) {
>>+ dump->zone = *pzone;
>>+ dump->filter_zone = true;
>>+ }
>>+ dump->ct = ct;
>>+
>>+ return 0;
>>+}
>>+
>>+int
>>+conntrack_dump_next(struct conntrack_dump *dump, struct ct_dpif_entry
>>*entry)
>>+{
>>+ struct conntrack *ct = dump->ct;
>>+ long long now = time_msec();
>>+
>>+ while (dump->bucket < CONNTRACK_BUCKETS) {
>>+ struct hmap_node *node;
>>+
>>+ ct_lock_lock(&ct->buckets[dump->bucket].lock);
>>+ for (;;) {
>>+ struct conn *conn;
>>+
>>+ node =
>>hmap_at_position(&ct->buckets[dump->bucket].connections,
>>+ &dump->bucket_pos);
>>+ if (!node) {
>>+ break;
>>+ }
>>+ INIT_CONTAINER(conn, node, node);
>>+ if (!dump->filter_zone || conn->key.zone == dump->zone) {
>>+ conn_to_ct_dpif_entry(conn, entry, now);
>>+ break;
>>+ }
>>+ /* Else continue, until we find an entry in the appropriate
>>zone
>>+ * or the bucket has been scanned completely. */
>>+ }
>>+ ct_lock_unlock(&ct->buckets[dump->bucket].lock);
>>+
>>+ if (!node) {
>>+ memset(&dump->bucket_pos, 0, sizeof dump->bucket_pos);
>>+ dump->bucket++;
>>+ } else {
>>+ return 0;
>>+ }
>>+ }
>>+ return EOF;
>>+}
>>+
>>+int
>>+conntrack_dump_done(struct conntrack_dump *dump OVS_UNUSED)
>>+{
>>+ return 0;
>>+}
>>diff --git a/lib/conntrack.h b/lib/conntrack.h
>>index 29bf4b7..eba3ae9 100644
>>--- a/lib/conntrack.h
>>+++ b/lib/conntrack.h
>>@@ -68,6 +68,21 @@ int conntrack_execute(struct conntrack *, struct
>>dp_packet_batch *,
>> bool commit, uint16_t zone, const uint32_t
>>*setmark,
>> const struct ovs_key_ct_labels *setlabel,
>> const char *helper);
>>+
>>+struct conntrack_dump {
>>+ struct conntrack *ct;
>>+ unsigned bucket;
>>+ struct hmap_position bucket_pos;
>>+ bool filter_zone;
>>+ uint16_t zone;
>>+};
>>+
>>+struct ct_dpif_entry;
>>+
>>+int conntrack_dump_start(struct conntrack *, struct conntrack_dump *,
>>+ const uint16_t *pzone);
>>+int conntrack_dump_next(struct conntrack_dump *, struct ct_dpif_entry *);
>>+int conntrack_dump_done(struct conntrack_dump *);
>> ?
>> /* 'struct ct_lock' is a wrapper for an adaptive mutex. It's useful to
>>try
>> * different types of locks (e.g. spinlocks) */
>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>index 48ec987..910ada3 100644
>>--- a/lib/dpif-netdev.c
>>+++ b/lib/dpif-netdev.c
>>@@ -35,6 +35,7 @@
>> #include "cmap.h"
>> #include "conntrack.h"
>> #include "coverage.h"
>>+#include "ct-dpif.h"
>> #include "csum.h"
>> #include "dp-packet.h"
>> #include "dpif.h"
>>@@ -4227,6 +4228,59 @@ dp_netdev_execute_actions(struct
>>dp_netdev_pmd_thread *pmd,
>> actions_len, dp_execute_cb);
>> }
>>
>>+struct dp_netdev_ct_dump {
>>+ struct ct_dpif_dump_state up;
>>+ struct conntrack_dump dump;
>>+ struct conntrack *ct;
>>+ struct dp_netdev *dp;
>>+};
>>+
>>+static int
>>+dpif_netdev_ct_dump_start(struct dpif *dpif, struct ct_dpif_dump_state
>>**dump_,
>>+ const uint16_t *pzone)
>>+{
>>+ struct dp_netdev *dp = get_dp_netdev(dpif);
>>+ struct dp_netdev_ct_dump *dump;
>>+
>>+ dump = xzalloc(sizeof *dump);
>>+ dump->dp = dp;
>>+ dump->ct = &dp->conntrack;
>>+
>>+ conntrack_dump_start(&dp->conntrack, &dump->dump, pzone);
>>+
>>+ *dump_ = &dump->up;
>>+
>>+ return 0;
>>+}
>>+
>>+static int
>>+dpif_netdev_ct_dump_next(struct dpif *dpif OVS_UNUSED,
>>+ struct ct_dpif_dump_state *dump_,
>>+ struct ct_dpif_entry *entry)
>>+{
>>+ struct dp_netdev_ct_dump *dump;
>>+
>>+ INIT_CONTAINER(dump, dump_, up);
>>+
>>+ return conntrack_dump_next(&dump->dump, entry);
>>+}
>>+
>>+static int
>>+dpif_netdev_ct_dump_done(struct dpif *dpif OVS_UNUSED,
>>+ struct ct_dpif_dump_state *dump_)
>>+{
>>+ struct dp_netdev_ct_dump *dump;
>>+ int err;
>>+
>>+ INIT_CONTAINER(dump, dump_, up);
>>+
>>+ err = conntrack_dump_done(&dump->dump);
>>+
>>+ free(dump);
>>+
>>+ return err;
>>+}
>>+
>> const struct dpif_class dpif_netdev_class = {
>> "netdev",
>> dpif_netdev_init,
>>@@ -4267,9 +4321,9 @@ const struct dpif_class dpif_netdev_class = {
>> dpif_netdev_enable_upcall,
>> dpif_netdev_disable_upcall,
>> dpif_netdev_get_datapath_version,
>>- NULL, /* ct_dump_start */
>>- NULL, /* ct_dump_next */
>>- NULL, /* ct_dump_done */
>>+ dpif_netdev_ct_dump_start,
>>+ dpif_netdev_ct_dump_next,
>>+ dpif_netdev_ct_dump_done,
>> NULL, /* ct_flush */
>> };
>>
>>--
>>2.8.1
>>
>>_______________________________________________
>>dev mailing list
>>dev@openvswitch.org
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>>n_listinfo_dev&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc
>>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=tNe8IQ972ivdkXYe_yHze175Gvfnc8
>>nkzR8IOYekTsY&s=27fwP-jORLNBENHqlEC-4Ozf8gkYCcKr-5uxM9C3OBE&e=
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
