[05/16] gtp-rtnl: and netns support
diff mbox

Message ID 1447686417-3979-6-git-send-email-aschultz@tpip.net
State Changes Requested
Delegated to: Pablo Neira
Headers show

Commit Message

Andreas Schultz Nov. 16, 2015, 3:06 p.m. UTC
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 libgtnl/include/libgtpnl/gtp.h   |  2 ++
 libgtnl/include/libgtpnl/gtpnl.h |  2 +-
 libgtnl/include/linux/gtp_nl.h   |  1 +
 libgtnl/src/gtp-genl.c           |  2 ++
 libgtnl/src/gtp-rtnl.c           |  8 +++++++-
 libgtnl/src/gtp.c                | 12 ++++++++++++
 libgtnl/src/internal.h           |  1 +
 libgtnl/src/libgtpnl.map         |  2 ++
 8 files changed, 28 insertions(+), 2 deletions(-)

Comments

Pablo Neira Ayuso Nov. 16, 2015, 5:43 p.m. UTC | #1
On Mon, Nov 16, 2015 at 04:06:46PM +0100, Andreas Schultz wrote:
> diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c
> index c1f60ab..9e68a30 100644
> --- a/libgtnl/src/gtp-genl.c
> +++ b/libgtnl/src/gtp-genl.c
> @@ -44,6 +44,8 @@
>  static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t)
>  {
>  	mnl_attr_put_u32(nlh, GTPA_VERSION, t->gtp_version);
> +	if (t->ifns > 0)
> +		mnl_attr_put_u32(nlh, GTPA_NET_NS_FD, t->ifns);

Any reason not to consider descriptor zero as valid?

I guess we'll need some flags for gtp_tunnel so we know what we have
set here.

>  	mnl_attr_put_u32(nlh, GTPA_LINK, t->ifidx);
>  	mnl_attr_put_u32(nlh, GTPA_SGSN_ADDRESS, t->sgsn_addr.s_addr);
>  	mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.s_addr);
> diff --git a/libgtnl/src/gtp-rtnl.c b/libgtnl/src/gtp-rtnl.c
> index 22b9430..db54653 100644
> --- a/libgtnl/src/gtp-rtnl.c
> +++ b/libgtnl/src/gtp-rtnl.c
> @@ -38,6 +38,10 @@
>  
>  #include "internal.h"
>  
> +#if !defined(IFLA_LINK_NETNSID)
> +#define IFLA_LINK_NETNSID (IFLA_PHYS_SWITCH_ID + 1)
> +#endif

Why do you need this?
Andreas Schultz Nov. 17, 2015, 10:09 a.m. UTC | #2
----- Original Message -----
> From: "Pablo Neira Ayuso" <pablo@soleta.eu>
> To: "Andreas Schultz" <aschultz@tpip.net>
> Cc: openbsc@lists.osmocom.org, "Harald Welte" <laforge@gnumonks.org>
> Sent: Monday, November 16, 2015 6:43:55 PM
> Subject: Re: [PATCH 05/16] gtp-rtnl: and netns support

> On Mon, Nov 16, 2015 at 04:06:46PM +0100, Andreas Schultz wrote:
>> diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c
>> index c1f60ab..9e68a30 100644
>> --- a/libgtnl/src/gtp-genl.c
>> +++ b/libgtnl/src/gtp-genl.c
>> @@ -44,6 +44,8 @@
>>  static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t)
>>  {
>>  	mnl_attr_put_u32(nlh, GTPA_VERSION, t->gtp_version);
>> +	if (t->ifns > 0)
>> +		mnl_attr_put_u32(nlh, GTPA_NET_NS_FD, t->ifns);
> 
> Any reason not to consider descriptor zero as valid?

ifns is a file descriptor and fd's are as far as I know always greater zero.
So, I didn't see a reason to permit zero there when simply not setting the
attribute would serve the same purpose.

> I guess we'll need some flags for gtp_tunnel so we know what we have
> set here.
> 
>>  	mnl_attr_put_u32(nlh, GTPA_LINK, t->ifidx);
>>  	mnl_attr_put_u32(nlh, GTPA_SGSN_ADDRESS, t->sgsn_addr.s_addr);
>>  	mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.s_addr);
>> diff --git a/libgtnl/src/gtp-rtnl.c b/libgtnl/src/gtp-rtnl.c
>> index 22b9430..db54653 100644
>> --- a/libgtnl/src/gtp-rtnl.c
>> +++ b/libgtnl/src/gtp-rtnl.c
>> @@ -38,6 +38,10 @@
>>  
>>  #include "internal.h"
>>  
>> +#if !defined(IFLA_LINK_NETNSID)
>> +#define IFLA_LINK_NETNSID (IFLA_PHYS_SWITCH_ID + 1)
>> +#endif
> 
> Why do you need this?

I'm test compiling the library on an older kernel. I'll remove that.
Pablo Neira Ayuso Nov. 17, 2015, 12:31 p.m. UTC | #3
On Tue, Nov 17, 2015 at 11:09:46AM +0100, Andreas Schultz wrote:
> ----- Original Message -----
> > From: "Pablo Neira Ayuso" <pablo@soleta.eu>
> > To: "Andreas Schultz" <aschultz@tpip.net>
> > Cc: openbsc@lists.osmocom.org, "Harald Welte" <laforge@gnumonks.org>
> > Sent: Monday, November 16, 2015 6:43:55 PM
> > Subject: Re: [PATCH 05/16] gtp-rtnl: and netns support
> 
> > On Mon, Nov 16, 2015 at 04:06:46PM +0100, Andreas Schultz wrote:
> >> diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c
> >> index c1f60ab..9e68a30 100644
> >> --- a/libgtnl/src/gtp-genl.c
> >> +++ b/libgtnl/src/gtp-genl.c
> >> @@ -44,6 +44,8 @@
> >>  static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t)
> >>  {
> >>  	mnl_attr_put_u32(nlh, GTPA_VERSION, t->gtp_version);
> >> +	if (t->ifns > 0)
> >> +		mnl_attr_put_u32(nlh, GTPA_NET_NS_FD, t->ifns);
> > 
> > Any reason not to consider descriptor zero as valid?
> 
> ifns is a file descriptor and fd's are as far as I know always greater zero.
> So, I didn't see a reason to permit zero there when simply not setting the
> attribute would serve the same purpose.

You close(0) and then allocate a descriptor via open(...) and see what
it happens.

#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

int main(void)
{
        int fd;

        close(0);

        fd = open("/etc/passwd", O_RDONLY);
        printf("fd = %d\n", fd);
}

Patch
diff mbox

diff --git a/libgtnl/include/libgtpnl/gtp.h b/libgtnl/include/libgtpnl/gtp.h
index 10b34d5..fa09d2a 100644
--- a/libgtnl/include/libgtpnl/gtp.h
+++ b/libgtnl/include/libgtpnl/gtp.h
@@ -8,6 +8,7 @@  struct gtp_tunnel;
 struct gtp_tunnel *gtp_tunnel_alloc(void);
 void gtp_tunnel_free(struct gtp_tunnel *t);
 
+void gtp_tunnel_set_ifns(struct gtp_tunnel *t, int ifns);
 void gtp_tunnel_set_ifidx(struct gtp_tunnel *t, uint32_t ifidx);
 void gtp_tunnel_set_ms_ip4(struct gtp_tunnel *t, struct in_addr *ms_addr);
 void gtp_tunnel_set_sgsn_ip4(struct gtp_tunnel *t, struct in_addr *sgsn_addr);
@@ -15,6 +16,7 @@  void gtp_tunnel_set_version(struct gtp_tunnel *t, uint32_t version);
 void gtp_tunnel_set_tid(struct gtp_tunnel *t, uint64_t tid);
 void gtp_tunnel_set_flowid(struct gtp_tunnel *t, uint16_t flowid);
 
+const int gtp_tunnel_get_ifns(struct gtp_tunnel *t);
 const uint32_t gtp_tunnel_get_ifidx(struct gtp_tunnel *t);
 const struct in_addr *gtp_tunnel_get_ms_ip4(struct gtp_tunnel *t);
 const struct in_addr *gtp_tunnel_get_sgsn_ip4(struct gtp_tunnel *t);
diff --git a/libgtnl/include/libgtpnl/gtpnl.h b/libgtnl/include/libgtpnl/gtpnl.h
index c4faf6c..3d3fd73 100644
--- a/libgtnl/include/libgtpnl/gtpnl.h
+++ b/libgtnl/include/libgtpnl/gtpnl.h
@@ -16,7 +16,7 @@  int genl_lookup_family(struct mnl_socket *nl, const char *family);
 
 struct in_addr;
 
-int gtp_dev_create(const char *gtp_ifname, const char *real_ifname,
+int gtp_dev_create(int dest_ns, const char *gtp_ifname, const char *real_ifname,
 		   int fd0, int fd1);
 int gtp_dev_config(const char *iface, struct in_addr *net, uint32_t prefix);
 int gtp_dev_destroy(const char *gtp_ifname);
diff --git a/libgtnl/include/linux/gtp_nl.h b/libgtnl/include/linux/gtp_nl.h
index 0a28046..a8fdf3a 100644
--- a/libgtnl/include/linux/gtp_nl.h
+++ b/libgtnl/include/linux/gtp_nl.h
@@ -40,6 +40,7 @@  enum gtp_attrs {
 	GTPA_SGSN_ADDRESS,
 	GTPA_MS_ADDRESS,
 	GTPA_FLOWID,	/* only for GTPv0 */
+	GTPA_NET_NS_FD,
 	__GTPA_MAX,
 };
 #define GTPA_MAX (__GTPA_MAX + 1)
diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c
index c1f60ab..9e68a30 100644
--- a/libgtnl/src/gtp-genl.c
+++ b/libgtnl/src/gtp-genl.c
@@ -44,6 +44,8 @@ 
 static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t)
 {
 	mnl_attr_put_u32(nlh, GTPA_VERSION, t->gtp_version);
+	if (t->ifns > 0)
+		mnl_attr_put_u32(nlh, GTPA_NET_NS_FD, t->ifns);
 	mnl_attr_put_u32(nlh, GTPA_LINK, t->ifidx);
 	mnl_attr_put_u32(nlh, GTPA_SGSN_ADDRESS, t->sgsn_addr.s_addr);
 	mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.s_addr);
diff --git a/libgtnl/src/gtp-rtnl.c b/libgtnl/src/gtp-rtnl.c
index 22b9430..db54653 100644
--- a/libgtnl/src/gtp-rtnl.c
+++ b/libgtnl/src/gtp-rtnl.c
@@ -38,6 +38,10 @@ 
 
 #include "internal.h"
 
+#if !defined(IFLA_LINK_NETNSID)
+#define IFLA_LINK_NETNSID (IFLA_PHYS_SWITCH_ID + 1)
+#endif
+
 static struct nlmsghdr *
 gtp_put_nlmsg(char *buf, uint16_t type, uint16_t nl_flags, uint32_t seq)
 {
@@ -104,7 +108,7 @@  static int gtp_dev_talk(struct nlmsghdr *nlh, uint32_t seq)
 	return ret;
 }
 
-int gtp_dev_create(const char *gtp_ifname, const char *real_ifname,
+int gtp_dev_create(int dest_ns, const char *gtp_ifname, const char *real_ifname,
 		   int fd0, int fd1)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
@@ -120,6 +124,8 @@  int gtp_dev_create(const char *gtp_ifname, const char *real_ifname,
 	ifm->ifi_change |= IFF_UP;
 	ifm->ifi_flags |= IFF_UP;
 
+	if (dest_ns > 0)
+		mnl_attr_put_u32(nlh, IFLA_NET_NS_FD, dest_ns);
 	mnl_attr_put_u32(nlh, IFLA_LINK, if_nametoindex(real_ifname));
 	mnl_attr_put_str(nlh, IFLA_IFNAME, gtp_ifname);
 	nest = mnl_attr_nest_start(nlh, IFLA_LINKINFO);
diff --git a/libgtnl/src/gtp.c b/libgtnl/src/gtp.c
index 4534091..6e3d473 100644
--- a/libgtnl/src/gtp.c
+++ b/libgtnl/src/gtp.c
@@ -39,6 +39,12 @@  void gtp_tunnel_free(struct gtp_tunnel *t)
 }
 EXPORT_SYMBOL(gtp_tunnel_free);
 
+void gtp_tunnel_set_ifns(struct gtp_tunnel *t, int ifns)
+{
+	t->ifns = ifns;
+}
+EXPORT_SYMBOL(gtp_tunnel_set_ifns);
+
 void gtp_tunnel_set_ifidx(struct gtp_tunnel *t, uint32_t ifidx)
 {
 	t->ifidx = ifidx;
@@ -75,6 +81,12 @@  void gtp_tunnel_set_flowid(struct gtp_tunnel *t, uint16_t flowid)
 }
 EXPORT_SYMBOL(gtp_tunnel_set_flowid);
 
+const int gtp_tunnel_get_ifns(struct gtp_tunnel *t)
+{
+	return t->ifns;
+}
+EXPORT_SYMBOL(gtp_tunnel_get_ifns);
+
 const uint32_t gtp_tunnel_get_ifidx(struct gtp_tunnel *t)
 {
 	return t->ifidx;
diff --git a/libgtnl/src/internal.h b/libgtnl/src/internal.h
index 75b3954..68f0135 100644
--- a/libgtnl/src/internal.h
+++ b/libgtnl/src/internal.h
@@ -13,6 +13,7 @@ 
 #include <netinet/in.h>
 
 struct gtp_tunnel {
+	int             ifns;
 	uint32_t	ifidx;
 	struct in_addr	ms_addr;
 	struct in_addr	sgsn_addr;
diff --git a/libgtnl/src/libgtpnl.map b/libgtnl/src/libgtpnl.map
index 6e69ef8..2368467 100644
--- a/libgtnl/src/libgtpnl.map
+++ b/libgtnl/src/libgtpnl.map
@@ -15,12 +15,14 @@  global:
 
   gtp_tunnel_alloc;
   gtp_tunnel_free;
+  gtp_tunnel_set_ifns;
   gtp_tunnel_set_ifidx;
   gtp_tunnel_set_ms_ip4;
   gtp_tunnel_set_sgsn_ip4;
   gtp_tunnel_set_version;
   gtp_tunnel_set_tid;
   gtp_tunnel_set_flowid;
+  gtp_tunnel_get_ifns;
   gtp_tunnel_get_ifidx;
   gtp_tunnel_get_ms_ip4;
   gtp_tunnel_get_sgsn_ip4;