Re: [PATCH net-next 2/4] gtp: add genl cmd to enable GTP encapsulation on UDP socket
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: 2017-03-14 11:43:40
On Tue, Mar 14, 2017 at 12:25:46PM +0100, Andreas Schultz wrote:
quoted hunk ↗ jump to hunk
@@ -1254,6 +1293,8 @@ static struct nla_policy gtp_genl_policy[GTPA_MAX + 1] = { [GTPA_NET_NS_FD] = { .type = NLA_U32, }, [GTPA_I_TEI] = { .type = NLA_U32, }, [GTPA_O_TEI] = { .type = NLA_U32, }, + [GTPA_PDP_HASHSIZE] = { .type = NLA_U32, },
This per PDP hashsize attribute clearly doesn't belong here. Moreover, we now have a rhashtable implementation, so we hopefully we can get rid of this. It should be very easy to convert this to use rhashtable, and it is very much desiderable.
+ [GTPA_FD] = { .type = NLA_U32, },This new atttribute has nothing to do with the PDP context. And enum gtp_attrs *only* describe a PDP context. Adding more attributes there to mix semantics is not the way to go. You likely have to inaugurate a new enum. This gtp_attrs enum only related to the PDP description. Why not add some interface to attach more sockets to the gtp device globally? So still the gtp device is the top-level structure. Then add a netlink attribute to specify to what VRF this tunnel belongs to, instead of implicitly using the socket to achieve this. Another possibility is to explicitly have an interface to add new/delete VRFs, attach sockets to them. In general, I'm still not convinced this is the right design for this.