Re: [PATCH v3 net-next 7/8] geneve: Consolidate Geneve functionality in single module.

On Tue, Aug 25, 2015 at 2:35 PM, Jesse Gross [off-list ref] wrote:

On Tue, Aug 25, 2015 at 1:54 PM, Pravin Shelar [off-list ref] wrote:

quoted

On Tue, Aug 25, 2015 at 12:03 PM, Jesse Gross [off-list ref] wrote:

quoted

On Mon, Aug 24, 2015 at 10:43 AM, Pravin B Shelar [off-list ref] wrote:

quoted

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index c05bc13..8eb875d 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c

@@ -492,36 +813,36 @@ static int geneve_configure(struct net *net, struct net_device *dev,

[...]

quoted

+       gs = geneve_find_sock(gn, geneve->dst_port);
+       if (gs) {
+               if (metadata) {
+                       if (gs->collect_md)
+                               return -EEXIST;
+                       else
+                               return -EPERM;
+               } else {
+                       if (gs->collect_md)
+                               return -EPERM;
+
+                       t = geneve_lookup(gn, htons(dst_port),
+                                         rem_addr, geneve->vni);
+                       if (t)
+                               return -EBUSY;
+               }
+       }

I like the new structure but unfortunately, I think there is a race.
If two devices are created with conflicting configurations but neither
is brought up then creation of both devices will succeed. However,
when the second one is brought up, it will silently collide with the
first.

geneve tunnel is added to hash table during configure time. So the
lookup does not have any dependency on device up or down state. The
Lookup and hash table updates are done under rtnl lock.

But the check for duplicates is contingent on finding a socket. If we
configure two devices before calling geneve_open(), then there won't
be a socket yet and therefore no check.

ah.. ok. I have to traverse the geneve tunnel list here.