[PATCH v3 1/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v3 1/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications
       [not found] ` <20170208014633.15438-1-stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
@ 2017-02-08  1:46   ` Stefan Brüns
  2017-02-08  9:55     ` Bjørn Mork
  2017-02-08  1:46   ` [PATCH v3 2/2] sierra_net: Skip validating irrelevant fields for IDLE LSIs Stefan Brüns
  1 sibling, 1 reply; 4+ messages in thread
From: Stefan Brüns @ 2017-02-08  1:46 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Brüns

If a context is configured as dualstack ("IPv4v6"), the modem indicates
the context activation with a slightly different indication message.
The dual-stack indication omits the link_type (IPv4/v6) and adds
additional address fields.
IPv6 LSIs are identical to IPv4 LSIs, but have a different link type.

Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
---
v2: Do not overwrite protocol field in rx_fixup
v3: Remove leftover struct ethhdr *eth declaration

Example LSI LINK UP indication:

0000   00 ed 78 00 04 01 00 e9 0a 14 00 54 00 65 00 6c  ..x........T.e.l
0010   00 65 00 6b 00 6f 00 6d 00 2e 00 64 00 65 48 03  .e.k.o.m...d.eH.
0020   c8 be d1 00 62 00 00 00 2c 80 f0 01 00 00 00 00  ....b...,.......
0030   30 cb 04 4c 49 4e 4b 20 55 50 00 00 00 00 00 00  0..LINK UP......
0040   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
0050   00 00 00 00 04 0a 23 38 db 10 2a 01 05 98 88 c0  ......#8..*.....
0060   1f da 00 01 00 01 91 23 a8 f9 00 00 00 00 00 00  .......#........
0070   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
0080   00 04 0a 4a d2 d2 10 2a 01 05 98 07 ff 00 00 00  ...J...*........
0090   10 00 74 02 10 02 10 04 0a 4a d2 d3 10 2a 01 05  ..t......J...*..
00a0   98 07 ff 00 00 00 10 00 74 02 10 02 11 00 00 00  ........t.......
00b0   00 00 00 00 00 00 00 00 00 00 00 00 00 ff ff 00  ................
00c0   00 00 00 00 00 c3 50 04 00 00 00 00 10 fe 80 00  ......P.........
00d0   00 00 00 00 00 00 00 00 00 00 00 00 05 00 00 00  ................
00e0   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00f0   00
---
 drivers/net/usb/sierra_net.c | 101 ++++++++++++++++++++++++++++---------------
 1 file changed, 66 insertions(+), 35 deletions(-)

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index a251588762ec..6300a4454ae5 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -73,8 +73,6 @@ static	atomic_t iface_counter = ATOMIC_INIT(0);
 /* Private data structure */
 struct sierra_net_data {
 
-	u8 ethr_hdr_tmpl[ETH_HLEN]; /* ethernet header template for rx'd pkts */
-
 	u16 link_up;		/* air link up or down */
 	u8 tx_hdr_template[4];	/* part of HIP hdr for tx'd packets */
 
@@ -122,6 +120,7 @@ struct param {
 
 /* LSI Protocol types */
 #define SIERRA_NET_PROTOCOL_UMTS      0x01
+#define SIERRA_NET_PROTOCOL_UMTS_DS   0x04
 /* LSI Coverage */
 #define SIERRA_NET_COVERAGE_NONE      0x00
 #define SIERRA_NET_COVERAGE_NOPACKET  0x01
@@ -129,7 +128,8 @@ struct param {
 /* LSI Session */
 #define SIERRA_NET_SESSION_IDLE       0x00
 /* LSI Link types */
-#define SIERRA_NET_AS_LINK_TYPE_IPv4  0x00
+#define SIERRA_NET_AS_LINK_TYPE_IPV4  0x00
+#define SIERRA_NET_AS_LINK_TYPE_IPV6  0x02
 
 struct lsi_umts {
 	u8 protocol;
@@ -137,9 +137,14 @@ struct lsi_umts {
 	__be16 length;
 	/* eventually use a union for the rest - assume umts for now */
 	u8 coverage;
-	u8 unused2[41];
+	u8 network_len; /* network name len */
+	u8 network[40]; /* network name (UCS2, bigendian) */
 	u8 session_state;
 	u8 unused3[33];
+} __packed;
+
+struct lsi_umts_single {
+	struct lsi_umts lsi;
 	u8 link_type;
 	u8 pdp_addr_len; /* NW-supplied PDP address len */
 	u8 pdp_addr[16]; /* NW-supplied PDP address (bigendian)) */
@@ -158,10 +163,31 @@ struct lsi_umts {
 	u8 reserved[8];
 } __packed;
 
+struct lsi_umts_dual {
+	struct lsi_umts lsi;
+	u8 pdp_addr4_len; /* NW-supplied PDP IPv4 address len */
+	u8 pdp_addr4[4];  /* NW-supplied PDP IPv4 address (bigendian)) */
+	u8 pdp_addr6_len; /* NW-supplied PDP IPv6 address len */
+	u8 pdp_addr6[16]; /* NW-supplied PDP IPv6 address (bigendian)) */
+	u8 unused4[23];
+	u8 dns1_addr4_len; /* NW-supplied 1st DNS v4 address len (bigendian) */
+	u8 dns1_addr4[4];  /* NW-supplied 1st DNS v4 address */
+	u8 dns1_addr6_len; /* NW-supplied 1st DNS v6 address len */
+	u8 dns1_addr6[16]; /* NW-supplied 1st DNS v6 address (bigendian)*/
+	u8 dns2_addr4_len; /* NW-supplied 2nd DNS v4 address len (bigendian) */
+	u8 dns2_addr4[4];  /* NW-supplied 2nd DNS v4 address */
+	u8 dns2_addr6_len; /* NW-supplied 2nd DNS v6 address len */
+	u8 dns2_addr6[16]; /* NW-supplied 2nd DNS v6 address (bigendian)*/
+	u8 unused5[68];
+} __packed;
+
 #define SIERRA_NET_LSI_COMMON_LEN      4
-#define SIERRA_NET_LSI_UMTS_LEN        (sizeof(struct lsi_umts))
+#define SIERRA_NET_LSI_UMTS_LEN        (sizeof(struct lsi_umts_single))
 #define SIERRA_NET_LSI_UMTS_STATUS_LEN \
 	(SIERRA_NET_LSI_UMTS_LEN - SIERRA_NET_LSI_COMMON_LEN)
+#define SIERRA_NET_LSI_UMTS_DS_LEN     (sizeof(struct lsi_umts_dual))
+#define SIERRA_NET_LSI_UMTS_DS_STATUS_LEN \
+	(SIERRA_NET_LSI_UMTS_DS_LEN - SIERRA_NET_LSI_COMMON_LEN)
 
 /* Forward definitions */
 static void sierra_sync_timer(unsigned long syncdata);
@@ -191,10 +217,11 @@ static inline void sierra_net_set_private(struct usbnet *dev,
 	dev->data[0] = (unsigned long)priv;
 }
 
-/* is packet IPv4 */
+/* is packet IPv4/IPv6 */
 static inline int is_ip(struct sk_buff *skb)
 {
-	return skb->protocol == cpu_to_be16(ETH_P_IP);
+	return skb->protocol == cpu_to_be16(ETH_P_IP) ||
+	       skb->protocol == cpu_to_be16(ETH_P_IPV6);
 }
 
 /*
@@ -350,38 +377,43 @@ static inline int sierra_net_is_valid_addrlen(u8 len)
 static int sierra_net_parse_lsi(struct usbnet *dev, char *data, int datalen)
 {
 	struct lsi_umts *lsi = (struct lsi_umts *)data;
+	u32 expected_length;
 
-	if (datalen < sizeof(struct lsi_umts)) {
-		netdev_err(dev->net, "%s: Data length %d, exp %Zu\n",
-				__func__, datalen,
-				sizeof(struct lsi_umts));
-		return -1;
-	}
-
-	if (lsi->length != cpu_to_be16(SIERRA_NET_LSI_UMTS_STATUS_LEN)) {
-		netdev_err(dev->net, "%s: LSI_UMTS_STATUS_LEN %d, exp %u\n",
-				__func__, be16_to_cpu(lsi->length),
-				(u32)SIERRA_NET_LSI_UMTS_STATUS_LEN);
+	if (datalen < sizeof(struct lsi_umts_single)) {
+		netdev_err(dev->net, "%s: Data length %d, exp >= %Zu\n",
+			   __func__, datalen, sizeof(struct lsi_umts_single));
 		return -1;
 	}
 
 	/* Validate the protocol  - only support UMTS for now */
-	if (lsi->protocol != SIERRA_NET_PROTOCOL_UMTS) {
+	if (lsi->protocol == SIERRA_NET_PROTOCOL_UMTS) {
+		struct lsi_umts_single *single = (struct lsi_umts_single *)lsi;
+
+		/* Validate the link type */
+		if (single->link_type != SIERRA_NET_AS_LINK_TYPE_IPV4 &&
+		    single->link_type != SIERRA_NET_AS_LINK_TYPE_IPV6) {
+			netdev_err(dev->net, "Link type unsupported: 0x%02x\n",
+				   single->link_type);
+			return -1;
+		}
+		expected_length = SIERRA_NET_LSI_UMTS_STATUS_LEN;
+	} else if (lsi->protocol == SIERRA_NET_PROTOCOL_UMTS_DS) {
+		expected_length = SIERRA_NET_LSI_UMTS_DS_STATUS_LEN;
+	} else {
 		netdev_err(dev->net, "Protocol unsupported, 0x%02x\n",
-			lsi->protocol);
+			   lsi->protocol);
 		return -1;
 	}
 
-	/* Validate the link type */
-	if (lsi->link_type != SIERRA_NET_AS_LINK_TYPE_IPv4) {
-		netdev_err(dev->net, "Link type unsupported: 0x%02x\n",
-			lsi->link_type);
+	if (be16_to_cpu(lsi->length) != expected_length) {
+		netdev_err(dev->net, "%s: LSI_UMTS_STATUS_LEN %d, exp %u\n",
+			   __func__, be16_to_cpu(lsi->length), expected_length);
 		return -1;
 	}
 
 	/* Validate the coverage */
-	if (lsi->coverage == SIERRA_NET_COVERAGE_NONE
-	   || lsi->coverage == SIERRA_NET_COVERAGE_NOPACKET) {
+	if (lsi->coverage == SIERRA_NET_COVERAGE_NONE ||
+	    lsi->coverage == SIERRA_NET_COVERAGE_NOPACKET) {
 		netdev_err(dev->net, "No coverage, 0x%02x\n", lsi->coverage);
 		return 0;
 	}
@@ -662,7 +694,6 @@ static int sierra_net_bind(struct usbnet *dev, struct usb_interface *intf)
 	u8	numendpoints;
 	u16	fwattr = 0;
 	int	status;
-	struct ethhdr *eth;
 	struct sierra_net_data *priv;
 	static const u8 sync_tmplate[sizeof(priv->sync_msg)] = {
 		0x00, 0x00, SIERRA_NET_HIP_MSYNC_ID, 0x00};
@@ -700,11 +731,6 @@ static int sierra_net_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->net->dev_addr[ETH_ALEN-2] = atomic_inc_return(&iface_counter);
 	dev->net->dev_addr[ETH_ALEN-1] = ifacenum;
 
-	/* we will have to manufacture ethernet headers, prepare template */
-	eth = (struct ethhdr *)priv->ethr_hdr_tmpl;
-	memcpy(&eth->h_dest, dev->net->dev_addr, ETH_ALEN);
-	eth->h_proto = cpu_to_be16(ETH_P_IP);

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications
  2017-02-08  1:46   ` [PATCH v3 1/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications Stefan Brüns
@ 2017-02-08  9:55     ` Bjørn Mork
  2017-02-08 15:23       ` David Miller
  0 siblings, 1 reply; 4+ messages in thread
From: Bjørn Mork @ 2017-02-08  9:55 UTC (permalink / raw)
  To: Stefan Brüns; +Cc: linux-usb, netdev, linux-kernel

Stefan Brüns <stefan.bruens@rwth-aachen.de> writes:

> If a context is configured as dualstack ("IPv4v6"), the modem indicates
> the context activation with a slightly different indication message.
> The dual-stack indication omits the link_type (IPv4/v6) and adds
> additional address fields.

Great!

> IPv6 LSIs are identical to IPv4 LSIs, but have a different link type.
> +struct lsi_umts_dual {
> +	struct lsi_umts lsi;
> +	u8 pdp_addr4_len; /* NW-supplied PDP IPv4 address len */
> +	u8 pdp_addr4[4];  /* NW-supplied PDP IPv4 address (bigendian)) */
> +	u8 pdp_addr6_len; /* NW-supplied PDP IPv6 address len */
> +	u8 pdp_addr6[16]; /* NW-supplied PDP IPv6 address (bigendian)) */


Maybe use "struct in_addr" and "struct in6_addr" for all the address
fields, making them a bit more accessible and avoiding having to define
endianness explicitly?

There is an extra ")" after "(bigendian)" here, BTW.
> -	/* Validate the link type */
> -	if (lsi->link_type != SIERRA_NET_AS_LINK_TYPE_IPv4) {
> -		netdev_err(dev->net, "Link type unsupported: 0x%02x\n",
> -			lsi->link_type);
> +	if (be16_to_cpu(lsi->length) != expected_length) {
> +		netdev_err(dev->net, "%s: LSI_UMTS_STATUS_LEN %d, exp %u\n",
> +			   __func__, be16_to_cpu(lsi->length), expected_length);
>  		return -1;
>  	}


Not that it really matters much, but you could make this ia tiny bit
more efficient on LE by making "expected_length" big endian
instead. Either by using cpu_to_be16() when setting it above, or
redefining the macros with cpu_to_be16().


> @@ -833,9 +859,14 @@ static int sierra_net_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
>  
>  		skb_pull(skb, hh.hdrlen);
>  
> -		/* We are going to accept this packet, prepare it */
> -		memcpy(skb->data, sierra_net_get_private(dev)->ethr_hdr_tmpl,
> -			ETH_HLEN);
> +		/* We are going to accept this packet, prepare it.
> +		 * In case protocol is IPv6, keep it, otherwise force IPv4.
> +		 */
> +		skb_reset_mac_header(skb);
> +		if (eth_hdr(skb)->h_proto != cpu_to_be16(ETH_P_IPV6))
> +			eth_hdr(skb)->h_proto = cpu_to_be16(ETH_P_IP);
> +		eth_zero_addr(eth_hdr(skb)->h_source);
> +		memcpy(eth_hdr(skb)->h_dest, dev->net->dev_addr, ETH_ALEN);
>  
>  		/* Last packet in batch handled by usbnet */
>  		if (hh.payload_len.word == skb->len)

Hmm, the old code would unconditionally overwrite the whole header
including the proto field.  Are you saying that this wasn't really
necessary, and that the proto is usually (always for IPv6) correct?

If so, then this makes sense.  But I think it might be worth a note
about what we replace here and why, in case you happen to know that...
I realise that this is inherited from the original driver.

Just minor nits.  Overall this looks very good to me, FWIW.


Reviewed-by: Bjørn Mork <bjorn@mork.no>



Bjørn

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications
  2017-02-08  9:55     ` Bjørn Mork
@ 2017-02-08 15:23       ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2017-02-08 15:23 UTC (permalink / raw)
  To: bjorn; +Cc: stefan.bruens, linux-usb, netdev, linux-kernel

From: Bjørn Mork <bjorn@mork.no>
Date: Wed, 08 Feb 2017 10:55:50 +0100

> Stefan Brüns <stefan.bruens@rwth-aachen.de> writes:
> 
>> If a context is configured as dualstack ("IPv4v6"), the modem indicates
>> the context activation with a slightly different indication message.
>> The dual-stack indication omits the link_type (IPv4/v6) and adds
>> additional address fields.
> 
> Great!
> 
>> IPv6 LSIs are identical to IPv4 LSIs, but have a different link type.
>> +struct lsi_umts_dual {
>> +	struct lsi_umts lsi;
>> +	u8 pdp_addr4_len; /* NW-supplied PDP IPv4 address len */
>> +	u8 pdp_addr4[4];  /* NW-supplied PDP IPv4 address (bigendian)) */
>> +	u8 pdp_addr6_len; /* NW-supplied PDP IPv6 address len */
>> +	u8 pdp_addr6[16]; /* NW-supplied PDP IPv6 address (bigendian)) */
> 
> 
> Maybe use "struct in_addr" and "struct in6_addr" for all the address
> fields, making them a bit more accessible and avoiding having to define
> endianness explicitly?

Can't do that, it will add padding to the structure and not match
what is really in the header.

The problem is that we get a "u8" length first which starts the
address on an odd byte.

And we don't want to use "__packed" to fix this either.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] sierra_net: Skip validating irrelevant fields for IDLE LSIs
       [not found] ` <20170208014633.15438-1-stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
  2017-02-08  1:46   ` [PATCH v3 1/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications Stefan Brüns
@ 2017-02-08  1:46   ` Stefan Brüns
  1 sibling, 0 replies; 4+ messages in thread
From: Stefan Brüns @ 2017-02-08  1:46 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Brüns

When the context is deactivated, the link_type is set to 0xff, which
triggers a warning message, and results in a wrong link status, as
the LSI is ignored.

Signed-off-by: Stefan Brüns <stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
---
 drivers/net/usb/sierra_net.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index 6300a4454ae5..0b5a84c9022c 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -385,6 +385,13 @@ static int sierra_net_parse_lsi(struct usbnet *dev, char *data, int datalen)
 		return -1;
 	}
 
+	/* Validate the session state */
+	if (lsi->session_state == SIERRA_NET_SESSION_IDLE) {
+		netdev_err(dev->net, "Session idle, 0x%02x\n",
+			   lsi->session_state);
+		return 0;
+	}
+
 	/* Validate the protocol  - only support UMTS for now */
 	if (lsi->protocol == SIERRA_NET_PROTOCOL_UMTS) {
 		struct lsi_umts_single *single = (struct lsi_umts_single *)lsi;
@@ -418,13 +425,6 @@ static int sierra_net_parse_lsi(struct usbnet *dev, char *data, int datalen)
 		return 0;
 	}
 
-	/* Validate the session state */
-	if (lsi->session_state == SIERRA_NET_SESSION_IDLE) {
-		netdev_err(dev->net, "Session idle, 0x%02x\n",
-			lsi->session_state);
-		return 0;
-	}

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-02-08 15:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20170208014633.15438-1-stefan.bruens@rwth-aachen.de>
     [not found] ` <20170208014633.15438-1-stefan.bruens-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org>
2017-02-08  1:46   ` [PATCH v3 1/2] sierra_net: Add support for IPv6 and Dual-Stack Link Sense Indications Stefan Brüns
2017-02-08  9:55     ` Bjørn Mork
2017-02-08 15:23       ` David Miller
2017-02-08  1:46   ` [PATCH v3 2/2] sierra_net: Skip validating irrelevant fields for IDLE LSIs Stefan Brüns

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).