[PATCH net-next v2 0/6] net: qualcomm: rmnet: Configuration options

[PATCH net-next v2 0/6] net: qualcomm: rmnet: Configuration options

[Subash Abhinov Kasiviswanathan]

This series adds support for configuring features on rmnet devices.
The rmnet specific features to be configured here are aggregation and
control commands.

Patch 1 is a cleanup of return codes in the transmit path.
Patch 2 removes some redundant ingress and egress macros.
Patch 3 restricts the creation of rmnet dev to one dev per mux id for a
given real dev.
Patch 4 adds ethernet data path support.
Patches 5-6 add support for configuring features on new and existing
rmnet devices.

v1->v2:
The memory leak fixed as part of patch 1 is merged seperately as
a896d94abd2c ("net: qualcomm: rmnet: Fix leak on transmit failure"). 
As a result, a896d94abd2c ("net: qualcomm: rmnet: Fix leak on transmit failure")
from net needs to be applied first before applying this series in net-next.

Fix a use after free in patch 4 if a packet with headroom lesser than ethernet
header length is received.

Subash Abhinov Kasiviswanathan (6):
  net: qualcomm: rmnet: Remove the rmnet_map_results enum
  net: qualcomm: rmnet: Remove the some redundant macros
  net: qualcomm: rmnet: Allow only one rmnet dev per muxid per real dev
  net: qualcomm: rmnet: Process packets over ethernet
  net: qualcomm: rmnet: Allow to configure flags for new devices
  net: qualcomm: rmnet: Allow to configure flags for existing devices

 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 64 ++++++++++++++++++----
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h |  1 -
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c   | 42 +++++++-------
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h    |  9 ---
 .../net/ethernet/qualcomm/rmnet/rmnet_private.h    | 10 +---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c    |  3 +
 6 files changed, 78 insertions(+), 51 deletions(-)

-- 
1.9.1

[PATCH net-next v2 1/6] net: qualcomm: rmnet: Remove the rmnet_map_results enum

[Subash Abhinov Kasiviswanathan]

Only the success and consumed entries were actually in use.
Use standard error codes instead.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 15 +++------------
 drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h      |  9 ---------
 2 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 08e4afc..1e1ea10 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -142,11 +142,11 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
 
 	skb->protocol = htons(ETH_P_MAP);
 
-	return RMNET_MAP_SUCCESS;
+	return 0;
 
 fail:
 	kfree_skb(skb);
-	return RMNET_MAP_CONSUMED;
+	return -ENOMEM;
 }
 
 static void
@@ -213,17 +213,8 @@ void rmnet_egress_handler(struct sk_buff *skb)
 	}
 
 	if (port->egress_data_format & RMNET_EGRESS_FORMAT_MAP) {
-		switch (rmnet_map_egress_handler(skb, port, mux_id, orig_dev)) {
-		case RMNET_MAP_CONSUMED:
+		if (rmnet_map_egress_handler(skb, port, mux_id, orig_dev))
 			return;
-
-		case RMNET_MAP_SUCCESS:
-			break;
-
-		default:
-			kfree_skb(skb);
-			return;
-		}
 	}
 
 	rmnet_vnd_tx_fixup(skb, orig_dev);
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index 3af3fe7..4df359d 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -30,15 +30,6 @@ struct rmnet_map_control_command {
 	};
 }  __aligned(1);
 
-enum rmnet_map_results {
-	RMNET_MAP_SUCCESS,
-	RMNET_MAP_CONSUMED,
-	RMNET_MAP_GENERAL_FAILURE,
-	RMNET_MAP_NOT_ENABLED,
-	RMNET_MAP_FAILED_AGGREGATION,
-	RMNET_MAP_FAILED_MUX
-};
-
 enum rmnet_map_commands {
 	RMNET_MAP_COMMAND_NONE,
 	RMNET_MAP_COMMAND_FLOW_DISABLE,
-- 
1.9.1

[PATCH net-next v2 2/6] net: qualcomm: rmnet: Remove the some redundant macros

[Subash Abhinov Kasiviswanathan]

Multiplexing is always enabled when transmiting from a rmnet device,
so remove the redundant egress macros. De-multiplexing is always
enabled when receiving packets from a rmnet device, so remove those
ingress macros.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c   | 10 ++--------
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h   |  1 -
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 19 +++++++------------
 drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h  | 10 ++--------
 4 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 71bee1a..5e530db 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -143,11 +143,7 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
 			 struct nlattr *tb[], struct nlattr *data[],
 			 struct netlink_ext_ack *extack)
 {
-	int ingress_format = RMNET_INGRESS_FORMAT_DEMUXING |
-			     RMNET_INGRESS_FORMAT_DEAGGREGATION |
-			     RMNET_INGRESS_FORMAT_MAP;
-	int egress_format = RMNET_EGRESS_FORMAT_MUXING |
-			    RMNET_EGRESS_FORMAT_MAP;
+	int ingress_format = RMNET_INGRESS_FORMAT_DEAGGREGATION;
 	struct net_device *real_dev;
 	int mode = RMNET_EPMODE_VND;
 	struct rmnet_endpoint *ep;
@@ -181,9 +177,7 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
 	if (err)
 		goto err2;
 
-	netdev_dbg(dev, "data format [ingress 0x%08X] [egress 0x%08X]\n",
-		   ingress_format, egress_format);
-	port->egress_data_format = egress_format;
+	netdev_dbg(dev, "data format [ingress 0x%08X]\n", ingress_format);
 	port->ingress_data_format = ingress_format;
 	port->rmnet_mode = mode;
 
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index c19259e..2ea9fe3 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -33,7 +33,6 @@ struct rmnet_endpoint {
 struct rmnet_port {
 	struct net_device *dev;
 	u32 ingress_data_format;
-	u32 egress_data_format;
 	u8 nr_rmnet_devs;
 	u8 rmnet_mode;
 	struct hlist_head muxed_ep[RMNET_MAX_LOGICAL_EP];
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 1e1ea10..a46053c 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -133,12 +133,10 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
 	if (!map_header)
 		goto fail;
 
-	if (port->egress_data_format & RMNET_EGRESS_FORMAT_MUXING) {
-		if (mux_id == 0xff)
-			map_header->mux_id = 0;
-		else
-			map_header->mux_id = mux_id;
-	}
+	if (mux_id == 0xff)
+		map_header->mux_id = 0;
+	else
+		map_header->mux_id = mux_id;
 
 	skb->protocol = htons(ETH_P_MAP);
 
@@ -178,8 +176,7 @@ rx_handler_result_t rmnet_rx_handler(struct sk_buff **pskb)
 
 	switch (port->rmnet_mode) {
 	case RMNET_EPMODE_VND:
-		if (port->ingress_data_format & RMNET_INGRESS_FORMAT_MAP)
-			rmnet_map_ingress_handler(skb, port);
+		rmnet_map_ingress_handler(skb, port);
 		break;
 	case RMNET_EPMODE_BRIDGE:
 		rmnet_bridge_handler(skb, port->bridge_ep);
@@ -212,10 +209,8 @@ void rmnet_egress_handler(struct sk_buff *skb)
 		return;
 	}
 
-	if (port->egress_data_format & RMNET_EGRESS_FORMAT_MAP) {
-		if (rmnet_map_egress_handler(skb, port, mux_id, orig_dev))
-			return;
-	}
+	if (rmnet_map_egress_handler(skb, port, mux_id, orig_dev))
+		return;
 
 	rmnet_vnd_tx_fixup(skb, orig_dev);
 
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h
index 49102f9..d214280 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_private.h
@@ -19,14 +19,8 @@
 #define RMNET_TX_QUEUE_LEN         1000
 
 /* Constants */
-#define RMNET_EGRESS_FORMAT_MAP                 BIT(1)
-#define RMNET_EGRESS_FORMAT_AGGREGATION         BIT(2)
-#define RMNET_EGRESS_FORMAT_MUXING              BIT(3)
-
-#define RMNET_INGRESS_FORMAT_MAP                BIT(1)
-#define RMNET_INGRESS_FORMAT_DEAGGREGATION      BIT(2)
-#define RMNET_INGRESS_FORMAT_DEMUXING           BIT(3)
-#define RMNET_INGRESS_FORMAT_MAP_COMMANDS       BIT(4)
+#define RMNET_INGRESS_FORMAT_DEAGGREGATION      BIT(0)
+#define RMNET_INGRESS_FORMAT_MAP_COMMANDS       BIT(1)
 
 /* Replace skb->dev to a virtual rmnet device and pass up the stack */
 #define RMNET_EPMODE_VND (1)
-- 
1.9.1

[PATCH net-next v2 3/6] net: qualcomm: rmnet: Allow only one rmnet dev per muxid per real dev

[Subash Abhinov Kasiviswanathan]

Upon de-multiplexing data from one real dev, the packets can be sent
to an unique rmnet device for a given mux id.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 9caa5e3..5bb29f4 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -185,6 +185,9 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
 	if (ep->egress_dev)
 		return -EINVAL;
 
+	if (rmnet_get_endpoint(port, id))
+		return -EBUSY;
+
 	rc = register_netdevice(rmnet_dev);
 	if (!rc) {
 		ep->egress_dev = rmnet_dev;
-- 
1.9.1

[PATCH net-next v2 4/6] net: qualcomm: rmnet: Process packets over ethernet

[Subash Abhinov Kasiviswanathan]

Add support to send and receive packets over ethernet.
An example of usage is testing the data path on UML. This can be
achieved by setting up two UML instances in multicast mode and
associating rmnet over the UML ethernet device.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index a46053c..0553932 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -15,6 +15,7 @@
 
 #include <linux/netdevice.h>
 #include <linux/netdev_features.h>
+#include <linux/if_arp.h>
 #include "rmnet_private.h"
 #include "rmnet_config.h"
 #include "rmnet_vnd.h"
@@ -104,6 +105,15 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 {
 	struct sk_buff *skbn;
 
+	if (skb->dev->type == ARPHRD_ETHER) {
+		if (pskb_expand_head(skb, ETH_HLEN, 0, GFP_KERNEL)) {
+			kfree_skb(skb);
+			return;
+		}
+
+		skb_push(skb, ETH_HLEN);
+	}
+
 	if (port->ingress_data_format & RMNET_INGRESS_FORMAT_DEAGGREGATION) {
 		while ((skbn = rmnet_map_deaggregate(skb)) != NULL)
 			__rmnet_map_ingress_handler(skbn, port);
-- 
1.9.1

[PATCH net-next v2 5/6] net: qualcomm: rmnet: Allow to configure flags for new devices

[Subash Abhinov Kasiviswanathan]

Add an option to configure the rmnet aggregation and command features
on device creation. This is achieved by using the vlan flags option.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 5e530db..2f5f661 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -177,11 +177,20 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
 	if (err)
 		goto err2;
 
-	netdev_dbg(dev, "data format [ingress 0x%08X]\n", ingress_format);
-	port->ingress_data_format = ingress_format;
 	port->rmnet_mode = mode;
 
 	hlist_add_head_rcu(&ep->hlnode, &port->muxed_ep[mux_id]);
+
+	if (data[IFLA_VLAN_FLAGS]) {
+		struct ifla_vlan_flags *flags;
+
+		flags = nla_data(data[IFLA_VLAN_FLAGS]);
+		ingress_format = flags->flags & flags->mask;
+	}
+
+	netdev_dbg(dev, "data format [ingress 0x%08X]\n", ingress_format);
+	port->ingress_data_format = ingress_format;
+
 	return 0;
 
 err2:
@@ -312,7 +321,8 @@ static int rmnet_rtnl_validate(struct nlattr *tb[], struct nlattr *data[],
 
 static size_t rmnet_get_size(const struct net_device *dev)
 {
-	return nla_total_size(2); /* IFLA_VLAN_ID */
+	return nla_total_size(2) /* IFLA_VLAN_ID */ +
+	nla_total_size(sizeof(struct ifla_vlan_flags)); /* IFLA_VLAN_FLAGS */
 }
 
 struct rtnl_link_ops rmnet_link_ops __read_mostly = {
-- 
1.9.1

[PATCH net-next v2 6/6] net: qualcomm: rmnet: Allow to configure flags for existing devices

[Subash Abhinov Kasiviswanathan]

Add an option to configure the mux id, aggregation and commad feature
for existing rmnet devices. Implement the changelink netlink
operation for this.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 2f5f661..3ec4092 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -319,6 +319,45 @@ static int rmnet_rtnl_validate(struct nlattr *tb[], struct nlattr *data[],
 	return 0;
 }
 
+static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
+			    struct nlattr *data[],
+			    struct netlink_ext_ack *extack)
+{
+	struct rmnet_priv *priv = netdev_priv(dev);
+	struct net_device *real_dev;
+	struct rmnet_endpoint *ep;
+	struct rmnet_port *port;
+	u16 mux_id;
+
+	real_dev = __dev_get_by_index(dev_net(dev),
+				      nla_get_u32(tb[IFLA_LINK]));
+
+	if (!real_dev || !dev || !rmnet_is_real_dev_registered(real_dev))
+		return -ENODEV;
+
+	port = rmnet_get_port_rtnl(real_dev);
+
+	if (data[IFLA_VLAN_ID]) {
+		mux_id = nla_get_u16(data[IFLA_VLAN_ID]);
+		ep = rmnet_get_endpoint(port, priv->mux_id);
+
+		hlist_del_init_rcu(&ep->hlnode);
+		hlist_add_head_rcu(&ep->hlnode, &port->muxed_ep[mux_id]);
+
+		ep->mux_id = mux_id;
+		priv->mux_id = mux_id;
+	}
+
+	if (data[IFLA_VLAN_FLAGS]) {
+		struct ifla_vlan_flags *flags;
+
+		flags = nla_data(data[IFLA_VLAN_FLAGS]);
+		port->ingress_data_format = flags->flags & flags->mask;
+	}
+
+	return 0;
+}
+
 static size_t rmnet_get_size(const struct net_device *dev)
 {
 	return nla_total_size(2) /* IFLA_VLAN_ID */ +
@@ -334,6 +373,7 @@ struct rtnl_link_ops rmnet_link_ops __read_mostly = {
 	.newlink	= rmnet_newlink,
 	.dellink	= rmnet_dellink,
 	.get_size	= rmnet_get_size,
+	.changelink     = rmnet_changelink,
 };
 
 /* Needs either rcu_read_lock() or rtnl lock */
-- 
1.9.1

Re: [PATCH net-next v2 5/6] net: qualcomm: rmnet: Allow to configure flags for new devices

[David Miller]

From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Sat,  9 Dec 2017 13:58:29 -0700

> @@ -312,7 +321,8 @@ static int rmnet_rtnl_validate(struct nlattr *tb[], struct nlattr *data[],
>  
>  static size_t rmnet_get_size(const struct net_device *dev)
>  {
> -	return nla_total_size(2); /* IFLA_VLAN_ID */
> +	return nla_total_size(2) /* IFLA_VLAN_ID */ +
> +	nla_total_size(sizeof(struct ifla_vlan_flags)); /* IFLA_VLAN_FLAGS */
>  }
>  

Please indent this return statement properly, the second line of the
expression definitely should not start at the same column as the
'return'.

Thank you.

Re: [PATCH net-next v2 5/6] net: qualcomm: rmnet: Allow to configure flags for new devices

[Dan Williams]

On Sat, 2017-12-09 at 13:58 -0700, Subash Abhinov Kasiviswanathan
wrote:
> Add an option to configure the rmnet aggregation and command features
> on device creation. This is achieved by using the vlan flags option.

Still seems kinda odd to overload IFLA_VLAN_FLAGS to carry
RMNET_INGRESS/EGRESS_FORMAT_* flags, but I'll leave that decision to
others...

Dan

> Signed-off-by: Subash Abhinov Kasiviswanathan
> <subashab@codeaurora.org>
> ---
>  drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 16
> +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index 5e530db..2f5f661 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> @@ -177,11 +177,20 @@ static int rmnet_newlink(struct net *src_net,
> struct net_device *dev,
>  	if (err)
>  		goto err2;
>  
> -	netdev_dbg(dev, "data format [ingress 0x%08X]\n",
> ingress_format);
> -	port->ingress_data_format = ingress_format;
>  	port->rmnet_mode = mode;
>  
>  	hlist_add_head_rcu(&ep->hlnode, &port->muxed_ep[mux_id]);
> +
> +	if (data[IFLA_VLAN_FLAGS]) {
> +		struct ifla_vlan_flags *flags;
> +
> +		flags = nla_data(data[IFLA_VLAN_FLAGS]);
> +		ingress_format = flags->flags & flags->mask;
> +	}
> +
> +	netdev_dbg(dev, "data format [ingress 0x%08X]\n",
> ingress_format);
> +	port->ingress_data_format = ingress_format;
> +
>  	return 0;
>  
>  err2:
> @@ -312,7 +321,8 @@ static int rmnet_rtnl_validate(struct nlattr
> *tb[], struct nlattr *data[],
>  
>  static size_t rmnet_get_size(const struct net_device *dev)
>  {
> -	return nla_total_size(2); /* IFLA_VLAN_ID */
> +	return nla_total_size(2) /* IFLA_VLAN_ID */ +
> +	nla_total_size(sizeof(struct ifla_vlan_flags)); /*
> IFLA_VLAN_FLAGS */
>  }
>  
>  struct rtnl_link_ops rmnet_link_ops __read_mostly = {