diff mbox

[ovs-dev,7/8] tnl-ports: Handle STT ports.

Message ID 1452496694-50996-8-git-send-email-pshelar@nicira.com
State Changes Requested
Headers show

Commit Message

Pravin B Shelar Jan. 11, 2016, 7:18 a.m. UTC 
STT uses TCP port so we need to filter traffic on basis of TCP
port numbers.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 lib/tnl-ports.c  | 55 +++++++++++++++++++++++++++++++------------------------
 lib/tnl-ports.h  |  4 ++--
 ofproto/tunnel.c |  5 ++++-
 3 files changed, 37 insertions(+), 27 deletions(-)

Comments

Jesse Gross Jan. 22, 2016, 7:51 p.m. UTC | #1 
On Sun, Jan 10, 2016 at 11:18 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
> diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
> index e7f2066..13d114d 100644
> --- a/lib/tnl-ports.c
> +++ b/lib/tnl-ports.c
> @@ -95,24 +96,28 @@ tnl_port_init_flow(struct flow *flow, struct eth_addr mac,
> -    if (udp_port) {
> -        flow->nw_proto = IPPROTO_UDP;
> +    if (tp_port) {
> +        if (!strcmp(type, "stt")) {
> +            flow->nw_proto = IPPROTO_TCP;
> +        } else {
> +            flow->nw_proto = IPPROTO_UDP;
> +        }
>      } else {
>          flow->nw_proto = IPPROTO_GRE;
>      }

Now that we have the type, it might be better to just use it in all
cases instead of heuristics like no port numbers.

> @@ -129,9 +134,9 @@ map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr,
>           /* XXX: No fragments support. */
>          match.wc.masks.nw_frag = FLOW_NW_FRAG_MASK;
>
> -        /* 'udp_port' is zero for non-UDP tunnels (e.g. GRE). In this case it
> +        /* 'tp_port' is zero for non-UDP tunnels (e.g. GRE). In this case it

Not just UDP tunnels any more.

>  void
>  tnl_port_map_insert(odp_port_t port,
> -                    ovs_be16 udp_port, const char dev_name[])
> +                    ovs_be16 tp_port, const char dev_name[], const char type[])
>  {
>      struct tnl_port *p;
>      struct ip_device *ip_dev;
>
>      ovs_mutex_lock(&mutex);
>      LIST_FOR_EACH(p, node, &port_list) {
> -        if (udp_port == p->udp_port) {
> +        if (tp_port == p->tp_port) {
>               goto out;
>          }
>      }

Doesn't this need to check the type? Also the same when we delete the tunnel.
Pravin Shelar Jan. 22, 2016, 8:52 p.m. UTC | #2 
On Fri, Jan 22, 2016 at 11:51 AM, Jesse Gross <jesse@kernel.org> wrote:
> On Sun, Jan 10, 2016 at 11:18 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
>> diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
>> index e7f2066..13d114d 100644
>> --- a/lib/tnl-ports.c
>> +++ b/lib/tnl-ports.c
>> @@ -95,24 +96,28 @@ tnl_port_init_flow(struct flow *flow, struct eth_addr mac,
>> -    if (udp_port) {
>> -        flow->nw_proto = IPPROTO_UDP;
>> +    if (tp_port) {
>> +        if (!strcmp(type, "stt")) {
>> +            flow->nw_proto = IPPROTO_TCP;
>> +        } else {
>> +            flow->nw_proto = IPPROTO_UDP;
>> +        }
>>      } else {
>>          flow->nw_proto = IPPROTO_GRE;
>>      }
>
> Now that we have the type, it might be better to just use it in all
> cases instead of heuristics like no port numbers.
>
ok.

>> @@ -129,9 +134,9 @@ map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr,
>>           /* XXX: No fragments support. */
>>          match.wc.masks.nw_frag = FLOW_NW_FRAG_MASK;
>>
>> -        /* 'udp_port' is zero for non-UDP tunnels (e.g. GRE). In this case it
>> +        /* 'tp_port' is zero for non-UDP tunnels (e.g. GRE). In this case it
>
> Not just UDP tunnels any more.
>
>>  void
>>  tnl_port_map_insert(odp_port_t port,
>> -                    ovs_be16 udp_port, const char dev_name[])
>> +                    ovs_be16 tp_port, const char dev_name[], const char type[])
>>  {
>>      struct tnl_port *p;
>>      struct ip_device *ip_dev;
>>
>>      ovs_mutex_lock(&mutex);
>>      LIST_FOR_EACH(p, node, &port_list) {
>> -        if (udp_port == p->udp_port) {
>> +        if (tp_port == p->tp_port) {
>>               goto out;
>>          }
>>      }
>
> Doesn't this need to check the type? Also the same when we delete the tunnel.

right, I will add type to this check.

> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
diff mbox

Patch

diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index e7f2066..13d114d 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -51,8 +51,9 @@  static struct ovs_list addr_list;
 
 struct tnl_port {
     odp_port_t port;
-    ovs_be16 udp_port;
+    ovs_be16 tp_port;
     char dev_name[IFNAMSIZ];
+    char type[IFNAMSIZ];
     struct ovs_list node;
 };
 
@@ -82,7 +83,7 @@  tnl_port_free(struct tnl_port_in *p)
 
 static void
 tnl_port_init_flow(struct flow *flow, struct eth_addr mac,
-                   struct in6_addr *addr, ovs_be16 udp_port)
+                   struct in6_addr *addr, ovs_be16 tp_port, const char type[])
 {
     memset(flow, 0, sizeof *flow);
 
@@ -95,24 +96,28 @@  tnl_port_init_flow(struct flow *flow, struct eth_addr mac,
         flow->ipv6_dst = *addr;
     }
 
-    if (udp_port) {
-        flow->nw_proto = IPPROTO_UDP;
+    if (tp_port) {
+        if (!strcmp(type, "stt")) {
+            flow->nw_proto = IPPROTO_TCP;
+        } else {
+            flow->nw_proto = IPPROTO_UDP;
+        }
     } else {
         flow->nw_proto = IPPROTO_GRE;
     }
-    flow->tp_dst = udp_port;
+    flow->tp_dst = tp_port;
 }
 
 static void
 map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr,
-           ovs_be16 udp_port, const char dev_name[])
+           ovs_be16 tp_port, const char dev_name[], const char type[])
 {
     const struct cls_rule *cr;
     struct tnl_port_in *p;
     struct match match;
 
     memset(&match, 0, sizeof match);
-    tnl_port_init_flow(&match.flow, mac, addr, udp_port);
+    tnl_port_init_flow(&match.flow, mac, addr, tp_port, type);
 
     do {
         cr = classifier_lookup(&cls, CLS_MAX_VERSION, &match.flow, NULL);
@@ -129,9 +134,9 @@  map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr,
          /* XXX: No fragments support. */
         match.wc.masks.nw_frag = FLOW_NW_FRAG_MASK;
 
-        /* 'udp_port' is zero for non-UDP tunnels (e.g. GRE). In this case it
+        /* 'tp_port' is zero for non-UDP tunnels (e.g. GRE). In this case it
          * doesn't make sense to match on UDP port numbers. */
-        if (udp_port) {
+        if (tp_port) {
             match.wc.masks.tp_dst = OVS_BE16_MAX;
         }
         if (IN6_IS_ADDR_V4MAPPED(addr)) {
@@ -152,33 +157,34 @@  map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr,
 
 void
 tnl_port_map_insert(odp_port_t port,
-                    ovs_be16 udp_port, const char dev_name[])
+                    ovs_be16 tp_port, const char dev_name[], const char type[])
 {
     struct tnl_port *p;
     struct ip_device *ip_dev;
 
     ovs_mutex_lock(&mutex);
     LIST_FOR_EACH(p, node, &port_list) {
-        if (udp_port == p->udp_port) {
+        if (tp_port == p->tp_port) {
              goto out;
         }
     }
 
     p = xzalloc(sizeof *p);
     p->port = port;
-    p->udp_port = udp_port;
+    p->tp_port = tp_port;
     ovs_strlcpy(p->dev_name, dev_name, sizeof p->dev_name);
+    ovs_strlcpy(p->type, type, sizeof p->dev_name);
     list_insert(&port_list, &p->node);
 
     LIST_FOR_EACH(ip_dev, node, &addr_list) {
         if (ip_dev->addr4 != INADDR_ANY) {
             struct in6_addr addr4 = in6_addr_mapped_ipv4(ip_dev->addr4);
             map_insert(p->port, ip_dev->mac, &addr4,
-                       p->udp_port, p->dev_name);
+                       p->tp_port, p->dev_name, p->type);
         }
         if (ipv6_addr_is_set(&ip_dev->addr6)) {
             map_insert(p->port, ip_dev->mac, &ip_dev->addr6,
-                       p->udp_port, p->dev_name);
+                       p->tp_port, p->dev_name, p->type);
         }
     }
 
@@ -199,19 +205,20 @@  tnl_port_unref(const struct cls_rule *cr)
 }
 
 static void
-map_delete(struct eth_addr mac, struct in6_addr *addr, ovs_be16 udp_port)
+map_delete(struct eth_addr mac, struct in6_addr *addr,
+           ovs_be16 tp_port, const char type[])
 {
     const struct cls_rule *cr;
     struct flow flow;
 
-    tnl_port_init_flow(&flow, mac, addr, udp_port);
+    tnl_port_init_flow(&flow, mac, addr, tp_port, type);
 
     cr = classifier_lookup(&cls, CLS_MAX_VERSION, &flow, NULL);
     tnl_port_unref(cr);
 }
 
 void
-tnl_port_map_delete(ovs_be16 udp_port)
+tnl_port_map_delete(ovs_be16 tp_port)
 {
     struct tnl_port *p, *next;
     struct ip_device *ip_dev;
@@ -219,7 +226,7 @@  tnl_port_map_delete(ovs_be16 udp_port)
 
     ovs_mutex_lock(&mutex);
     LIST_FOR_EACH_SAFE(p, next, node, &port_list) {
-        if (p->udp_port == udp_port) {
+        if (p->tp_port == tp_port) {
             list_remove(&p->node);
             found = true;
             break;
@@ -232,10 +239,10 @@  tnl_port_map_delete(ovs_be16 udp_port)
     LIST_FOR_EACH(ip_dev, node, &addr_list) {
         if (ip_dev->addr4 != INADDR_ANY) {
             struct in6_addr addr4 = in6_addr_mapped_ipv4(ip_dev->addr4);
-            map_delete(ip_dev->mac, &addr4, udp_port);
+            map_delete(ip_dev->mac, &addr4, tp_port, p->type);
         }
         if (ipv6_addr_is_set(&ip_dev->addr6)) {
-            map_delete(ip_dev->mac, &ip_dev->addr6, udp_port);
+            map_delete(ip_dev->mac, &ip_dev->addr6, tp_port, p->type);
         }
     }
 
@@ -334,11 +341,11 @@  map_insert_ipdev(struct ip_device *ip_dev)
         if (ip_dev->addr4 != INADDR_ANY) {
             struct in6_addr addr4 = in6_addr_mapped_ipv4(ip_dev->addr4);
             map_insert(p->port, ip_dev->mac, &addr4,
-                       p->udp_port, p->dev_name);
+                       p->tp_port, p->dev_name, p->type);
         }
         if (ipv6_addr_is_set(&ip_dev->addr6)) {
             map_insert(p->port, ip_dev->mac, &ip_dev->addr6,
-                       p->udp_port, p->dev_name);
+                       p->tp_port, p->dev_name, p->type);
         }
     }
 }
@@ -391,10 +398,10 @@  delete_ipdev(struct ip_device *ip_dev)
     LIST_FOR_EACH(p, node, &port_list) {
         if (ip_dev->addr4 != INADDR_ANY) {
             struct in6_addr addr4 = in6_addr_mapped_ipv4(ip_dev->addr4);
-            map_delete(ip_dev->mac, &addr4, p->udp_port);
+            map_delete(ip_dev->mac, &addr4, p->tp_port, p->type);
         }
         if (ipv6_addr_is_set(&ip_dev->addr6)) {
-            map_delete(ip_dev->mac, &ip_dev->addr6, p->udp_port);
+            map_delete(ip_dev->mac, &ip_dev->addr6, p->tp_port, p->type);
         }
     }
 
diff --git a/lib/tnl-ports.h b/lib/tnl-ports.h
index 4195e6a..8dbee06 100644
--- a/lib/tnl-ports.h
+++ b/lib/tnl-ports.h
@@ -26,8 +26,8 @@ 
 
 odp_port_t tnl_port_map_lookup(struct flow *flow, struct flow_wildcards *wc);
 
-void tnl_port_map_insert(odp_port_t port, ovs_be16 udp_port,
-                         const char dev_name[]);
+void tnl_port_map_insert(odp_port_t port, ovs_be16 tp_port,
+                         const char dev_name[], const char type[]);
 
 void tnl_port_map_delete(ovs_be16 udp_port);
 void tnl_port_map_insert_ipdev(const char dev[]);
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 7ac8841..7aa0a05 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -194,7 +194,10 @@  tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev,
     tnl_port_mod_log(tnl_port, "adding");
 
     if (native_tnl) {
-        tnl_port_map_insert(odp_port, cfg->dst_port, name);
+        const char *type;
+
+        type = netdev_get_type(netdev);
+        tnl_port_map_insert(odp_port, cfg->dst_port, name, type);
     }
     return true;
 }