Netdev List
 help / color / mirror / Atom feed
* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
From: Neil Horman @ 2017-01-23 14:53 UTC (permalink / raw)
  To: David Laight
  Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
	Marcelo Ricardo Leitner, Vlad Yasevich, davem@davemloft.net
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026B563@AcuExch.aculab.com>

On Mon, Jan 23, 2017 at 11:25:56AM +0000, David Laight wrote:
> From: Xin Long
> > Sent: 19 January 2017 17:19
> > This patch is to implement Sender-Side Procedures for the Add
> > Outgoing and Incoming Streams Request Parameter described in
> > rfc6525 section 5.1.5-5.1.6.
> > 
> > It is also to add sockopt SCTP_ADD_STREAMS in rfc6525 section
> > 6.3.4 for users.
> ...
> > +	out = params->sas_outstrms;
> > +	in  = params->sas_instrms;
> > +
> > +	if (!out && !in)
> > +		goto out;
> > +
> > +	if (out) {
> > +		__u16 nums = stream->outcnt + out;
> 
> Make nums 'unsigned int', the code will be smaller and you can
> use the value for the overflow check.
> 
> > +		/* Check for overflow, can't use nums here */
> > +		if (stream->outcnt + out > SCTP_MAX_STREAM)
> > +			goto out;
> > +
> > +		/* Use ksize to check if stream array really needs to realloc */
> > +		if (ksize(stream->out) / sizeof(*stream->out) < nums) {
> > +			struct sctp_stream_out *streamout;
> > +
> > +			streamout = kcalloc(nums, sizeof(*streamout),
> > +					    GFP_KERNEL);
> > +			if (!streamout) {
> > +				retval = -ENOMEM;
> > +				goto out;
> > +			}
> > +
> > +			memcpy(streamout, stream->out,
> > +			       sizeof(*streamout) * stream->outcnt);
> > +
> > +			kfree(stream->out);
> > +			stream->out = streamout;
> > +		}
> 
> Does kcalloc() zero the entire area, or just the length you ask for?
> If the latter you need to zero the rest here.
Better still, just use krealloc.  You still need to zero out any space beyond
the old length, but it will make the code shorter, and avoid the need for
additional temporary variables.

Neil

> ...
> 
> 	David
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
From: Neil Horman @ 2017-01-23 14:50 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Xin Long, network dev, linux-sctp, Vlad Yasevich, davem
In-Reply-To: <20170119221812.GP3781@localhost.localdomain>

On Thu, Jan 19, 2017 at 08:18:13PM -0200, Marcelo Ricardo Leitner wrote:
> On Thu, Jan 19, 2017 at 03:17:18PM -0500, Neil Horman wrote:
> > On Fri, Jan 20, 2017 at 01:19:14AM +0800, Xin Long wrote:
> > > This patch is to implement Sender-Side Procedures for the Add
> > > Outgoing and Incoming Streams Request Parameter described in
> > > rfc6525 section 5.1.5-5.1.6.
> > > 
> > > It is also to add sockopt SCTP_ADD_STREAMS in rfc6525 section
> > > 6.3.4 for users.
> > > 
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > >  include/net/sctp/sctp.h   |  2 +
> > >  include/uapi/linux/sctp.h |  7 ++++
> > >  net/sctp/socket.c         | 29 ++++++++++++++
> > >  net/sctp/stream.c         | 99 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 137 insertions(+)
> > > 
> > > diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> > > index b93820f..68ee1a6 100644
> > > --- a/include/net/sctp/sctp.h
> > > +++ b/include/net/sctp/sctp.h
> > > @@ -199,6 +199,8 @@ int sctp_offload_init(void);
> > >  int sctp_send_reset_streams(struct sctp_association *asoc,
> > >  			    struct sctp_reset_streams *params);
> > >  int sctp_send_reset_assoc(struct sctp_association *asoc);
> > > +int sctp_send_add_streams(struct sctp_association *asoc,
> > > +			  struct sctp_add_streams *params);
> > >  
> > >  /*
> > >   * Module global variables
> > > diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> > > index c0bd8c3..a91a9cc 100644
> > > --- a/include/uapi/linux/sctp.h
> > > +++ b/include/uapi/linux/sctp.h
> > > @@ -118,6 +118,7 @@ typedef __s32 sctp_assoc_t;
> > >  #define SCTP_ENABLE_STREAM_RESET	118
> > >  #define SCTP_RESET_STREAMS	119
> > >  #define SCTP_RESET_ASSOC	120
> > > +#define SCTP_ADD_STREAMS	121
> > >  
> > >  /* PR-SCTP policies */
> > >  #define SCTP_PR_SCTP_NONE	0x0000
> > > @@ -1027,4 +1028,10 @@ struct sctp_reset_streams {
> > >  	uint16_t srs_stream_list[];	/* list if srs_num_streams is not 0 */
> > >  };
> > >  
> > > +struct sctp_add_streams {
> > > +	sctp_assoc_t sas_assoc_id;
> > > +	uint16_t sas_instrms;
> > > +	uint16_t sas_outstrms;
> > > +};
> > > +
> > >  #endif /* _UAPI_SCTP_H */
> > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > index 2c5c9ca..ae0a99e 100644
> > > --- a/net/sctp/socket.c
> > > +++ b/net/sctp/socket.c
> > > @@ -3838,6 +3838,32 @@ static int sctp_setsockopt_reset_assoc(struct sock *sk,
> > >  	return retval;
> > >  }
> > >  
> > > +static int sctp_setsockopt_add_streams(struct sock *sk,
> > > +				       char __user *optval,
> > > +				       unsigned int optlen)
> > > +{
> > you are going to need to provide some locking here or in sctp_send_add_streams.
> > By replacing the in/out streams pointer you run the risk of multiple callers
> > modifying the pointers in parallel, or in the sctp state machine reading it
> > while you do.
> 
> There can't be multiple callers here because the socket is locked before
> calling this function, in sctp_setsockopt().
> I also don't see how a BH handler, timers or sleeping call (recvmsg)
> could trip on this.
You're right, I missed the socket lock in sctp_setsockopt, that covers us both
from multiple processes and bottom halves.

> 
>   Marcelo
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Urgent Please,,
From: Joyes Dadi @ 2017-01-23 14:11 UTC (permalink / raw)


Good Day Dear,

My name is Ms. Joyes Dadi, I am glad you are reading this letter and I hope
we will start our communication and I know that this message will look strange,
surprising and probably unbelievable to you, but it is the reality. I want to
make a donation of money to you.

I contact you by the will of God. I am a firm German woman specialized in
mining gold and diamonds in Africa. But now, I'm very sick of a cancer. My
husband died in an accident two years ago with our two children and now I have
cancer of the esophagus that damaged almost all the cells in my system/agencies
and I'll die soon according to my doctor.

My most concern now is, we grew up in the orphanage and were married in
orphanage. If I die this deposited fund will soon be left alone in the hand of
the bank, and I do want to it that  way. Please, if you can be reliable and
sincere to accept my humble proposal; I have (10.5Millions Euro) in a fixed
deposit account; I will order the Bank to transfer the money into your account
in your country immediately, and then you will take the fund to  your country
and invest it to the orphanage homes Please, answer as quickly as possible.

God bless you.
Ms. Joyes Dadi
Email: joyesdadi767@citromail.hu

^ permalink raw reply

* wrong smp_mb__after_atomic() in tcp_check_space() ?
From: Oleg Nesterov @ 2017-01-23 14:30 UTC (permalink / raw)
  To: Jason Baron, David S. Miller; +Cc: Herbert Xu, Yauheni Kaliuta, netdev

Hello,

smp_mb__after_atomic() looks wrong and misleading, sock_reset_flag() does the
non-atomic __clear_bit() and thus it can not guarantee test_bit(SOCK_NOSPACE)
(non-atomic too) won't be reordered.

It was added by 3c7151275c0c9a "tcp: add memory barriers to write space paths"
and the patch looks correct in that we need the barriers in tcp_check_space()
and tcp_poll() in theory, so it seems tcp_check_space() needs smp_mb() ?

Oleg.

^ permalink raw reply

* [PATCH] net: ethernet: mvneta: add support for 2.5G DRSGMII mode
From: Jan Luebbe @ 2017-01-23 14:22 UTC (permalink / raw)
  To: netdev, devicetree, davem
  Cc: Rob Herring, Mark Rutland, Thomas Petazzoni, Florian Fainelli,
	Jan Luebbe

The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
called DRSGMII.

This patch adds a corresponding phy-mode string 'drsgmii' and parses it
from DT. The MVNETA then configures the SERDES protocol value
accordingly.

It was successfully tested on a MV78460 connected to a FPGA.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
---
 Documentation/devicetree/bindings/net/ethernet.txt | 1 +
 drivers/net/ethernet/marvell/mvneta.c              | 5 +++++
 include/linux/phy.h                                | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 05150957ecfd..de40c5977d8f 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -29,6 +29,7 @@ The following properties are common to the Ethernet controllers:
   * "smii"
   * "xgmii"
   * "trgmii"
+  * "drsgmii"
 - phy-connection-type: the same as "phy-mode" property but described in ePAPR;
 - phy-handle: phandle, specifies a reference to a node representing a PHY
   device; this property is described in ePAPR and so preferred;
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index e05e22705cf7..8cb43e0d9d0e 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -105,6 +105,7 @@
 #define MVNETA_SERDES_CFG			 0x24A0
 #define      MVNETA_SGMII_SERDES_PROTO		 0x0cc7
 #define      MVNETA_QSGMII_SERDES_PROTO		 0x0667
+#define      MVNETA_DRSGMII_SERDES_PROTO	 0x1107
 #define MVNETA_TYPE_PRIO                         0x24bc
 #define      MVNETA_FORCE_UNI                    BIT(21)
 #define MVNETA_TXQ_CMD_1                         0x24e4
@@ -4047,6 +4048,10 @@ static int mvneta_port_power_up(struct mvneta_port *pp, int phy_mode)
 	 * SGMII or QSGMII mode, the RGMII bit needs to be set.
 	 */
 	switch(phy_mode) {
+	case PHY_INTERFACE_MODE_DRSGMII:
+		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_DRSGMII_SERDES_PROTO);
+		ctrl |= MVNETA_GMAC2_PCS_ENABLE | MVNETA_GMAC2_PORT_RGMII;
+		break;
 	case PHY_INTERFACE_MODE_QSGMII:
 		mvreg_write(pp, MVNETA_SERDES_CFG, MVNETA_QSGMII_SERDES_PROTO);
 		ctrl |= MVNETA_GMAC2_PCS_ENABLE | MVNETA_GMAC2_PORT_RGMII;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index f7d95f644eed..a3d83bc96035 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -82,6 +82,7 @@ typedef enum {
 	PHY_INTERFACE_MODE_MOCA,
 	PHY_INTERFACE_MODE_QSGMII,
 	PHY_INTERFACE_MODE_TRGMII,
+	PHY_INTERFACE_MODE_DRSGMII,
 	PHY_INTERFACE_MODE_MAX,
 } phy_interface_t;
 
@@ -142,6 +143,8 @@ static inline const char *phy_modes(phy_interface_t interface)
 		return "qsgmii";
 	case PHY_INTERFACE_MODE_TRGMII:
 		return "trgmii";
+	case PHY_INTERFACE_MODE_DRSGMII:
+		return "drsgmii";
 	default:
 		return "unknown";
 	}
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v1 0/4]crypto:chcr- Bug Fixes for 4.10
From: Herbert Xu @ 2017-01-23 13:39 UTC (permalink / raw)
  To: Harsh Jain; +Cc: hariprasad, netdev, linux-crypto
In-Reply-To: <cover.1484309965.git.harsh@chelsio.com>

On Fri, Jan 13, 2017 at 05:59:19PM +0530, Harsh Jain wrote:
> This patch series is based on Herbert's cryptodev-2.6 tree.
> It includes several critical bug fixes.
> 
> Atul Gupta (3):
>   crypto:chcr-Change flow IDs
>   crypto:chcr- Fix panic on dma_unmap_sg
>   crypto:chcr- Check device is allocated before use
> Julia Lawall (1):
>   crypto:chcr-fix itnull.cocci warnings

Patches 2 and 3 look critical, but the other ones do not.  Please
don't mix fixes targeted for this cycle with other patches.

Please resubmit them as two separate series.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [patch] samples/bpf: silence shift wrapping warning
From: Arnaldo Carvalho de Melo @ 2017-01-23 13:27 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Dan Carpenter, Thomas Graf, Alexei Starovoitov, Joe Stringer,
	David S. Miller, linux-kernel, kernel-janitors, netdev
In-Reply-To: <20170122225123.GA73160@ast-mbp.thefacebook.com>

Em Sun, Jan 22, 2017 at 02:51:25PM -0800, Alexei Starovoitov escreveu:
> On Sat, Jan 21, 2017 at 07:51:43AM +0300, Dan Carpenter wrote:
> > max_key is a value in the 0-63 range, so on 32 bit systems the shift
> > could wrap.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Looks fine. I think 'net-next' is ok.

I could process these patches, if that would help,

- Arnaldo
 
> Acked-by: Alexei Starovoitov <ast@kernel.org>
 
> > diff --git a/samples/bpf/lwt_len_hist_user.c b/samples/bpf/lwt_len_hist_user.c
> > index ec8f3bb..bd06eef 100644
> > --- a/samples/bpf/lwt_len_hist_user.c
> > +++ b/samples/bpf/lwt_len_hist_user.c
> > @@ -68,7 +68,7 @@ int main(int argc, char **argv)
> >  	for (i = 1; i <= max_key + 1; i++) {
> >  		stars(starstr, data[i - 1], max_value, MAX_STARS);
> >  		printf("%8ld -> %-8ld : %-8ld |%-*s|\n",
> > -		       (1l << i) >> 1, (1l << i) - 1, data[i - 1],
> > +		       (1ULL << i) >> 1, (1ULL << i) - 1, data[i - 1],
> >  		       MAX_STARS, starstr);
> >  	}
> >  

^ permalink raw reply

* Re: [PATCH net-next v6 1/1] net sched actions: Add support for user cookies
From: Simon Horman @ 2017-01-23 12:58 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: davem, netdev, jiri, paulb, john.fastabend, mrv, hadarh, ogerlitz,
	roid, xiyou.wangcong, daniel
In-Reply-To: <1485116750-31198-1-git-send-email-jhs@emojatatu.com>

Hi Jamal,

On Sun, Jan 22, 2017 at 03:25:50PM -0500, Jamal Hadi Salim wrote:

...

> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index cd08df9..58cf1c5 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -24,6 +24,7 @@
>  #include <net/net_namespace.h>
>  #include <net/sock.h>
>  #include <net/sch_generic.h>
> +#include <net/pkt_cls.h>
>  #include <net/act_api.h>
>  #include <net/netlink.h>
>  
> @@ -33,6 +34,8 @@ static void free_tcf(struct rcu_head *head)
>  
>  	free_percpu(p->cpu_bstats);
>  	free_percpu(p->cpu_qstats);
> +	kfree(p->act_cookie->data);

Does the above need to be protected by a check for p->act_cookie being non-NULL?

> +	kfree(p->act_cookie);
>  	kfree(p);
>  }
>  

...

> @@ -575,6 +584,33 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
>  	if (err < 0)
>  		goto err_mod;
>  
> +	if (tb[TCA_ACT_COOKIE]) {
> +		int cklen = nla_len(tb[TCA_ACT_COOKIE]);
> +
> +		if (cklen > TC_COOKIE_MAX_SIZE) {
> +			err = -EINVAL;
> +			tcf_hash_release(a, bind);
> +			goto err_mod;
> +		}
> +
> +		a->act_cookie = kzalloc(sizeof(*a->act_cookie), GFP_KERNEL);
> +		if (!a->act_cookie) {
> +			err = -ENOMEM;
> +			tcf_hash_release(a, bind);
> +			goto err_mod;
> +		}
> +
> +		a->act_cookie->data = nla_memdup(tb[TCA_ACT_COOKIE],
> +						 GFP_KERNEL);
> +		if (!a->act_cookie->data) {
> +			err = -ENOMEM;
> +			kfree(a->act_cookie);
> +			tcf_hash_release(a, bind);
> +			goto err_mod;
> +		}
> +		a->act_cookie->len = cklen;

FWIW, the above looks correct but it also looks like the error handling
could be done less verbosely if the logic was moved to a separate function.

> +	}
> +
>  	/* module count goes up only when brand new policy is created
>  	 * if it exists and is only bound to in a_o->init() then
>  	 * ACT_P_CREATED is not returned (a zero is).
> -- 
> 1.9.1
> 

^ permalink raw reply

* Reference counting struct inet_peer
From: David Windsor @ 2017-01-23 12:42 UTC (permalink / raw)
  To: netdev, Kees Cook, Reshetova, Elena, Hans Liljestrand

Hi,

I'm working on a patchset that adds overflow protection to kernel
reference counters, as part of the KSPP effort.  We're introducing a
new type, tentatively called refcount_t, that will ultimately replace
atomic_t as the type used for kernel reference counters.  refcount_t
has a constrained interface relative to atomic_t and stores reference
counts as unsigned integers.

While performing an audit of kernel reference counters, we've come
upon a few corner cases that we're unable to cleanly migrate to
refcount_t.  One of these is the reference counting scheme for struct
inet_peer.

struct inet_peer objects get freed when their reference count becomes
-1, not 0 as is the usual case.  Is there a reason why this is so?
The common use case I'm seeing is this:

struct inet_peer *p = inet_getpeer_v[4|6]();
...
inet_putpeer(p);


inet_getpeer_v4() and inet_getpeer_v6() are wrappers around
inet_getpeer().  From inet_getpeer():

struct inet_peer *p;
...
p = lookup_rcu(daddr, base);
...
p = lookup(daddr, stack, base);
if (p != peer_avl_empty) {
    atomic_inc(&p->refcnt);
    write_sequnlock_bh(&base->lock);
    return p;
}
...
p = create ? kmem_cache_alloc(peer_cachep, GFP_ATOMIC) : NULL;
if (p) {
    ...
    atomic_set(&p->refcnt, 1);
    ...
}
return p;

This all looks straightforward: p->refcnt is incremented by one when
it is found in the peer node tree, and it is set to one when it is
newly created.

However, in lookup_rcu(), this same reference count is checked against
-1.  From lookup_rcu():

struct inet_peer *u = rcu_dereference(base->root);
...
/* Before taking a reference, check if this entry was
* deleted (refcnt=-1)
*/
if (!atomic_add_unless(&u->refcnt, 1, -1))
    u = NULL;
return u;

Rather than delve further into net's internal garbage collectors, or
into RCU internals, I figured I'd ask here if there's a reason for the
check against -1 in rcu_lookup().

We're also seeing the same thing (freeing shared objects when their
refcount becomes -1) in ip_vs.h:

http://lxr.free-electrons.com/source/include/net/ip_vs.h#L1424

static inline void ip_vs_dest_put_and_free(struct ip_vs_dest *dest)
{
    if (atomic_dec_return(&dest->refcnt) < 0)
        kfree(dest);
}

Note that this example also appears in a garbage collector internal to net/.

Thanks,
David Windsor

^ permalink raw reply

* Re: [PATCHv3 net-next 3/4] sctp: add support for generating stream reconf add incoming/outgoing streams request chunk
From: 'Marcelo Ricardo Leitner' @ 2017-01-23 12:36 UTC (permalink / raw)
  To: David Laight
  Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
	Neil Horman, Vlad Yasevich, davem@davemloft.net
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026B8E0@AcuExch.aculab.com>

On Mon, Jan 23, 2017 at 12:26:12PM +0000, David Laight wrote:
> From: Marcelo Ricardo Leitner > Sent: 20 January 2017 16:39
> > To: David Laight
> > On Fri, Jan 20, 2017 at 02:50:01PM +0000, David Laight wrote:
> > > From: Xin Long
> > > > Sent: 19 January 2017 17:19
> > > > This patch is to define Add Incoming/Outgoing Streams Request
> > > > Parameter described in rfc6525 section 4.5 and 4.6. They can
> > > > be in one same chunk trunk as rfc6525 section 3.1-7 describes,
> > > > so make them in one function.
> > > ...
> > > > +struct sctp_strreset_addstrm {
> > > > +	sctp_paramhdr_t param_hdr;
> > > > +	__u32 request_seq;
> > > > +	__u16 number_of_streams;
> > > > +	__u16 reserved;
> > > > +} __packed;
> > > ...
> > > > +		addstrm.param_hdr.type = SCTP_PARAM_RESET_ADD_OUT_STREAMS;
> > > > +		addstrm.param_hdr.length = htons(size);
> > > > +		addstrm.number_of_streams = htons(out);
> > > > +		addstrm.request_seq = htonl(asoc->strreset_outseq);
> > > > +		addstrm.reserved = 0;
> > > > +
> > > > +		sctp_addto_chunk(retval, size, &addstrm);
> > >
> > > Since you allocate the sctp_strreset_addstrm structure on stack
> > > there is no requirement for it to be packed.
> > 
> > It shouldn't matter that it's allocated on stack. Why should it?
> > We need it to be packed as this is a header that will be sent out to
> > another peer, so there can't be any padding on it.
> 
> That isn't what __packed means.
> It means that the compiler must assume that the structure can be
> misaligned in memory and must use byte memory accesses on systems
> that fault misaligned memory accesses.

That's a side-effect of it.

https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#Common-Type-Attributes
"This attribute, attached to struct or union type definition, specifies
that each member (other than zero-width bit-fields) of the structure or
union is placed to minimize the memory required. "

So, no padding. A field just after the other, which is what we want on a
network header.

  Marcelo

^ permalink raw reply

* RE: [PATCHv3 net-next 3/4] sctp: add support for generating stream reconf add incoming/outgoing streams request chunk
From: David Laight @ 2017-01-23 12:26 UTC (permalink / raw)
  To: 'Marcelo Ricardo Leitner'
  Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
	Neil Horman, Vlad Yasevich, davem@davemloft.net
In-Reply-To: <20170120163921.GU3781@localhost.localdomain>

From: Marcelo Ricardo Leitner > Sent: 20 January 2017 16:39
> To: David Laight
> On Fri, Jan 20, 2017 at 02:50:01PM +0000, David Laight wrote:
> > From: Xin Long
> > > Sent: 19 January 2017 17:19
> > > This patch is to define Add Incoming/Outgoing Streams Request
> > > Parameter described in rfc6525 section 4.5 and 4.6. They can
> > > be in one same chunk trunk as rfc6525 section 3.1-7 describes,
> > > so make them in one function.
> > ...
> > > +struct sctp_strreset_addstrm {
> > > +	sctp_paramhdr_t param_hdr;
> > > +	__u32 request_seq;
> > > +	__u16 number_of_streams;
> > > +	__u16 reserved;
> > > +} __packed;
> > ...
> > > +		addstrm.param_hdr.type = SCTP_PARAM_RESET_ADD_OUT_STREAMS;
> > > +		addstrm.param_hdr.length = htons(size);
> > > +		addstrm.number_of_streams = htons(out);
> > > +		addstrm.request_seq = htonl(asoc->strreset_outseq);
> > > +		addstrm.reserved = 0;
> > > +
> > > +		sctp_addto_chunk(retval, size, &addstrm);
> >
> > Since you allocate the sctp_strreset_addstrm structure on stack
> > there is no requirement for it to be packed.
> 
> It shouldn't matter that it's allocated on stack. Why should it?
> We need it to be packed as this is a header that will be sent out to
> another peer, so there can't be any padding on it.

That isn't what __packed means.
It means that the compiler must assume that the structure can be
misaligned in memory and must use byte memory accesses on systems
that fault misaligned memory accesses.

	David

^ permalink raw reply

* [PATCH] phy: marvell: remove conflicting initializer
From: Arnd Bergmann @ 2017-01-23 12:18 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Arnd Bergmann, David S. Miller, Andrew Lunn,
	Charles-Antoine Couret, Clemens Gruber, netdev, linux-kernel

One line was apparently pasted incorrectly during a new feature patch:

drivers/net/phy/marvell.c:2090:15: error: initialized field overwritten [-Werror=override-init]
   .features = PHY_GBIT_FEATURES,

I'm removing the extraneous line here to avoid the W=1 warning and restore
the previous flags value, and I'm slightly reordering the lines for consistency
to make it less likely to happen again in the future. The ordering in the
array is still not the same as in the structure definition, instead I picked
the order that is most common in this file and that seems to make more sense
here.

Fixes: 0b04680fdae4 ("phy: marvell: Add support for temperature sensor")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/phy/marvell.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 64229976ace1..b5b73ff4329a 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -1900,8 +1900,8 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1101",
 		.features = PHY_GBIT_FEATURES,
-		.probe = marvell_probe,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_init = &marvell_config_init,
 		.config_aneg = &marvell_config_aneg,
 		.read_status = &genphy_read_status,
@@ -1971,10 +1971,10 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1121R,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1121R",
-		.probe = &m88e1121_probe,
-		.remove = &marvell_remove,
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = &m88e1121_probe,
+		.remove = &marvell_remove,
 		.config_init = &m88e1121_config_init,
 		.config_aneg = &m88e1121_config_aneg,
 		.read_status = &marvell_read_status,
@@ -2085,10 +2085,9 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1510",
 		.features = PHY_GBIT_FEATURES | SUPPORTED_FIBRE,
+		.flags = PHY_HAS_INTERRUPT,
 		.probe = &m88e1510_probe,
 		.remove = &marvell_remove,
-		.features = PHY_GBIT_FEATURES,
-		.flags = PHY_HAS_INTERRUPT,
 		.config_init = &m88e1510_config_init,
 		.config_aneg = &m88e1510_config_aneg,
 		.read_status = &marvell_read_status,
@@ -2105,10 +2104,10 @@ static struct phy_driver marvell_drivers[] = {
 		.phy_id = MARVELL_PHY_ID_88E1540,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1540",
-		.probe = m88e1510_probe,
-		.remove = &marvell_remove,
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = m88e1510_probe,
+		.remove = &marvell_remove,
 		.config_init = &marvell_config_init,
 		.config_aneg = &m88e1510_config_aneg,
 		.read_status = &marvell_read_status,
-- 
2.9.0

^ permalink raw reply related

* [PATCH 03/17] gtp: make GTP sockets in gtp_newlink optional
From: Andreas Schultz @ 2017-01-23 11:56 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

A fixed binding of gtp enabled socket to netdevice does not make
sense (GTP TEID are unique per socket and not per L3 IP device).

To untangle netdevice and gtp sockets without breaking the UAPI,
make per netdevice sockets optional.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 158 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 83 insertions(+), 75 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 1df54d6..60946b7 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -276,30 +276,6 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 	return ret;
 }
 
-static void gtp_encap_disable(struct gtp_dev *gtp)
-{
-	if (gtp->sock0 && gtp->sock0->sk) {
-		udp_sk(gtp->sock0->sk)->encap_type = 0;
-		rcu_assign_sk_user_data(gtp->sock0->sk, NULL);
-	}
-	if (gtp->sock1u && gtp->sock1u->sk) {
-		udp_sk(gtp->sock1u->sk)->encap_type = 0;
-		rcu_assign_sk_user_data(gtp->sock1u->sk, NULL);
-	}
-
-	gtp->sock0 = NULL;
-	gtp->sock1u = NULL;
-}
-
-static void gtp_encap_destroy(struct sock *sk)
-{
-	struct gtp_dev *gtp;
-
-	gtp = rcu_dereference_sk_user_data(sk);
-	if (gtp)
-		gtp_encap_disable(gtp);
-}
-
 /* UDP encapsulation receive handler. See net/ipv4/udp.c.
  * Return codes: 0: success, <0: error, >0: pass up to userspace UDP socket.
  */
@@ -363,6 +339,17 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
 	return 0;
 }
 
+static void gtp_encap_destroy(struct sock *sk)
+{
+	struct gtp_dev *gtp;
+
+	gtp = rcu_dereference_sk_user_data(sk);
+	if (gtp) {
+		udp_sk(sk)->encap_type = 0;
+		rcu_assign_sk_user_data(sk, NULL);
+	}
+}
+
 static int gtp_dev_init(struct net_device *dev)
 {
 	struct gtp_dev *gtp = netdev_priv(dev);
@@ -378,9 +365,6 @@ static int gtp_dev_init(struct net_device *dev)
 
 static void gtp_dev_uninit(struct net_device *dev)
 {
-	struct gtp_dev *gtp = netdev_priv(dev);
-
-	gtp_encap_disable(gtp);
 	free_percpu(dev->tstats);
 }
 
@@ -658,35 +642,32 @@ static void gtp_link_setup(struct net_device *dev)
 static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize);
 static void gtp_hashtable_free(struct gtp_dev *gtp);
 static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
-			    int fd_gtp0, int fd_gtp1, struct net *src_net);
+			    struct net *src_net, struct nlattr *data[]);
+static void gtp_encap_disable(struct gtp_dev *gtp);
 
 static int gtp_newlink(struct net *src_net, struct net_device *dev,
 			struct nlattr *tb[], struct nlattr *data[])
 {
-	int hashsize, err, fd0, fd1;
+	int hashsize, err;
 	struct gtp_dev *gtp;
 	struct gtp_net *gn;
 
-	if (!data[IFLA_GTP_FD0] || !data[IFLA_GTP_FD1])
-		return -EINVAL;
-
 	gtp = netdev_priv(dev);
 
-	fd0 = nla_get_u32(data[IFLA_GTP_FD0]);
-	fd1 = nla_get_u32(data[IFLA_GTP_FD1]);
-
-	err = gtp_encap_enable(dev, gtp, fd0, fd1, src_net);
-	if (err < 0)
-		goto out_err;
-
 	if (!data[IFLA_GTP_PDP_HASHSIZE])
 		hashsize = 1024;
 	else
 		hashsize = nla_get_u32(data[IFLA_GTP_PDP_HASHSIZE]);
 
+	if (data[IFLA_GTP_FD0] || data[IFLA_GTP_FD1]) {
+		err = gtp_encap_enable(dev, gtp, src_net, data);
+		if (err < 0)
+			goto out_err;
+	}
+
 	err = gtp_hashtable_new(gtp, hashsize);
 	if (err < 0)
-		goto out_encap;
+		goto out_socket;
 
 	err = register_netdevice(dev);
 	if (err < 0) {
@@ -703,7 +684,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
 
 out_hashtable:
 	gtp_hashtable_free(gtp);
-out_encap:
+out_socket:
 	gtp_encap_disable(gtp);
 out_err:
 	return err;
@@ -713,8 +694,11 @@ static void gtp_dellink(struct net_device *dev, struct list_head *head)
 {
 	struct gtp_dev *gtp = netdev_priv(dev);
 
-	gtp_encap_disable(gtp);
 	gtp_hashtable_free(gtp);
+	if (gtp->sock0)
+		sockfd_put(gtp->sock0);
+	if (gtp->sock1u)
+		sockfd_put(gtp->sock1u);
 	list_del_rcu(&gtp->list);
 	unregister_netdevice_queue(dev, head);
 }
@@ -820,38 +804,63 @@ static void gtp_hashtable_free(struct gtp_dev *gtp)
 	kfree(gtp->tid_hash);
 }
 
-static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
-			    int fd_gtp0, int fd_gtp1, struct net *src_net)
+static struct socket *gtp_encap_enable_socket(int fd, int type,
+					      struct gtp_dev *gtp)
 {
 	struct udp_tunnel_sock_cfg tuncfg = {NULL};
-	struct socket *sock0, *sock1u;
+	struct socket *sock;
 	int err;
 
-	netdev_dbg(dev, "enable gtp on %d, %d\n", fd_gtp0, fd_gtp1);
+	pr_debug("enable gtp on %d, %d\n", fd, type);
 
-	sock0 = sockfd_lookup(fd_gtp0, &err);
-	if (sock0 == NULL) {
-		netdev_dbg(dev, "socket fd=%d not found (gtp0)\n", fd_gtp0);
-		return -ENOENT;
+	sock = sockfd_lookup(fd, &err);
+	if (!sock) {
+		pr_debug("gtp socket fd=%d not found\n", fd);
+		return NULL;
 	}
 
-	if (sock0->sk->sk_protocol != IPPROTO_UDP) {
-		netdev_dbg(dev, "socket fd=%d not UDP\n", fd_gtp0);
+	if (sock->sk->sk_protocol != IPPROTO_UDP) {
+		pr_debug("socket fd=%d not UDP\n", fd);
 		err = -EINVAL;
-		goto err1;
+		goto out_sock;
 	}
 
-	sock1u = sockfd_lookup(fd_gtp1, &err);
-	if (sock1u == NULL) {
-		netdev_dbg(dev, "socket fd=%d not found (gtp1u)\n", fd_gtp1);
-		err = -ENOENT;
-		goto err1;
+	tuncfg.sk_user_data = gtp;
+	tuncfg.encap_type = type;
+	tuncfg.encap_rcv = gtp_encap_recv;
+	tuncfg.encap_destroy = gtp_encap_destroy;
+
+	setup_udp_tunnel_sock(sock_net(sock->sk), sock, &tuncfg);
+	return sock;
+
+out_sock:
+	sockfd_put(sock);
+	return ERR_PTR(err);
+}
+
+static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
+			    struct net *src_net, struct nlattr *data[])
+{
+	struct socket *sock0 = NULL;
+	struct socket *sock1u = NULL;
+
+	if (data[IFLA_GTP_FD0]) {
+		u32 fd0 = nla_get_u32(data[IFLA_GTP_FD0]);
+
+		sock0 = gtp_encap_enable_socket(fd0, UDP_ENCAP_GTP0, gtp);
+		if (IS_ERR(sock0))
+			return PTR_ERR(sock0);
 	}
 
-	if (sock1u->sk->sk_protocol != IPPROTO_UDP) {
-		netdev_dbg(dev, "socket fd=%d not UDP\n", fd_gtp1);
-		err = -EINVAL;
-		goto err2;
+	if (data[IFLA_GTP_FD1]) {
+		u32 fd1 = nla_get_u32(data[IFLA_GTP_FD1]);
+
+		sock1u = gtp_encap_enable_socket(fd1, UDP_ENCAP_GTP1U, gtp);
+		if (IS_ERR(sock1u)) {
+			if (sock0)
+				sockfd_put(sock0);
+			return PTR_ERR(sock1u);
+		}
 	}
 
 	netdev_dbg(dev, "enable gtp on %p, %p\n", sock0, sock1u);
@@ -860,22 +869,21 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
 	gtp->sock1u = sock1u;
 	gtp->net = src_net;
 
-	tuncfg.sk_user_data = gtp;
-	tuncfg.encap_rcv = gtp_encap_recv;
-	tuncfg.encap_destroy = gtp_encap_destroy;
+	return 0;
+}
 
-	tuncfg.encap_type = UDP_ENCAP_GTP0;
-	setup_udp_tunnel_sock(sock_net(gtp->sock0->sk), gtp->sock0, &tuncfg);
+static void gtp_encap_disable_sock(struct socket *sock)
+{
+	if (!sock || !sock->sk)
+		return;
 
-	tuncfg.encap_type = UDP_ENCAP_GTP1U;
-	setup_udp_tunnel_sock(sock_net(gtp->sock1u->sk), gtp->sock1u, &tuncfg);
+	gtp_encap_destroy(sock->sk);
+}
 
-	err = 0;
-err2:
-	sockfd_put(sock1u);
-err1:
-	sockfd_put(sock0);
-	return err;
+static void gtp_encap_disable(struct gtp_dev *gtp)
+{
+	gtp_encap_disable_sock(gtp->sock0);
+	gtp_encap_disable_sock(gtp->sock1u);
 }
 
 static struct net_device *gtp_find_dev(struct net *net, int ifindex)
-- 
2.10.2

^ permalink raw reply related

* [PATCH 17/17] gtp: add support to select a GTP socket during PDP context creation
From: Andreas Schultz @ 2017-01-23 11:57 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 3 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 157cf40..53faaa4 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1016,6 +1016,7 @@ static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info)
 	struct net_device *dev;
 	struct net *net;
 	struct socket *sock;
+	int err;
 
 	if (!info->attrs[GTPA_VERSION] ||
 	    !info->attrs[GTPA_LINK] ||
@@ -1053,11 +1054,19 @@ static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info)
 	}
 	put_net(net);
 
-	sock = gtp_genl_new_pdp_select_socket(version, dev);
-	if (!sock)
+	if (info->attrs[GTPA_FD])
+		sock = sockfd_lookup(nla_get_u32(info->attrs[GTPA_FD]), &err);
+	else
+		sock = gtp_genl_new_pdp_select_socket(version, dev);
+	if (!sock || !sock->sk)
 		return -ENODEV;
 
-	return ipv4_pdp_add(dev, sock->sk, info);
+	err = ipv4_pdp_add(dev, sock->sk, info);
+
+	if (info->attrs[GTPA_FD])
+		sockfd_put(sock);
+
+	return err;
 }
 
 static struct pdp_ctx *gtp_genl_find_pdp_by_link(struct sk_buff *skb,
@@ -1089,11 +1098,64 @@ static struct pdp_ctx *gtp_genl_find_pdp_by_link(struct sk_buff *skb,
 	return ipv4_pdp_find(gtp, ms_addr);
 }
 
+static struct pdp_ctx *gtp_genl_find_pdp_by_socket(struct sk_buff *skb,
+						   struct genl_info *info)
+{
+	struct socket *sock;
+	struct gtp_sock *gsk;
+	struct pdp_ctx *pctx;
+	int fd, err = 0;
+
+	if (!info->attrs[GTPA_FD])
+		return ERR_PTR(-EINVAL);
+
+	fd = nla_get_u32(info->attrs[GTPA_FD]);
+	sock = sockfd_lookup(fd, &err);
+	if (!sock) {
+		pr_debug("gtp socket fd=%d not found\n", fd);
+		return ERR_PTR(-EBADF);
+	}
+
+	gsk = rcu_dereference_sk_user_data(sock->sk);
+	if (!gsk) {
+		pctx = ERR_PTR(-EINVAL);
+		goto out_sock;
+	}
+
+	switch (nla_get_u32(info->attrs[GTPA_VERSION])) {
+	case GTP_V0:
+		if (!info->attrs[GTPA_TID]) {
+			pctx = ERR_PTR(-EINVAL);
+			break;
+		}
+		pctx = gtp0_pdp_find(gsk, nla_get_u64(info->attrs[GTPA_TID]));
+		break;
+
+	case GTP_V1:
+		if (!info->attrs[GTPA_I_TEI]) {
+			pctx = ERR_PTR(-EINVAL);
+			break;
+		}
+		pctx = gtp1_pdp_find(gsk, nla_get_u64(info->attrs[GTPA_I_TEI]));
+		break;
+
+	default:
+		pctx = ERR_PTR(-EINVAL);
+		break;
+	}
+
+out_sock:
+	sockfd_put(sock);
+	return pctx;
+}
+
 static struct pdp_ctx *gtp_genl_find_pdp(struct sk_buff *skb,
 					 struct genl_info *info)
 {
 	if (info->attrs[GTPA_LINK])
 		return gtp_genl_find_pdp_by_link(skb, info);
+	else if (info->attrs[GTPA_FD])
+		return gtp_genl_find_pdp_by_socket(skb, info);
 	else
 		return ERR_PTR(-EINVAL);
 }
-- 
2.10.2

^ permalink raw reply related

* [PATCH 00/17] gtp: fixes and support multiple VRF's per GTP socket
From: Andreas Schultz @ 2017-01-23 11:56 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc

The current linking of GTP network devices and GTP enabled sockets means that
we can not have multiple VRF's per GTP socket. This series seperates the
sockets from network device, makes sockets attached to GTP network device
optional and adds a API function to enable GTP encapsulation on socket
without having to create a new GTP device.

It is still possible to use the old API. The network device attached socket is
then used when no socket is specified on PDP context creation.

During that work some smaller problems where found and fixes for them are
included.

Regards
Andreas

^ permalink raw reply

* [PATCH 11/17] gtp: consolidate gtp socket rx path
From: Andreas Schultz @ 2017-01-23 11:57 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

Add network device to gtp context in preparation for splitting
the TEID from the network device.

Use this to rework the socker rx path. Move the common RX part
of v0 and v1 into a helper. Also move the final rx part into
that helper as well.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 92 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 47 insertions(+), 45 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 6600c92..868467d 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -58,6 +58,7 @@ struct pdp_ctx {
 	struct in_addr		ms_addr_ip4;
 	struct in_addr		sgsn_addr_ip4;
 
+	struct net_device       *dev;
 	struct sock		*sk;
 
 	atomic_t		tx_seq;
@@ -179,9 +180,42 @@ static bool gtp_check_src_ms(struct sk_buff *skb, struct pdp_ctx *pctx,
 	return false;
 }
 
+static int gtp_rx(struct sk_buff *skb, struct pdp_ctx *pctx, unsigned int hdrlen)
+{
+	struct pcpu_sw_netstats *stats;
+
+	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
+		pr_debug("No PDP ctx for this MS\n");
+		return -1;
+	}
+
+	/* Get rid of the GTP + UDP headers. */
+	if (iptunnel_pull_header(skb, hdrlen, skb->protocol,
+				 !net_eq(sock_net(skb->sk), dev_net(pctx->dev))))
+		return -1;
+
+	pr_debug("forwarding packet from GGSN to uplink\n");
+
+	/* Now that the UDP and the GTP header have been removed, set up the
+	 * new network header. This is required by the upper layer to
+	 * calculate the transport header.
+	 */
+	skb_reset_network_header(skb);
+
+	skb->dev = pctx->dev;
+
+	stats = this_cpu_ptr(pctx->dev->tstats);
+	u64_stats_update_begin(&stats->syncp);
+	stats->rx_packets++;
+	stats->rx_bytes += skb->len;
+	u64_stats_update_end(&stats->syncp);
+
+	netif_rx(skb);
+	return 0;
+}
+
 /* 1 means pass up to the stack, -1 means drop and 0 means decapsulated. */
-static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
-			       bool xnet)
+static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
 {
 	unsigned int hdrlen = sizeof(struct udphdr) +
 			      sizeof(struct gtp0_header);
@@ -205,17 +239,10 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 		return -1;
 	}
 
-	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
-		netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
-		return -1;
-	}
-
-	/* Get rid of the GTP + UDP headers. */
-	return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet);
+	return gtp_rx(skb, pctx, hdrlen);
 }
 
-static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
-				bool xnet)
+static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
 {
 	unsigned int hdrlen = sizeof(struct udphdr) +
 			      sizeof(struct gtp1_header);
@@ -254,13 +281,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 		return -1;
 	}
 
-	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
-		netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
-		return -1;
-	}
-
-	/* Get rid of the GTP + UDP headers. */
-	return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet);
+	return gtp_rx(skb, pctx, hdrlen);
 }
 
 /* UDP encapsulation receive handler. See net/ipv4/udp.c.
@@ -268,10 +289,8 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
  */
 static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
 {
-	struct pcpu_sw_netstats *stats;
 	struct gtp_dev *gtp;
-	bool xnet;
-	int ret;
+	int ret = 0;
 
 	gtp = rcu_dereference_sk_user_data(sk);
 	if (!gtp)
@@ -279,16 +298,14 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
 
 	netdev_dbg(gtp->dev, "encap_recv sk=%p\n", sk);
 
-	xnet = !net_eq(sock_net(sk), dev_net(gtp->dev));
-
 	switch (udp_sk(sk)->encap_type) {
 	case UDP_ENCAP_GTP0:
 		netdev_dbg(gtp->dev, "received GTP0 packet\n");
-		ret = gtp0_udp_encap_recv(gtp, skb, xnet);
+		ret = gtp0_udp_encap_recv(gtp, skb);
 		break;
 	case UDP_ENCAP_GTP1U:
 		netdev_dbg(gtp->dev, "received GTP1U packet\n");
-		ret = gtp1u_udp_encap_recv(gtp, skb, xnet);
+		ret = gtp1u_udp_encap_recv(gtp, skb);
 		break;
 	default:
 		ret = -1; /* Shouldn't happen. */
@@ -297,33 +314,17 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
 	switch (ret) {
 	case 1:
 		netdev_dbg(gtp->dev, "pass up to the process\n");
-		return 1;
+		break;
 	case 0:
-		netdev_dbg(gtp->dev, "forwarding packet from GGSN to uplink\n");
 		break;
 	case -1:
 		netdev_dbg(gtp->dev, "GTP packet has been dropped\n");
 		kfree_skb(skb);
-		return 0;
+		ret = 0;
+		break;
 	}
 
-	/* Now that the UDP and the GTP header have been removed, set up the
-	 * new network header. This is required by the upper layer to
-	 * calculate the transport header.
-	 */
-	skb_reset_network_header(skb);
-
-	skb->dev = gtp->dev;
-
-	stats = this_cpu_ptr(gtp->dev->tstats);
-	u64_stats_update_begin(&stats->syncp);
-	stats->rx_packets++;
-	stats->rx_bytes += skb->len;
-	u64_stats_update_end(&stats->syncp);
-
-	netif_rx(skb);
-
-	return 0;
+	return ret;
 }
 
 static void gtp_encap_destroy(struct sock *sk)
@@ -929,6 +930,7 @@ static int ipv4_pdp_add(struct net_device *dev, struct sock *sk,
 
 	sock_hold(sk);
 	pctx->sk = sk;
+	pctx->dev = dev;
 	ipv4_pdp_fill(pctx, info);
 	atomic_set(&pctx->tx_seq, 0);
 
-- 
2.10.2

^ permalink raw reply related

* [PATCH 07/17] gtp: remove unnecessary rcu_read_lock
From: Andreas Schultz @ 2017-01-23 11:56 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

The rcu read lock is hold by default in the ip input path. There
is no need to hold it twice in the socket recv decapsulate code path.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index bc8734b7..f434f84 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -183,7 +183,6 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 			      sizeof(struct gtp0_header);
 	struct gtp0_header *gtp0;
 	struct pdp_ctx *pctx;
-	int ret = 0;
 
 	if (!pskb_may_pull(skb, hdrlen))
 		return -1;
@@ -196,26 +195,19 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 	if (gtp0->type != GTP_TPDU)
 		return 1;
 
-	rcu_read_lock();
 	pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid));
 	if (IS_ERR(pctx)) {
 		netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
-		ret = -1;
-		goto out_rcu;
+		return -1;
 	}
 
 	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
 		netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
-		ret = -1;
-		goto out_rcu;
+		return -1;
 	}
-	rcu_read_unlock();
 
 	/* Get rid of the GTP + UDP headers. */
 	return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet);
-out_rcu:
-	rcu_read_unlock();
-	return ret;
 }
 
 static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
@@ -225,7 +217,6 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 			      sizeof(struct gtp1_header);
 	struct gtp1_header *gtp1;
 	struct pdp_ctx *pctx;
-	int ret = 0;
 
 	if (!pskb_may_pull(skb, hdrlen))
 		return -1;
@@ -253,26 +244,19 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 
 	gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
 
-	rcu_read_lock();
 	pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
 	if (IS_ERR(pctx)) {
 		netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
-		ret = -1;
-		goto out_rcu;
+		return -1;
 	}
 
 	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
 		netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
-		ret = -1;
-		goto out_rcu;
+		return -1;
 	}
-	rcu_read_unlock();
 
 	/* Get rid of the GTP + UDP headers. */
 	return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet);
-out_rcu:
-	rcu_read_unlock();
-	return ret;
 }
 
 /* UDP encapsulation receive handler. See net/ipv4/udp.c.
-- 
2.10.2

^ permalink raw reply related

* [PATCH 16/17] gtp: add genl cmd to enable GTP encapsulation on UDP socket
From: Andreas Schultz @ 2017-01-23 11:57 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c        | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/gtp.h |  4 ++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 55bf098..157cf40 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1240,6 +1240,46 @@ static int gtp_genl_dump_pdp(struct sk_buff *skb,
 	return skb->len;
 }
 
+static int gtp_genl_enable_socket(struct sk_buff *skb, struct genl_info *info)
+{
+	u32 version, fd, hashsize;
+	struct socket *sock;
+
+	if (!info->attrs[GTPA_VERSION] ||
+	    !info->attrs[GTPA_FD])
+		return -EINVAL;
+
+	if (!info->attrs[GTPA_PDP_HASHSIZE])
+		hashsize = 1024;
+	else
+		hashsize = nla_get_u32(info->attrs[IFLA_GTP_PDP_HASHSIZE]);
+
+	version = nla_get_u32(info->attrs[GTPA_VERSION]);
+	fd = nla_get_u32(info->attrs[GTPA_FD]);
+
+	switch (version) {
+	case GTP_V0:
+		sock = gtp_encap_enable_socket(fd, UDP_ENCAP_GTP0, hashsize);
+		break;
+
+	case GTP_V1:
+		sock = gtp_encap_enable_socket(fd, UDP_ENCAP_GTP1U, hashsize);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	if (!sock)
+		return -EINVAL;
+
+	if (IS_ERR(sock))
+		return PTR_ERR(sock);
+
+	sockfd_put(sock);
+	return 0;
+}
+
 static struct nla_policy gtp_genl_policy[GTPA_MAX + 1] = {
 	[GTPA_LINK]		= { .type = NLA_U32, },
 	[GTPA_VERSION]		= { .type = NLA_U32, },
@@ -1250,6 +1290,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, },
+	[GTPA_FD]		= { .type = NLA_U32, },
 };
 
 static const struct genl_ops gtp_genl_ops[] = {
@@ -1272,6 +1314,12 @@ static const struct genl_ops gtp_genl_ops[] = {
 		.policy = gtp_genl_policy,
 		.flags = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd = GTP_CMD_ENABLE_SOCKET,
+		.doit = gtp_genl_enable_socket,
+		.policy = gtp_genl_policy,
+		.flags = GENL_ADMIN_PERM,
+	},
 };
 
 static struct genl_family gtp_genl_family __ro_after_init = {
diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h
index 72a04a0..a9e9fe0 100644
--- a/include/uapi/linux/gtp.h
+++ b/include/uapi/linux/gtp.h
@@ -6,6 +6,8 @@ enum gtp_genl_cmds {
 	GTP_CMD_DELPDP,
 	GTP_CMD_GETPDP,
 
+	GTP_CMD_ENABLE_SOCKET,
+
 	GTP_CMD_MAX,
 };
 
@@ -26,6 +28,8 @@ enum gtp_attrs {
 	GTPA_I_TEI,	/* for GTPv1 only */
 	GTPA_O_TEI,	/* for GTPv1 only */
 	GTPA_PAD,
+	GTPA_PDP_HASHSIZE,
+	GTPA_FD,
 	__GTPA_MAX,
 };
 #define GTPA_MAX (__GTPA_MAX + 1)
-- 
2.10.2

^ permalink raw reply related

* [PATCH 09/17] gtp: use addr_hash when traversing pdp contexts
From: Andreas Schultz @ 2017-01-23 11:56 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

This prepares for the removal of the tid_hash from the device.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 5b35ebb..762dd6b 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -778,7 +778,7 @@ static void gtp_hashtable_free(struct gtp_dev *gtp)
 	int i;
 
 	for (i = 0; i < gtp->hash_size; i++)
-		hlist_for_each_entry_rcu(pctx, &gtp->tid_hash[i], hlist_tid)
+		hlist_for_each_entry_rcu(pctx, &gtp->addr_hash[i], hlist_addr)
 			pdp_context_delete(pctx);
 
 	synchronize_rcu();
@@ -1188,7 +1188,7 @@ static int gtp_genl_dump_pdp(struct sk_buff *skb,
 			last_gtp = NULL;
 
 		for (i = k; i < gtp->hash_size; i++) {
-			hlist_for_each_entry_rcu(pctx, &gtp->tid_hash[i], hlist_tid) {
+			hlist_for_each_entry_rcu(pctx, &gtp->addr_hash[i], hlist_addr) {
 				if (tid && tid != pctx->u.tid)
 					continue;
 				else
-- 
2.10.2

^ permalink raw reply related

* [PATCH 14/17] gtp: move TEID hash to per socket structure
From: Andreas Schultz @ 2017-01-23 11:57 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

Untangele the TEID information from the network device and move
it into a per socket structure.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 100 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 60 insertions(+), 40 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 276cc66..019e80f 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -75,10 +75,15 @@ struct gtp_dev {
 	struct net_device	*dev;
 
 	unsigned int		hash_size;
-	struct hlist_head	*tid_hash;
 	struct hlist_head	*addr_hash;
 };
 
+/* One instance of the GTP socket. */
+struct gtp_sock {
+	unsigned int		hash_size;
+	struct hlist_head	tid_hash[];
+};
+
 static unsigned int gtp_net_id __read_mostly;
 
 struct gtp_net {
@@ -106,12 +111,12 @@ static inline u32 ipv4_hashfn(__be32 ip)
 }
 
 /* Resolve a PDP context structure based on the 64bit TID. */
-static struct pdp_ctx *gtp0_pdp_find(struct gtp_dev *gtp, u64 tid)
+static struct pdp_ctx *gtp0_pdp_find(struct gtp_sock *gsk, u64 tid)
 {
 	struct hlist_head *head;
 	struct pdp_ctx *pdp;
 
-	head = &gtp->tid_hash[gtp0_hashfn(tid) % gtp->hash_size];
+	head = &gsk->tid_hash[gtp0_hashfn(tid) % gsk->hash_size];
 
 	hlist_for_each_entry_rcu(pdp, head, hlist_tid) {
 		if (pdp->gtp_version == GTP_V0 &&
@@ -122,12 +127,12 @@ static struct pdp_ctx *gtp0_pdp_find(struct gtp_dev *gtp, u64 tid)
 }
 
 /* Resolve a PDP context structure based on the 32bit TEI. */
-static struct pdp_ctx *gtp1_pdp_find(struct gtp_dev *gtp, u32 tid)
+static struct pdp_ctx *gtp1_pdp_find(struct gtp_sock *gsk, u32 tid)
 {
 	struct hlist_head *head;
 	struct pdp_ctx *pdp;
 
-	head = &gtp->tid_hash[gtp1u_hashfn(tid) % gtp->hash_size];
+	head = &gsk->tid_hash[gtp1u_hashfn(tid) % gsk->hash_size];
 
 	hlist_for_each_entry_rcu(pdp, head, hlist_tid) {
 		if (pdp->gtp_version == GTP_V1 &&
@@ -215,7 +220,7 @@ static int gtp_rx(struct sk_buff *skb, struct pdp_ctx *pctx, unsigned int hdrlen
 }
 
 /* 1 means pass up to the stack, -1 means drop and 0 means decapsulated. */
-static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
+static int gtp0_udp_encap_recv(struct gtp_sock *gsk, struct sk_buff *skb)
 {
 	unsigned int hdrlen = sizeof(struct udphdr) +
 			      sizeof(struct gtp0_header);
@@ -233,7 +238,7 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
 	if (gtp0->type != GTP_TPDU)
 		return 1;
 
-	pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid));
+	pctx = gtp0_pdp_find(gsk, be64_to_cpu(gtp0->tid));
 	if (IS_ERR(pctx)) {
 		pr_debug("No PDP ctx to decap skb=%p\n", skb);
 		return 1;
@@ -242,7 +247,7 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
 	return gtp_rx(skb, pctx, hdrlen);
 }
 
-static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
+static int gtp1u_udp_encap_recv(struct gtp_sock *gsk, struct sk_buff *skb)
 {
 	unsigned int hdrlen = sizeof(struct udphdr) +
 			      sizeof(struct gtp1_header);
@@ -275,7 +280,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
 
 	gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
 
-	pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
+	pctx = gtp1_pdp_find(gsk, ntohl(gtp1->tid));
 	if (IS_ERR(pctx)) {
 		pr_debug("No PDP ctx to decap skb=%p\n", skb);
 		return 1;
@@ -289,11 +294,11 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
  */
 static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
 {
-	struct gtp_dev *gtp;
+	struct gtp_sock *gsk;
 	int ret = 0;
 
-	gtp = rcu_dereference_sk_user_data(sk);
-	if (!gtp)
+	gsk = rcu_dereference_sk_user_data(sk);
+	if (!gsk)
 		return 1;
 
 	pr_debug("encap_recv sk=%p\n", sk);
@@ -301,11 +306,11 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
 	switch (udp_sk(sk)->encap_type) {
 	case UDP_ENCAP_GTP0:
 		pr_debug("received GTP0 packet\n");
-		ret = gtp0_udp_encap_recv(gtp, skb);
+		ret = gtp0_udp_encap_recv(gsk, skb);
 		break;
 	case UDP_ENCAP_GTP1U:
 		pr_debug("received GTP1U packet\n");
-		ret = gtp1u_udp_encap_recv(gtp, skb);
+		ret = gtp1u_udp_encap_recv(gsk, skb);
 		break;
 	default:
 		ret = -1; /* Shouldn't happen. */
@@ -329,12 +334,21 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
 
 static void gtp_encap_destroy(struct sock *sk)
 {
-	struct gtp_dev *gtp;
+	struct gtp_sock *gsk;
+	struct pdp_ctx *pctx;
+	int i;
 
-	gtp = rcu_dereference_sk_user_data(sk);
-	if (gtp) {
+	gsk = rcu_dereference_sk_user_data(sk);
+	if (gsk) {
 		udp_sk(sk)->encap_type = 0;
 		rcu_assign_sk_user_data(sk, NULL);
+
+		for (i = 0; i < gsk->hash_size; i++)
+			hlist_for_each_entry_rcu(pctx, &gsk->tid_hash[i], hlist_tid)
+				pdp_context_delete(pctx);
+
+		synchronize_rcu();
+		kfree(gsk);
 	}
 }
 
@@ -607,7 +621,7 @@ static void gtp_link_setup(struct net_device *dev)
 static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize);
 static void gtp_hashtable_free(struct gtp_dev *gtp);
 static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
-			    struct nlattr *data[]);
+			    int hsize, struct nlattr *data[]);
 static void gtp_encap_disable(struct gtp_dev *gtp);
 
 static int gtp_newlink(struct net *src_net, struct net_device *dev,
@@ -625,7 +639,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
 		hashsize = nla_get_u32(data[IFLA_GTP_PDP_HASHSIZE]);
 
 	if (data[IFLA_GTP_FD0] || data[IFLA_GTP_FD1]) {
-		err = gtp_encap_enable(dev, gtp, data);
+		err = gtp_encap_enable(dev, gtp, hashsize, data);
 		if (err < 0)
 			goto out_err;
 	}
@@ -736,20 +750,12 @@ static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize)
 	if (gtp->addr_hash == NULL)
 		return -ENOMEM;
 
-	gtp->tid_hash = kmalloc(sizeof(struct hlist_head) * hsize, GFP_KERNEL);
-	if (gtp->tid_hash == NULL)
-		goto err1;
-
 	gtp->hash_size = hsize;
 
-	for (i = 0; i < hsize; i++) {
+	for (i = 0; i < hsize; i++)
 		INIT_HLIST_HEAD(&gtp->addr_hash[i]);
-		INIT_HLIST_HEAD(&gtp->tid_hash[i]);
-	}
+
 	return 0;
-err1:
-	kfree(gtp->addr_hash);
-	return -ENOMEM;
 }
 
 static void gtp_hashtable_free(struct gtp_dev *gtp)
@@ -763,15 +769,14 @@ static void gtp_hashtable_free(struct gtp_dev *gtp)
 
 	synchronize_rcu();
 	kfree(gtp->addr_hash);
-	kfree(gtp->tid_hash);
 }
 
-static struct socket *gtp_encap_enable_socket(int fd, int type,
-					      struct gtp_dev *gtp)
+static struct socket *gtp_encap_enable_socket(int fd, int type, int hsize)
 {
 	struct udp_tunnel_sock_cfg tuncfg = {NULL};
+	struct gtp_sock *gsk;
 	struct socket *sock;
-	int err;
+	int err, i;
 
 	pr_debug("enable gtp on %d, %d\n", fd, type);
 
@@ -787,7 +792,17 @@ static struct socket *gtp_encap_enable_socket(int fd, int type,
 		goto out_sock;
 	}
 
-	tuncfg.sk_user_data = gtp;
+	gsk = kzalloc(sizeof(*gsk) + sizeof(struct hlist_head) * hsize, GFP_KERNEL);
+	if (!gsk) {
+		err = -ENOMEM;
+		goto out_sock;
+	}
+
+	gsk->hash_size = hsize;
+	for (i = 0; i < hsize; i++)
+		INIT_HLIST_HEAD(&gsk->tid_hash[i]);
+
+	tuncfg.sk_user_data = gsk;
 	tuncfg.encap_type = type;
 	tuncfg.encap_rcv = gtp_encap_recv;
 	tuncfg.encap_destroy = gtp_encap_destroy;
@@ -801,7 +816,7 @@ static struct socket *gtp_encap_enable_socket(int fd, int type,
 }
 
 static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
-			    struct nlattr *data[])
+			    int hsize, struct nlattr *data[])
 {
 	struct socket *sock0 = NULL;
 	struct socket *sock1u = NULL;
@@ -809,7 +824,7 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
 	if (data[IFLA_GTP_FD0]) {
 		u32 fd0 = nla_get_u32(data[IFLA_GTP_FD0]);
 
-		sock0 = gtp_encap_enable_socket(fd0, UDP_ENCAP_GTP0, gtp);
+		sock0 = gtp_encap_enable_socket(fd0, UDP_ENCAP_GTP0, hsize);
 		if (IS_ERR(sock0))
 			return PTR_ERR(sock0);
 	}
@@ -817,7 +832,7 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
 	if (data[IFLA_GTP_FD1]) {
 		u32 fd1 = nla_get_u32(data[IFLA_GTP_FD1]);
 
-		sock1u = gtp_encap_enable_socket(fd1, UDP_ENCAP_GTP1U, gtp);
+		sock1u = gtp_encap_enable_socket(fd1, UDP_ENCAP_GTP1U, hsize);
 		if (IS_ERR(sock1u)) {
 			if (sock0)
 				sockfd_put(sock0);
@@ -890,11 +905,16 @@ static int ipv4_pdp_add(struct net_device *dev, struct sock *sk,
 			struct genl_info *info)
 {
 	struct gtp_dev *gtp = netdev_priv(dev);
+	struct gtp_sock *gsk;
 	u32 hash_ms, hash_tid = 0;
 	struct pdp_ctx *pctx;
 	bool found = false;
 	__be32 ms_addr;
 
+	gsk = rcu_dereference_sk_user_data(sk);
+	if (!gsk)
+		return -ENODEV;
+
 	ms_addr = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
 	hash_ms = ipv4_hashfn(ms_addr) % gtp->hash_size;
 
@@ -941,15 +961,15 @@ static int ipv4_pdp_add(struct net_device *dev, struct sock *sk,
 		 * situation in which this doesn't unambiguosly identify the
 		 * PDP context.
 		 */
-		hash_tid = gtp0_hashfn(pctx->u.v0.tid) % gtp->hash_size;
+		hash_tid = gtp0_hashfn(pctx->u.v0.tid) % gsk->hash_size;
 		break;
 	case GTP_V1:
-		hash_tid = gtp1u_hashfn(pctx->u.v1.i_tei) % gtp->hash_size;
+		hash_tid = gtp1u_hashfn(pctx->u.v1.i_tei) % gsk->hash_size;
 		break;
 	}
 
 	hlist_add_head_rcu(&pctx->hlist_addr, &gtp->addr_hash[hash_ms]);
-	hlist_add_head_rcu(&pctx->hlist_tid, &gtp->tid_hash[hash_tid]);
+	hlist_add_head_rcu(&pctx->hlist_tid, &gsk->tid_hash[hash_tid]);
 
 	switch (pctx->gtp_version) {
 	case GTP_V0:
-- 
2.10.2

^ permalink raw reply related

* [PATCH 15/17] gtp: rename gtp hashtable helpers
From: Andreas Schultz @ 2017-01-23 11:57 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

The gtp_hashtable helper are now olny used for the per netdevice
address hashes. Rename them to make their purpose clearer.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 019e80f..55bf098 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -618,8 +618,8 @@ static void gtp_link_setup(struct net_device *dev)
 				  sizeof(struct gtp0_header);
 }
 
-static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize);
-static void gtp_hashtable_free(struct gtp_dev *gtp);
+static int gtp_dev_hashtable_new(struct gtp_dev *gtp, int hsize);
+static void gtp_dev_hashtable_free(struct gtp_dev *gtp);
 static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
 			    int hsize, struct nlattr *data[]);
 static void gtp_encap_disable(struct gtp_dev *gtp);
@@ -644,7 +644,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
 			goto out_err;
 	}
 
-	err = gtp_hashtable_new(gtp, hashsize);
+	err = gtp_dev_hashtable_new(gtp, hashsize);
 	if (err < 0)
 		goto out_socket;
 
@@ -662,7 +662,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
 	return 0;
 
 out_hashtable:
-	gtp_hashtable_free(gtp);
+	gtp_dev_hashtable_free(gtp);
 out_socket:
 	gtp_encap_disable(gtp);
 out_err:
@@ -673,7 +673,7 @@ static void gtp_dellink(struct net_device *dev, struct list_head *head)
 {
 	struct gtp_dev *gtp = netdev_priv(dev);
 
-	gtp_hashtable_free(gtp);
+	gtp_dev_hashtable_free(gtp);
 	if (gtp->sock0)
 		sockfd_put(gtp->sock0);
 	if (gtp->sock1u)
@@ -742,7 +742,7 @@ static struct net *gtp_genl_get_net(struct net *src_net, struct nlattr *tb[])
 	return net;
 }
 
-static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize)
+static int gtp_dev_hashtable_new(struct gtp_dev *gtp, int hsize)
 {
 	int i;
 
@@ -758,7 +758,7 @@ static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize)
 	return 0;
 }
 
-static void gtp_hashtable_free(struct gtp_dev *gtp)
+static void gtp_dev_hashtable_free(struct gtp_dev *gtp)
 {
 	struct pdp_ctx *pctx;
 	int i;
-- 
2.10.2

^ permalink raw reply related

* [PATCH 08/17] gtp: consolidate pdp context destruction into helper
From: Andreas Schultz @ 2017-01-23 11:56 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index f434f84..5b35ebb 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -84,6 +84,8 @@ struct gtp_net {
 
 static u32 gtp_h_initval;
 
+static void pdp_context_delete(struct pdp_ctx *pctx);
+
 static inline u32 gtp0_hashfn(u64 tid)
 {
 	u32 *tid32 = (u32 *) &tid;
@@ -775,13 +777,10 @@ static void gtp_hashtable_free(struct gtp_dev *gtp)
 	struct pdp_ctx *pctx;
 	int i;
 
-	for (i = 0; i < gtp->hash_size; i++) {
-		hlist_for_each_entry_rcu(pctx, &gtp->tid_hash[i], hlist_tid) {
-			hlist_del_rcu(&pctx->hlist_tid);
-			hlist_del_rcu(&pctx->hlist_addr);
-			kfree_rcu(pctx, rcu_head);
-		}
-	}
+	for (i = 0; i < gtp->hash_size; i++)
+		hlist_for_each_entry_rcu(pctx, &gtp->tid_hash[i], hlist_tid)
+			pdp_context_delete(pctx);
+
 	synchronize_rcu();
 	kfree(gtp->addr_hash);
 	kfree(gtp->tid_hash);
@@ -984,6 +983,13 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info)
 	return 0;
 }
 
+static void pdp_context_delete(struct pdp_ctx *pctx)
+{
+	hlist_del_rcu(&pctx->hlist_tid);
+	hlist_del_rcu(&pctx->hlist_addr);
+	kfree_rcu(pctx, rcu_head);
+}
+
 static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info)
 {
 	struct net_device *dev;
@@ -1082,10 +1088,7 @@ static int gtp_genl_del_pdp(struct sk_buff *skb, struct genl_info *info)
 		pr_debug("GTPv1-U: deleting tunnel id = %x/%x (pdp %p)\n",
 			 pctx->u.v1.i_tei, pctx->u.v1.o_tei, pctx);
 
-	hlist_del_rcu(&pctx->hlist_tid);
-	hlist_del_rcu(&pctx->hlist_addr);
-	kfree_rcu(pctx, rcu_head);
-
+	pdp_context_delete(pctx);
 	return 0;
 }
 
-- 
2.10.2

^ permalink raw reply related

* [PATCH 05/17] gtp: unify genl_find_pdp and prepare for per socket lookup
From: Andreas Schultz @ 2017-01-23 11:56 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

TEID are unique per socket and not per network device. Therefore
the API needs to be changed here.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 99 +++++++++++++++++--------------------------------------
 1 file changed, 31 insertions(+), 68 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index e95c856..7a3c5f6 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1044,56 +1044,61 @@ static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info)
 	return ipv4_pdp_add(dev, info);
 }
 
-static int gtp_genl_del_pdp(struct sk_buff *skb, struct genl_info *info)
+static struct pdp_ctx *gtp_genl_find_pdp_by_link(struct sk_buff *skb,
+						 struct genl_info *info)
 {
 	struct net_device *dev;
-	struct pdp_ctx *pctx;
 	struct gtp_dev *gtp;
 	struct net *net;
+	__be32 ms_addr;
 
-	if (!info->attrs[GTPA_VERSION] ||
-	    !info->attrs[GTPA_LINK])
-		return -EINVAL;
+	if (!info->attrs[GTPA_MS_ADDRESS])
+		return ERR_PTR(-EINVAL);
+	ms_addr = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
 
 	net = gtp_genl_get_net(sock_net(skb->sk), info->attrs);
 	if (IS_ERR(net))
-		return PTR_ERR(net);
+		return ERR_PTR(PTR_ERR(net));
 
 	/* Check if there's an existing gtpX device to configure */
 	dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK]));
 	if (dev == NULL) {
 		put_net(net);
-		return -ENODEV;
+		return ERR_PTR(-ENODEV);
 	}
 	put_net(net);
 
 	gtp = netdev_priv(dev);
 
-	switch (nla_get_u32(info->attrs[GTPA_VERSION])) {
-	case GTP_V0:
-		if (!info->attrs[GTPA_TID])
-			return -EINVAL;
-		pctx = gtp0_pdp_find(gtp, nla_get_u64(info->attrs[GTPA_TID]));
-		break;
-	case GTP_V1:
-		if (!info->attrs[GTPA_I_TEI])
-			return -EINVAL;
-		pctx = gtp1_pdp_find(gtp, nla_get_u64(info->attrs[GTPA_I_TEI]));
-		break;
+	return ipv4_pdp_find(gtp, ms_addr);
+}
 
-	default:
+static struct pdp_ctx *gtp_genl_find_pdp(struct sk_buff *skb,
+					 struct genl_info *info)
+{
+	if (info->attrs[GTPA_LINK])
+		return gtp_genl_find_pdp_by_link(skb, info);
+	else
+		return ERR_PTR(-EINVAL);
+}
+
+static int gtp_genl_del_pdp(struct sk_buff *skb, struct genl_info *info)
+{
+	struct pdp_ctx *pctx;
+
+	if (!info->attrs[GTPA_VERSION])
 		return -EINVAL;
-	}
 
+	pctx = gtp_genl_find_pdp(skb, info);
 	if (IS_ERR(pctx))
 		return PTR_ERR(pctx);
 
 	if (pctx->gtp_version == GTP_V0)
-		netdev_dbg(dev, "GTPv0-U: deleting tunnel id = %llx (pdp %p)\n",
-			   pctx->u.v0.tid, pctx);
+		pr_debug("GTPv0-U: deleting tunnel id = %llx (pdp %p)\n",
+			 pctx->u.v0.tid, pctx);
 	else if (pctx->gtp_version == GTP_V1)
-		netdev_dbg(dev, "GTPv1-U: deleting tunnel id = %x/%x (pdp %p)\n",
-			   pctx->u.v1.i_tei, pctx->u.v1.o_tei, pctx);
+		pr_debug("GTPv1-U: deleting tunnel id = %x/%x (pdp %p)\n",
+			 pctx->u.v1.i_tei, pctx->u.v1.o_tei, pctx);
 
 	hlist_del_rcu(&pctx->hlist_tid);
 	hlist_del_rcu(&pctx->hlist_addr);
@@ -1143,57 +1148,15 @@ static int gtp_genl_fill_info(struct sk_buff *skb, u32 snd_portid, u32 snd_seq,
 static int gtp_genl_get_pdp(struct sk_buff *skb, struct genl_info *info)
 {
 	struct pdp_ctx *pctx = NULL;
-	struct net_device *dev;
 	struct sk_buff *skb2;
-	struct gtp_dev *gtp;
-	u32 gtp_version;
-	struct net *net;
 	int err;
 
-	if (!info->attrs[GTPA_VERSION] ||
-	    !info->attrs[GTPA_LINK])
-		return -EINVAL;
-
-	gtp_version = nla_get_u32(info->attrs[GTPA_VERSION]);
-	switch (gtp_version) {
-	case GTP_V0:
-	case GTP_V1:
-		break;
-	default:
+	if (!info->attrs[GTPA_VERSION])
 		return -EINVAL;
-	}
-
-	net = gtp_genl_get_net(sock_net(skb->sk), info->attrs);
-	if (IS_ERR(net))
-		return PTR_ERR(net);
-
-	/* Check if there's an existing gtpX device to configure */
-	dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK]));
-	if (dev == NULL) {
-		put_net(net);
-		return -ENODEV;
-	}
-	put_net(net);
-
-	gtp = netdev_priv(dev);
 
 	rcu_read_lock();
-	if (gtp_version == GTP_V0 &&
-	    info->attrs[GTPA_TID]) {
-		u64 tid = nla_get_u64(info->attrs[GTPA_TID]);
-
-		pctx = gtp0_pdp_find(gtp, tid);
-	} else if (gtp_version == GTP_V1 &&
-		 info->attrs[GTPA_I_TEI]) {
-		u32 tid = nla_get_u32(info->attrs[GTPA_I_TEI]);
-
-		pctx = gtp1_pdp_find(gtp, tid);
-	} else if (info->attrs[GTPA_MS_ADDRESS]) {
-		__be32 ip = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
-
-		pctx = ipv4_pdp_find(gtp, ip);
-	}
 
+	pctx = gtp_genl_find_pdp(skb, info);
 	if (IS_ERR(pctx)) {
 		err = PTR_ERR(pctx);
 		goto err_unlock;
-- 
2.10.2

^ permalink raw reply related

* [PATCH 01/17] gtp: add genl family modules alias
From: Andreas Schultz @ 2017-01-23 11:56 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

Auto-load the module when userspace asks for the gtp netlink
family.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 8b6810b..7580ccc 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1376,3 +1376,4 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Harald Welte <hwelte@sysmocom.de>");
 MODULE_DESCRIPTION("Interface driver for GTP encapsulated traffic");
 MODULE_ALIAS_RTNL_LINK("gtp");
+MODULE_ALIAS_GENL_FAMILY("gtp");
-- 
2.10.2

^ permalink raw reply related

* [PATCH 12/17] gtp: let userspace handle packets for invalid tunnels
From: Andreas Schultz @ 2017-01-23 11:57 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Harald Welte, Lionel Gauthier, openbsc
In-Reply-To: <20170123115706.4354-1-aschultz@tpip.net>

enable userspace to send error replies for invalid tunnels

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 868467d..290c400 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -186,7 +186,7 @@ static int gtp_rx(struct sk_buff *skb, struct pdp_ctx *pctx, unsigned int hdrlen
 
 	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
 		pr_debug("No PDP ctx for this MS\n");
-		return -1;
+		return 1;
 	}
 
 	/* Get rid of the GTP + UDP headers. */
@@ -236,7 +236,7 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
 	pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid));
 	if (IS_ERR(pctx)) {
 		netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
-		return -1;
+		return 1;
 	}
 
 	return gtp_rx(skb, pctx, hdrlen);
@@ -278,7 +278,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
 	pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
 	if (IS_ERR(pctx)) {
 		netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
-		return -1;
+		return 1;
 	}
 
 	return gtp_rx(skb, pctx, hdrlen);
-- 
2.10.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox