Hi Harald,
On 7/13/17, 12:35 AM, "Harald Welte"
<laforge(a)gnumonks.org> wrote:
+static int gtp_dev_open(struct net_device
*dev)
+{
+ struct gtp_dev *gtp = netdev_priv(dev);
+ struct net *net = gtp->net;
+ struct socket *sock1u;
+ struct socket *sock0;
+ struct udp_tunnel_sock_cfg tunnel_cfg;
+ struct udp_port_cfg udp_conf;
+ int err;
+
+ memset(&udp_conf, 0, sizeof(udp_conf));
+
+ udp_conf.family = AF_INET;
+ udp_conf.local_ip.s_addr = htonl(INADDR_ANY);
+ udp_conf.local_udp_port = htons(GTP1U_PORT);
+
+ err = udp_sock_create(gtp->net, &udp_conf, &sock1u);
+ if (err < 0)
+ return err;
+
+ udp_conf.local_udp_port = htons(GTP0_PORT);
+ err = udp_sock_create(gtp->net, &udp_conf, &sock0);
+ if (err < 0)
+ return err;
you're unconditionally binding to both GTP0 and GTP1 UDP ports. This is
done selectively based on netlink attributes in the existing "normal"
non-OVS kernel code, i.e. the control is left to the user.
Is this function is only called/used in the context of OVS? If so,
since you explicitly implement only GTPv1, why bind to GTPv0 port?
I had doubts on how this flow-based GTPv1 code path should fit in, which is why
the GTPv0 and the GTPv1 code pieces are mixed in my changes. Should I
explicitly claim that the flow-based change is for GTPv1 only?
+ setup_udp_tunnel_sock(net, sock1u, &tunnel_cfg);
even here, you're only setting up the v1 and not v0.
same reason as above.
+ /* Assume
largest header, ie. GTPv0. */
+ dev->needed_headroom = LL_MAX_HEADER +
+ sizeof(struct iphdr) +
+ sizeof(struct udphdr) +
+ sizeof(struct gtp0_header);
... and here you're using headroom for a GTPv0 header, despite (I think)
only supporting GTPv1 from this configuration?
Yes, only GTPv1 is supported.
+ err = gtp_hashtable_new(gtp, GTP_PDP_HASHSIZE);
// JO: when to free??
I think that question about when to free needs to be resolved before any
merge. Did you check that it persists even after the device is
closed/removed?
I didn't investigate it. What do you mean by persist?
Thanks
-Jiannan