Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 1/2 RESEND v3] net: use NETIF_F_ALL_TSO for vlan features
From: Shan Wei @ 2011-05-13  2:10 UTC (permalink / raw)
  To: Dimitris Michailidis; +Cc: David Miller, netdev, eilong, leedom
In-Reply-To: <4DCC1372.3040607@chelsio.com>

Dimitris Michailidis wrote, at 2011年05月13日 01:05:
> But vlan_features doesn't indicate by itself what the device supports, and it is because of this that you can use NETIF_F_ALL_TSO in vlan_features without much future breakage risk.  There's still the potential that in the future some additional TSO variant will be added that some of the drivers you're changing support, but not over VLANs, and then they'll need to undo you change.
 
For this part.

David, please discard this patch.
 
-- 
Best Regards
-----
Shan Wei

^ permalink raw reply

* question about drivers/isdn/hisax/st5481_init.c
From: Julia Lawall @ 2011-05-13  5:53 UTC (permalink / raw)
  To: Karsten Keil, netdev, linux-kernel

The function probe_st5481 allocates an adapter value using kzalloc, adds 
it to adapter_list, and then performs various initialization operations, 
which may fail.  adapter_list is a static variable that is never otherwise 
referenced in the file.  There is a list_del that removes the adapter from 
the list in the function disconnect_st5481.  The presence of the adapter 
on the list makes it possibly unsafe to free adapter in the failure cases.

Could the list just be removed, if it is not being used anywhere?

Or if the list should be kept because it is useful or it is planned to be 
useful in the future, could the insertion into the list be moved to the 
end of the function, after the potentially failing operations, so that 
adapter can be freed when a failure occurs?

julia

^ permalink raw reply

* [PATCH 1/1] IPVS: seq_release_net should be used.
From: Hans Schillstrom @ 2011-05-13  6:03 UTC (permalink / raw)
  To: horms, ja, lvs-devel, netdev, netfilter-devel; +Cc: hans, Hans Schillstrom

Without this patch every access to ip_vs in procfs will increase
the netns count i.e. an unbalanced get_net()/put_net().
(ipvsadm commands also use procfs.)
The result is you can't exit a netns if reading ip_vs_* procfs entries.

Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
---
 net/netfilter/ipvs/ip_vs_app.c  |    2 +-
 net/netfilter/ipvs/ip_vs_conn.c |    4 ++--
 net/netfilter/ipvs/ip_vs_ctl.c  |    6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c
index 51f3af7..059af31 100644
--- a/net/netfilter/ipvs/ip_vs_app.c
+++ b/net/netfilter/ipvs/ip_vs_app.c
@@ -572,7 +572,7 @@ static const struct file_operations ip_vs_app_fops = {
 	.open	 = ip_vs_app_open,
 	.read	 = seq_read,
 	.llseek  = seq_lseek,
-	.release = seq_release,
+	.release = seq_release_net,
 };
 #endif
 
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index d3fd91b..bf28ac2 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -1046,7 +1046,7 @@ static const struct file_operations ip_vs_conn_fops = {
 	.open    = ip_vs_conn_open,
 	.read    = seq_read,
 	.llseek  = seq_lseek,
-	.release = seq_release,
+	.release = seq_release_net,
 };
 
 static const char *ip_vs_origin_name(unsigned flags)
@@ -1114,7 +1114,7 @@ static const struct file_operations ip_vs_conn_sync_fops = {
 	.open    = ip_vs_conn_sync_open,
 	.read    = seq_read,
 	.llseek  = seq_lseek,
-	.release = seq_release,
+	.release = seq_release_net,
 };
 
 #endif
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 89842f0..699c79a 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2066,7 +2066,7 @@ static const struct file_operations ip_vs_info_fops = {
 	.open    = ip_vs_info_open,
 	.read    = seq_read,
 	.llseek  = seq_lseek,
-	.release = seq_release_private,
+	.release = seq_release_net,
 };
 
 static int ip_vs_stats_show(struct seq_file *seq, void *v)
@@ -2106,7 +2106,7 @@ static const struct file_operations ip_vs_stats_fops = {
 	.open = ip_vs_stats_seq_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
-	.release = single_release,
+	.release = single_release_net,
 };
 
 static int ip_vs_stats_percpu_show(struct seq_file *seq, void *v)
@@ -2175,7 +2175,7 @@ static const struct file_operations ip_vs_stats_percpu_fops = {
 	.open = ip_vs_stats_percpu_seq_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
-	.release = single_release,
+	.release = single_release_net,
 };
 #endif
 
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH] net: netlink: don't try unicast when dst_pid is zero for NETLINK_USERSOCK
From: Changli Gao @ 2011-05-13  6:24 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Changli Gao

For NETLINK_USERSOCK, no one listens on PID 0, so sending a message only to
to a multicast group should not return -ECONNREFUSED.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 net/netlink/af_netlink.c |    2 ++
 1 file changed, 2 insertions(+)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index c8f35b5..ba89304 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1379,6 +1379,8 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock,
 	if (dst_group) {
 		atomic_inc(&skb->users);
 		netlink_broadcast(sk, skb, dst_pid, dst_group, GFP_KERNEL);
+		if (dst_pid == 0 && sk->sk_protocol == NETLINK_USERSOCK)
+			return len;
 	}
 	err = netlink_unicast(sk, skb, dst_pid, msg->msg_flags&MSG_DONTWAIT);
 

^ permalink raw reply related

* [net-next 1/2 (V2)] stmmac: don't go through ethtool to start auto-negotiation
From: Giuseppe CAVALLARO @ 2011-05-13  6:28 UTC (permalink / raw)
  To: netdev; +Cc: David Decotigny, Giuseppe Cavallaro

From: David Decotigny <decot@google.com>

The driver used to call phy's ethtool configuration routine to start
auto-negotiation. This change has it call directly phy's routine to
start auto-negotiation.

The initial version was hiding phy_start_aneg() return value,
this patch returns it (<0 upon error).

Tested: module compiles, tested on STM HDK7108 STB.

Signed-off-by: David Decotigny <decot@google.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/stmmac_ethtool.c |   13 ++-----------
 1 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/net/stmmac/stmmac_ethtool.c b/drivers/net/stmmac/stmmac_ethtool.c
index 6f5aaeb..9c05cf0 100644
--- a/drivers/net/stmmac/stmmac_ethtool.c
+++ b/drivers/net/stmmac/stmmac_ethtool.c
@@ -236,17 +236,8 @@ stmmac_set_pauseparam(struct net_device *netdev,
 	priv->flow_ctrl = new_pause;
 
 	if (phy->autoneg) {
-		if (netif_running(netdev)) {
-			struct ethtool_cmd cmd = { .cmd = ETHTOOL_SSET };
-			/* auto-negotiation automatically restarted */
-			cmd.supported = phy->supported;
-			cmd.advertising = phy->advertising;
-			cmd.autoneg = phy->autoneg;
-			ethtool_cmd_speed_set(&cmd, phy->speed);
-			cmd.duplex = phy->duplex;
-			cmd.phy_address = phy->addr;
-			ret = phy_ethtool_sset(phy, &cmd);
-		}
+		if (netif_running(netdev))
+			ret = phy_start_aneg(phy);
 	} else
 		priv->hw->mac->flow_ctrl(priv->ioaddr, phy->duplex,
 					 priv->flow_ctrl, priv->pause);
-- 
1.7.4.4


^ permalink raw reply related

* [net-next 2/2] stmmac: fix autoneg in set_pauseparam
From: Giuseppe CAVALLARO @ 2011-05-13  6:28 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro
In-Reply-To: <1305268085-603-1-git-send-email-peppe.cavallaro@st.com>

This patch fixes a bug in the set_pauseparam
function that didn't well manage the ANE
field and returned broken values when use
ethtool -A|-a.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/stmmac_ethtool.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/stmmac/stmmac_ethtool.c b/drivers/net/stmmac/stmmac_ethtool.c
index 9c05cf0..ae5213a 100644
--- a/drivers/net/stmmac/stmmac_ethtool.c
+++ b/drivers/net/stmmac/stmmac_ethtool.c
@@ -234,6 +234,7 @@ stmmac_set_pauseparam(struct net_device *netdev,
 		new_pause |= FLOW_TX;
 
 	priv->flow_ctrl = new_pause;
+	phy->autoneg = pause->autoneg;
 
 	if (phy->autoneg) {
 		if (netif_running(netdev))
-- 
1.7.4.4


^ permalink raw reply related

* Re: [PATCH 1/2] net/stmmac: don't go through ethtool to start autonegociation
From: Giuseppe CAVALLARO @ 2011-05-13  6:31 UTC (permalink / raw)
  To: David Decotigny
  Cc: Giuseppe CAVALLARO, David S. Miller, Joe Perches,
	Stanislaw Gruszka, netdev, linux-kernel
In-Reply-To: <4DC941D4.8030409@st.com>

On 5/10/2011 3:47 PM, Giuseppe CAVALLARO wrote:
> On 5/10/2011 2:19 AM, David Decotigny wrote:
>> The driver used to call phy's ethtool configuration routine to start
>> autonegociation. This change has it call directly phy's routine to
>> start autonegociation.
>>
>> IMPORTANT: initial version was hiding phy_start_aneg() return value,
>>            this patch returns it (<0 upon error).
>>
>> Tested: module compiles, NOT tested on real hardware.
>> Signed-off-by: David Decotigny <decot@google.com>
> 
> Sorry for the delay, I'm doing some tests with the stmmac on a "real"
> HW. I'll come back asap.


Hello.

I've tested the patch and, as discussed with David, I have sent it again
plus a new fix in the stmmac_set_pauseparam I have done while testing
ethtool -A|-a on my ST box.

Best Regards,
Peppe

> 
> Regards
> Peppe
> 
>> ---
>>  drivers/net/stmmac/stmmac_ethtool.c |   13 ++-----------
>>  1 files changed, 2 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/stmmac/stmmac_ethtool.c b/drivers/net/stmmac/stmmac_ethtool.c
>> index 6f5aaeb..9c05cf0 100644
>> --- a/drivers/net/stmmac/stmmac_ethtool.c
>> +++ b/drivers/net/stmmac/stmmac_ethtool.c
>> @@ -236,17 +236,8 @@ stmmac_set_pauseparam(struct net_device *netdev,
>>  	priv->flow_ctrl = new_pause;
>>  
>>  	if (phy->autoneg) {
>> -		if (netif_running(netdev)) {
>> -			struct ethtool_cmd cmd = { .cmd = ETHTOOL_SSET };
>> -			/* auto-negotiation automatically restarted */
>> -			cmd.supported = phy->supported;
>> -			cmd.advertising = phy->advertising;
>> -			cmd.autoneg = phy->autoneg;
>> -			ethtool_cmd_speed_set(&cmd, phy->speed);
>> -			cmd.duplex = phy->duplex;
>> -			cmd.phy_address = phy->addr;
>> -			ret = phy_ethtool_sset(phy, &cmd);
>> -		}
>> +		if (netif_running(netdev))
>> +			ret = phy_start_aneg(phy);
>>  	} else
>>  		priv->hw->mac->flow_ctrl(priv->ioaddr, phy->duplex,
>>  					 priv->flow_ctrl, priv->pause);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" 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: [PATCH net-next] net:set valid name before calling ndo_init()
From: Jiri Pirko @ 2011-05-13  7:15 UTC (permalink / raw)
  To: Weiping Pan
  Cc: David S. Miller, Eric Dumazet, Michał Mirosław,
	Tom Herbert, Ben Hutchings, open list:NETWORKING [GENERAL],
	open list
In-Reply-To: <1305251217-4254-1-git-send-email-panweiping3@gmail.com>

Fri, May 13, 2011 at 03:46:56AM CEST, panweiping3@gmail.com wrote:
>In commit 1c5cae815d19 (net: call dev_alloc_name from register_netdevice),
>a bug of bonding was invloved, see example 1 and 2.
>
>In register_netdevice(), the name of net_device is not valid until
>dev_get_valid_name() is called. But dev->netdev_ops->ndo_init(that is
>bond_init) is called before dev_get_valid_name(),
>and it uses the invalid name of net_device.
>
>I think register_netdevice() should make sure that the name of net_device is
>valid before calling ndo_init().
>
>example 1:
>modprobe bonding
>ls  /proc/net/bonding/bond%d
>
>ps -eLf
>root      3398     2  3398  0    1 21:34 ?        00:00:00 [bond%d]
>
>example 2:
>modprobe bonding max_bonds=3
>
>[  170.100292] bonding: Ethernet Channel Bonding Driver: v3.7.1 (April 27, 2011)
>[  170.101090] bonding: Warning: either miimon or arp_interval and arp_ip_target module parameters must be specified, otherwise bonding will not detect link failures! see bonding.txt for details.
>[  170.102469] ------------[ cut here ]------------
>[  170.103150] WARNING: at /home/pwp/net-next-2.6/fs/proc/generic.c:586 proc_register+0x126/0x157()
>[  170.104075] Hardware name: VirtualBox
>[  170.105065] proc_dir_entry 'bonding/bond%d' already registered
>[  170.105613] Modules linked in: bonding(+) sunrpc ipv6 uinput microcode ppdev parport_pc parport joydev e1000 pcspkr i2c_piix4 i2c_core [last unloaded: bonding]
>[  170.108397] Pid: 3457, comm: modprobe Not tainted 2.6.39-rc2+ #14
>[  170.108935] Call Trace:
>[  170.109382]  [<c0438f3b>] warn_slowpath_common+0x6a/0x7f
>[  170.109911]  [<c051a42a>] ? proc_register+0x126/0x157
>[  170.110329]  [<c0438fc3>] warn_slowpath_fmt+0x2b/0x2f
>[  170.110846]  [<c051a42a>] proc_register+0x126/0x157
>[  170.111870]  [<c051a4dd>] proc_create_data+0x82/0x98
>[  170.112335]  [<f94e6af6>] bond_create_proc_entry+0x3f/0x73 [bonding]
>[  170.112905]  [<f94dd806>] bond_init+0x77/0xa5 [bonding]
>[  170.113319]  [<c0721ac6>] register_netdevice+0x8c/0x1d3
>[  170.113848]  [<f94e0e30>] bond_create+0x6c/0x90 [bonding]
>[  170.114322]  [<f94f4763>] bonding_init+0x763/0x7b1 [bonding]
>[  170.114879]  [<c0401240>] do_one_initcall+0x76/0x122
>[  170.115317]  [<f94f4000>] ? 0xf94f3fff
>[  170.115799]  [<c0463f1e>] sys_init_module+0x1286/0x140d
>[  170.116879]  [<c07c6d9f>] sysenter_do_call+0x12/0x28
>[  170.117404] ---[ end trace 64e4fac3ae5fff1a ]---
>[  170.117924] bond%d: Warning: failed to register to debugfs
>[  170.128728] ------------[ cut here ]------------
>[  170.129360] WARNING: at /home/pwp/net-next-2.6/fs/proc/generic.c:586 proc_register+0x126/0x157()
>[  170.130323] Hardware name: VirtualBox
>[  170.130797] proc_dir_entry 'bonding/bond%d' already registered
>[  170.131315] Modules linked in: bonding(+) sunrpc ipv6 uinput microcode ppdev parport_pc parport joydev e1000 pcspkr i2c_piix4 i2c_core [last unloaded: bonding]
>[  170.133731] Pid: 3457, comm: modprobe Tainted: G        W   2.6.39-rc2+ #14
>[  170.134308] Call Trace:
>[  170.134743]  [<c0438f3b>] warn_slowpath_common+0x6a/0x7f
>[  170.135305]  [<c051a42a>] ? proc_register+0x126/0x157
>[  170.135820]  [<c0438fc3>] warn_slowpath_fmt+0x2b/0x2f
>[  170.137168]  [<c051a42a>] proc_register+0x126/0x157
>[  170.137700]  [<c051a4dd>] proc_create_data+0x82/0x98
>[  170.138174]  [<f94e6af6>] bond_create_proc_entry+0x3f/0x73 [bonding]
>[  170.138745]  [<f94dd806>] bond_init+0x77/0xa5 [bonding]
>[  170.139278]  [<c0721ac6>] register_netdevice+0x8c/0x1d3
>[  170.139828]  [<f94e0e30>] bond_create+0x6c/0x90 [bonding]
>[  170.140361]  [<f94f4763>] bonding_init+0x763/0x7b1 [bonding]
>[  170.140927]  [<c0401240>] do_one_initcall+0x76/0x122
>[  170.141494]  [<f94f4000>] ? 0xf94f3fff
>[  170.141975]  [<c0463f1e>] sys_init_module+0x1286/0x140d
>[  170.142463]  [<c07c6d9f>] sysenter_do_call+0x12/0x28
>[  170.142974] ---[ end trace 64e4fac3ae5fff1b ]---
>[  170.144949] bond%d: Warning: failed to register to debugfs
>
>Signed-off-by: Weiping Pan <panweiping3@gmail.com>
>---
> net/core/dev.c |    8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/net/core/dev.c b/net/core/dev.c
>index ea23353..3ed09f8 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -5437,6 +5437,10 @@ int register_netdevice(struct net_device *dev)
> 
> 	dev->iflink = -1;
> 
>+	ret = dev_get_valid_name(dev, dev->name);
>+	if (ret < 0)
>+		goto out;
>+
> 	/* Init, if this function is available */
> 	if (dev->netdev_ops->ndo_init) {
> 		ret = dev->netdev_ops->ndo_init(dev);
>@@ -5447,10 +5451,6 @@ int register_netdevice(struct net_device *dev)
> 		}
> 	}
> 
>-	ret = dev_get_valid_name(dev, dev->name);
>-	if (ret < 0)
>-		goto err_uninit;
>-
> 	dev->ifindex = dev_new_index(net);
> 	if (dev->iflink == -1)
> 		dev->iflink = dev->ifindex;
>-- 
>1.7.4.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html


Reviewed-by: Jiri Pirko <jpirko@redhat.com>

^ permalink raw reply

* Re: [patch 1/9] [PATCH] qeth: convert to hw_features part 2
From: Michał Mirosław @ 2011-05-13  7:38 UTC (permalink / raw)
  To: frank.blaschka; +Cc: davem, netdev, linux-s390
In-Reply-To: <20110513044547.424857838@de.ibm.com>

On Fri, May 13, 2011 at 06:45:01AM +0200, frank.blaschka@de.ibm.com wrote:
> From: Frank Blaschka <frank.blaschka@de.ibm.com>
> Set rx csum default to hw checksumming again.
> Remove sysfs interface for rx csum (checksumming) and TSO (large_send).
> With the new hw_features it does not work to keep the old sysfs
> interface in parallel. Convert options.checksum_type to new hw_features.

Please see following patch (replacing qeth_l3_main.c part of your patch) for
an illustration of my earlier comments.

Best Regards,
Michał Mirosław

diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 1496661..9b8ce44 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1445,70 +1445,37 @@ static int qeth_l3_send_checksum_command(struct qeth_card *card)
 	return 0;
 }
 
-int qeth_l3_set_rx_csum(struct qeth_card *card,
-	enum qeth_checksum_types csum_type)
+static int qeth_l3_set_rx_csum(struct qeth_card *card, int on)
 {
-	int rc = 0;
+	int rc;
 
-	if (card->options.checksum_type == HW_CHECKSUMMING) {
-		if ((csum_type != HW_CHECKSUMMING) &&
-			(card->state != CARD_STATE_DOWN)) {
-			rc = qeth_l3_send_simple_setassparms(card,
-				IPA_INBOUND_CHECKSUM, IPA_CMD_ASS_STOP, 0);
-			if (rc)
-				return -EIO;
-		}
-		card->dev->features |= NETIF_F_RXCSUM;
-	} else {
-		if (csum_type == HW_CHECKSUMMING) {
-			if (card->state != CARD_STATE_DOWN) {
-				if (!qeth_is_supported(card,
-				    IPA_INBOUND_CHECKSUM))
-					return -EPERM;
-				rc = qeth_l3_send_checksum_command(card);
-				if (rc)
-					return -EIO;
-			}
-		}
-		card->dev->features &= ~NETIF_F_RXCSUM;
+	if (on) {
+		rc = qeth_l3_send_checksum_command(card);
+		if (rc)
+			return -EIO;
+	} else
+		rc = qeth_l3_send_simple_setassparms(card,
+			IPA_INBOUND_CHECKSUM, IPA_CMD_ASS_STOP, 0);
+		if (rc)
+			return -EIO;
 	}
-	card->options.checksum_type = csum_type;
-	return rc;
+
+	return 0;
 }
 
 static int qeth_l3_start_ipa_checksum(struct qeth_card *card)
 {
-	int rc = 0;
-
 	QETH_CARD_TEXT(card, 3, "strtcsum");
 
-	if (card->options.checksum_type == NO_CHECKSUMMING) {
-		dev_info(&card->gdev->dev,
-			"Using no checksumming on %s.\n",
-			QETH_CARD_IFNAME(card));
-		return 0;
-	}
-	if (card->options.checksum_type == SW_CHECKSUMMING) {
-		dev_info(&card->gdev->dev,
-			"Using SW checksumming on %s.\n",
-			QETH_CARD_IFNAME(card));
-		return 0;
-	}
-	if (!qeth_is_supported(card, IPA_INBOUND_CHECKSUM)) {
-		dev_info(&card->gdev->dev,
-			"Inbound HW Checksumming not "
-			"supported on %s,\ncontinuing "
-			"using Inbound SW Checksumming\n",
-			QETH_CARD_IFNAME(card));
-		card->options.checksum_type = SW_CHECKSUMMING;
-		return 0;
+	if (card->dev->features & NETIF_F_RXCSUM) {
+		rtnl_lock();
+		/* force set_features call */
+		card->dev->features &= ~NETIF_F_RXCSUM;
+		netdev_update_features(card->dev);
+		rtnl_unlock();
 	}
-	rc = qeth_l3_send_checksum_command(card);
-	if (!rc)
-		dev_info(&card->gdev->dev,
-			"HW Checksumming (inbound) enabled\n");
 
-	return rc;
+	return 0;
 }
 
 static int qeth_l3_start_ipa_tx_checksum(struct qeth_card *card)
@@ -2037,14 +2004,7 @@ static inline int qeth_l3_rebuild_skb(struct qeth_card *card,
 		is_vlan = 1;
 	}
 
-	switch (card->options.checksum_type) {
-	case SW_CHECKSUMMING:
-		skb->ip_summed = CHECKSUM_NONE;
-		break;
-	case NO_CHECKSUMMING:
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
-		break;
-	case HW_CHECKSUMMING:
+	if (card->dev->features & NETIF_F_RXCSUM) {
 		if ((hdr->hdr.l3.ext_flags &
 		    (QETH_HDR_EXT_CSUM_HDR_REQ |
 		     QETH_HDR_EXT_CSUM_TRANSP_REQ)) ==
@@ -2053,7 +2013,8 @@ static inline int qeth_l3_rebuild_skb(struct qeth_card *card,
 			skb->ip_summed = CHECKSUM_UNNECESSARY;
 		else
 			skb->ip_summed = CHECKSUM_NONE;
-	}
+	} else
+		skb->ip_summed = CHECKSUM_NONE;
 
 	return is_vlan;
 }
@@ -3235,20 +3196,20 @@ static u32 qeth_l3_fix_features(struct net_device *dev, u32 features)
 
 static int qeth_l3_set_features(struct net_device *dev, u32 features)
 {
-	enum qeth_checksum_types csum_type;
 	struct qeth_card *card = dev->ml_priv;
 	u32 changed = dev->features ^ features;
+	int on;
 
 	if (!(changed & NETIF_F_RXCSUM))
 		return 0;
 
-	if (features & NETIF_F_RXCSUM)
-		csum_type = HW_CHECKSUMMING;
-	else
-		csum_type = SW_CHECKSUMMING;
+	if (card->state == CARD_STATE_DOWN)
+		return 0;
 
+	/* commit other changes in case of error */
 	dev->features = features ^ NETIF_F_RXCSUM;
-	return qeth_l3_set_rx_csum(card, csum_type);
+
+	return qeth_l3_set_rx_csum(card, features & NETIF_F_RXCSUM);
 }
 
 static const struct ethtool_ops qeth_l3_ethtool_ops = {
@@ -3342,6 +3303,12 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
 			if (!(card->info.unique_id & UNIQUE_ID_NOT_BY_CARD))
 				card->dev->dev_id = card->info.unique_id &
 							 0xffff;
+			if (!card->info.guestlan) {
+				card->dev->hw_features = NETIF_F_SG |
+					NETIF_F_RXCSUM | NETIF_F_IP_CSUM |
+					NETIF_F_TSO;
+				card->dev->features = NETIF_F_RXCSUM;
+			}
 		}
 	} else if (card->info.type == QETH_CARD_TYPE_IQD) {
 		card->dev = alloc_netdev(0, "hsi%d", ether_setup);
@@ -3357,8 +3324,6 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
 	card->dev->watchdog_timeo = QETH_TX_TIMEOUT;
 	card->dev->mtu = card->info.initial_mtu;
 	SET_ETHTOOL_OPS(card->dev, &qeth_l3_ethtool_ops);
-	card->dev->hw_features = NETIF_F_SG | NETIF_F_RXCSUM |
-		NETIF_F_IP_CSUM | NETIF_F_TSO;
 	card->dev->features |=	NETIF_F_HW_VLAN_TX |
 				NETIF_F_HW_VLAN_RX |
 				NETIF_F_HW_VLAN_FILTER;
@@ -3382,9 +3347,6 @@ static int qeth_l3_probe_device(struct ccwgroup_device *gdev)
 	card->discipline.output_handler = (qdio_handler_t *)
 		qeth_qdio_output_handler;
 	card->discipline.recover = qeth_l3_recover;
-	if ((card->info.type == QETH_CARD_TYPE_OSD) ||
-	    (card->info.type == QETH_CARD_TYPE_OSX))
-		card->options.checksum_type = HW_CHECKSUMMING;
 	return 0;
 }
 

^ permalink raw reply related

* Re: [PATCHv1] e1000e: Allow ethtool to enable/disable loopback.
From: Michał Mirosław @ 2011-05-13  7:49 UTC (permalink / raw)
  To: Mahesh Bandewar
  Cc: Jeff Kirsher, e1000-devel, David Miller, netdev, Tom Herbert
In-Reply-To: <BANLkTimsUga1LEF1y61+2To2uQQGpxcPBg@mail.gmail.com>

W dniu 12 maja 2011 23:41 użytkownik Mahesh Bandewar
<maheshb@google.com> napisał:
> On Wed, May 11, 2011 at 10:50 PM, Michał Mirosław <mirqus@gmail.com> wrote:
>> W dniu 12 maja 2011 01:11 użytkownik Mahesh Bandewar
>> <maheshb@google.com> napisał:
>>> On Wed, May 11, 2011 at 12:15 PM, Michał Mirosław <mirqus@gmail.com> wrote:
>>>> If e1000_set_loopback() fails, this should set dev->features to passed
>>>> features (but keeping NETIF_F_LOOPBACK unchanged in dev->features) to
>>>> keep the state consistent.
>>> set_features() can return the return code of set_loopback() instead of
>>> 0; this way the consistency will be maintained.
>> Only as long as NETIF_F_LOOPBACK is the only bit set in hw_features.
>> netdev_update_features() can't really know which features were changed
>> and which failed when ndo_set_features callback returns non-zero.
> This is more of an API shortcoming. Callback will have to revert
> changes made (rollback) before returning non-zero value to keep it
> consistent.

It might just update dev->features to match instead of rollback. It
could also start some recovery process that eventually calls
netdev_update_features() again to try the change again.

IOW, the information what changes failed are returned implicitly in
modified dev->features. When callback returns 0,
netdev_update_features() assumes that all were set correctly and
updates dev->features itself.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [patch 1/9] [PATCH] qeth: convert to hw_features part 2
From: Frank Blaschka @ 2011-05-13  7:57 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: davem, netdev, linux-s390
In-Reply-To: <20110513073807.GA18312@rere.qmqm.pl>

On Fri, May 13, 2011 at 09:38:07AM +0200, Michał Mirosław wrote:
> On Fri, May 13, 2011 at 06:45:01AM +0200, frank.blaschka@de.ibm.com wrote:
> > From: Frank Blaschka <frank.blaschka@de.ibm.com>
> > Set rx csum default to hw checksumming again.
> > Remove sysfs interface for rx csum (checksumming) and TSO (large_send).
> > With the new hw_features it does not work to keep the old sysfs
> > interface in parallel. Convert options.checksum_type to new hw_features.
> 
> Please see following patch (replacing qeth_l3_main.c part of your patch) for
> an illustration of my earlier comments.
>

Thx for the illustration. I will review and do some more testing. For now
Dave should apply my latest patch set so we get this stuff into next merge
window. 

^ permalink raw reply

* Re: [patch 1/9] [PATCH] qeth: convert to hw_features part 2
From: Michał Mirosław @ 2011-05-13  8:07 UTC (permalink / raw)
  To: frank.blaschka; +Cc: davem, netdev, linux-s390
In-Reply-To: <20110513044547.424857838@de.ibm.com>

2011/5/13  <frank.blaschka@de.ibm.com>:
> From: Frank Blaschka <frank.blaschka@de.ibm.com>
>
> Set rx csum default to hw checksumming again.
> Remove sysfs interface for rx csum (checksumming) and TSO (large_send).
> With the new hw_features it does not work to keep the old sysfs
> interface in parallel. Convert options.checksum_type to new hw_features.
[...]
> @@ -1482,32 +1478,34 @@ static int qeth_l3_start_ipa_checksum(st
>
>        QETH_CARD_TEXT(card, 3, "strtcsum");
>
> -       if (card->options.checksum_type == NO_CHECKSUMMING) {
> -               dev_info(&card->gdev->dev,
> -                       "Using no checksumming on %s.\n",
> -                       QETH_CARD_IFNAME(card));
> -               return 0;
> -       }
> -       if (card->options.checksum_type == SW_CHECKSUMMING) {
> -               dev_info(&card->gdev->dev,
> -                       "Using SW checksumming on %s.\n",
> -                       QETH_CARD_IFNAME(card));
> -               return 0;
> -       }
> -       if (!qeth_is_supported(card, IPA_INBOUND_CHECKSUM)) {
> -               dev_info(&card->gdev->dev,
> +       if (card->dev->features & NETIF_F_RXCSUM) {
> +               /* hw may have changed during offline or recovery */
> +               if (!qeth_is_supported(card, IPA_INBOUND_CHECKSUM)) {
> +                       dev_info(&card->gdev->dev,
>                        "Inbound HW Checksumming not "
>                        "supported on %s,\ncontinuing "
>                        "using Inbound SW Checksumming\n",
>                        QETH_CARD_IFNAME(card));
> -               card->options.checksum_type = SW_CHECKSUMMING;
> -               return 0;
> -       }
> -       rc = qeth_l3_send_checksum_command(card);
> -       if (!rc)
> -               dev_info(&card->gdev->dev,
> +                       goto update_feature;
> +               }
> +
> +               rc = qeth_l3_send_checksum_command(card);
> +               if (!rc)
> +                       dev_info(&card->gdev->dev,
>                        "HW Checksumming (inbound) enabled\n");
> +               else
> +                       goto update_feature;
> +       } else
> +               dev_info(&card->gdev->dev,
> +                       "Using SW checksumming on %s.\n",
> +                       QETH_CARD_IFNAME(card));
> +       return 0;
>
> +update_feature:
> +       rtnl_lock();
> +       card->dev->features &= ~NETIF_F_RXCSUM;
> +       netdev_update_features(card->dev);
> +       rtnl_unlock();
>        return rc;
>  }

This will retry starting the RX checksumming via set_features(). Is
that the intention? If not, then just use something like my example in
qeth_l3_start_ipa_checksum().

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re.funds
From: rhumana @ 2011-05-13  7:36 UTC (permalink / raw)


My associate has helped me to send your first payment
of $7,500 USD to you as instructed by the Malaysian
Government and Mr. David Cameron the United Kingdom
prime minister after the last G20 meeting that was
held in Malaysia, making you one of the beneficiaries through
an email natural simple ballot selection.

He told Allan Davis to keep sending you $5,000 USD via Money Gram
twice a week until the FULL payment of ($820,000.00 )
is completed.

MONEY TRANSFER REFERENCE:2116-3297

SENDER'S NAME:Patrick Lee Chun
AMOUNT: US$5000

To track your funds forward money gram
Transfer agent Mr Allan Davis

Your Name.____________
Phone .______________

Contact Allan Davis for the funds clearance
certificate (FCC) neccessary for the realise of your funds

E-mail:money_gramlimited@mspil.edu.cn
D/L: Tel:+601-635-44376

You cannot pickup the money until the certificate is
obtained by you.

Regards
Mr. Allan Davis.
Tel: +601-635-44376

Best Regards,
Mr Allan Davis





----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.


^ permalink raw reply

* Re: AAARGH bisection is hard (Re: [2.6.39 regression] X locks up hard right after logging in)
From: Christian Couder @ 2011-05-13  8:20 UTC (permalink / raw)
  To: Andrew Lutomirski; +Cc: linux-kernel, netdev, git, Linus Torvalds, Shuang He
In-Reply-To: <BANLkTi=kb_m-CfrpnD8qQTVYLGaDdgy_tg@mail.gmail.com>

On Thu, May 12, 2011 at 7:15 PM, Andrew Lutomirski <luto@mit.edu> wrote:
>
> OK, this sucks.  In the course of bisecting this, I've hit two other
> apparently unrelated bugs that prevent my from testing large numbers
> of kernels.  Do I have two questions:
>
> 1. Anyone have any ideas from looking at the log?
>
> It looks like most of what's left is network code, so cc netdev.
>
> 2.  The !&$#@ bisection is skipping all over the place.  I've seen
> 2.6.37 versions and all manner of -rc's out of order.  Linus, and
> other people who like pontificating about git bisection: is there any
> way to get the bisection to follow Linus' tree?  I think that if
> bisect could be persuaded to consider only changes that are reached by
> following only the *first* merge parent all the way from the bad
> revision to the good revision, then the bisection would build versions
> that were at least good enough for Linus to pull and might have fewer
> bisection-killing bugs.
>
> (This isn't a new idea [1], and git rev-list --bisect --first-parent
> isn't so bad except that it doesn't bisect.)

Did you forget to put the reference [1] in your email? Was it this one
you were thinking about:

http://thread.gmane.org/gmane.comp.version-control.git/165433/

?

Thanks,
Christian.

^ permalink raw reply

* [PATCH v1] bonding,llc: Fix structure sizeof incompatibility for some PDUs
From: Vitalii Demianets @ 2011-05-13  9:04 UTC (permalink / raw)
  To: netdev
  Cc: bonding-devel, David Miller, Eric Dumazet, Ben Hutchings,
	Joe Perches, Jay Vosburgh, Andy Gospodarek,
	Arnaldo Carvalho de Melo

With some combinations of arch/compiler (e.g. arm-linux-gcc) the sizeof 
operator on structure returns value greater than expected. In cases when the 
structure is used for mapping PDU fields it may lead to unexpected results 
(such as holes and alignment problems in skb data). __packed prevents this 
undesired behavior.

Signed-off-by: Vitalii Demianets <vitas@nppfactor.kiev.ua>
---
v1: use __packed macro instead of __attribute__((packed))

---
 drivers/net/bonding/bond_3ad.h |   10 +++++-----
 include/net/llc_pdu.h          |    8 ++++----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index 291dbd4..0ee3f16 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -39,7 +39,7 @@
 
 typedef struct mac_addr {
 	u8 mac_addr_value[ETH_ALEN];
-} mac_addr_t;
+} __packed mac_addr_t;
 
 enum {
 	BOND_AD_STABLE = 0,
@@ -134,12 +134,12 @@ typedef struct lacpdu {
 	u8 tlv_type_terminator;	     // = terminator
 	u8 terminator_length;	     // = 0
 	u8 reserved_50[50];	     // = 0
-} lacpdu_t;
+} __packed lacpdu_t;
 
 typedef struct lacpdu_header {
 	struct ethhdr hdr;
 	struct lacpdu lacpdu;
-} lacpdu_header_t;
+} __packed lacpdu_header_t;
 
 // Marker Protocol Data Unit(PDU) structure(43.5.3.2 in the 802.3ad standard)
 typedef struct bond_marker {
@@ -155,12 +155,12 @@ typedef struct bond_marker {
 	u8 tlv_type_terminator;	     //  = 0x00
 	u8 terminator_length;	     //  = 0x00
 	u8 reserved_90[90];	     //  = 0
-} bond_marker_t;
+} __packed bond_marker_t;
 
 typedef struct bond_marker_header {
 	struct ethhdr hdr;
 	struct bond_marker marker;
-} bond_marker_header_t;
+} __packed bond_marker_header_t;
 
 #pragma pack()
 
diff --git a/include/net/llc_pdu.h b/include/net/llc_pdu.h
index 75b8e29..f57e7d4 100644
--- a/include/net/llc_pdu.h
+++ b/include/net/llc_pdu.h
@@ -199,7 +199,7 @@ struct llc_pdu_sn {
 	u8 ssap;
 	u8 ctrl_1;
 	u8 ctrl_2;
-};
+} __packed;
 
 static inline struct llc_pdu_sn *llc_pdu_sn_hdr(struct sk_buff *skb)
 {
@@ -211,7 +211,7 @@ struct llc_pdu_un {
 	u8 dsap;
 	u8 ssap;
 	u8 ctrl_1;
-};
+} __packed;
 
 static inline struct llc_pdu_un *llc_pdu_un_hdr(struct sk_buff *skb)
 {
@@ -359,7 +359,7 @@ struct llc_xid_info {
 	u8 fmt_id;	/* always 0x81 for LLC */
 	u8 type;	/* different if NULL/non-NULL LSAP */
 	u8 rw;		/* sender receive window */
-};
+} __packed;
 
 /**
  *	llc_pdu_init_as_xid_cmd - sets bytes 3, 4 & 5 of LLC header as XID
@@ -415,7 +415,7 @@ struct llc_frmr_info {
 	u8  curr_ssv;		/* current send state variable val */
 	u8  curr_rsv;		/* current receive state variable */
 	u8  ind_bits;		/* indicator bits set with macro */
-};
+} __packed;
 
 extern void llc_pdu_set_cmd_rsp(struct sk_buff *skb, u8 type);
 extern void llc_pdu_set_pf_bit(struct sk_buff *skb, u8 bit_value);
-- 
1.7.3.4


^ permalink raw reply related

* Re: [PATCH 1/9] iwlegacy: Silence DEBUG_STRICT_USER_COPY_CHECKS=y warning
From: Stanislaw Gruszka @ 2011-05-13  9:08 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: John W. Linville, linux-kernel, linux-wireless, netdev,
	Johannes Berg
In-Reply-To: <1305244212-19183-2-git-send-email-sboyd@codeaurora.org>

On Thu, May 12, 2011 at 04:50:04PM -0700, Stephen Boyd wrote:
> Enabling DEBUG_STRICT_USER_COPY_CHECKS causes the following
> warning:
> 
> In file included from arch/x86/include/asm/uaccess.h:573,
>                  from include/net/checksum.h:25,
>                  from include/linux/skbuff.h:28,
>                  from drivers/net/wireless/iwlegacy/iwl-4965-rs.c:28:
> In function 'copy_from_user',
>     inlined from 'iwl4965_rs_sta_dbgfs_scale_table_write' at
>     drivers/net/wireless/iwlegacy/iwl-4965-rs.c:2616:
> arch/x86/include/asm/uaccess_64.h:65:
> warning: call to 'copy_from_user_overflow' declared with
> attribute warning: copy_from_user() buffer size is not provably
> correct
> 
> presumably due to buf_size being signed causing GCC to fail to
> see that buf_size can't become negative.
> 
> Cc: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/net/wireless/iwlegacy/iwl-4965-rs.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> index 31ac672..cc4751c 100644
> --- a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> +++ b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> @@ -2604,7 +2604,7 @@ static ssize_t iwl4965_rs_sta_dbgfs_scale_table_write(struct file *file,
>  	struct iwl_lq_sta *lq_sta = file->private_data;
>  	struct iwl_priv *priv;
>  	char buf[64];
> -	int buf_size;
> +	size_t buf_size;

ACK, but we have more of that, perhaps you would like to fix them all.
If you don't want to, I'll do it. Note this can be easily done by:

sed -i 's/int buf_size;/size_t buf_size;/g' drivers/net/wireless/iwlegacy/*.[ch]

Stanislaw

^ permalink raw reply

* question about UFO behavior for bridge device
From: Shan Wei @ 2011-05-13  9:21 UTC (permalink / raw)
  To: 单卫, Herbert Xu, Ben Hutchings, netdev

UDP protocol creates a big packet(skb) in which the data length is greater than MTU.

If device(eth0, lo) does not supports UFO, This big skb will be fragmented to
many fragments in IP protocol, and fragments are sent one by one by device.

But, if device(eth0, lo) supports UFO, IP protocol don't fragment it, and
device directly sends it out.

For bridge device which enable UFO, and Ethernet device(eth0) which not 
support UFO, IP protocol also doesn't fragment it, and bridge forwards original
skb to eth0. For this UFO disabled eth0, kernel needs to perform segmentation
on so big skb in dev_gso_segment(), and link segmented skbs to next field
of original skb. Then device sends segmented skbs out, but not original skb.

But, actually, i saw original big skb in eth0's tcpdump file, but not segmented skbs.
The behavior is right or what we want?
Is there anything missed about my analysis?

-- 
Best Regards
-----
Shan Wei

^ permalink raw reply

* [net-next-2.6 09/10] caif: Bugfix debugfs directory name must be unique.
From: Sjur Brændeland @ 2011-05-13 12:44 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland
In-Reply-To: <1305290648-9613-1-git-send-email-sjur.brandeland@stericsson.com>

Race condition caused debugfs_create_dir() to fail due to duplicate
name. Use atomic counter to create unique directory name.

net_ratelimit() is introduced to limit debug printouts.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/caif_socket.c |   34 ++++++++++++++++++++++------------
 1 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 7baae11..5cf9c73 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -48,6 +48,7 @@ static struct dentry *debugfsdir;
 #ifdef CONFIG_DEBUG_FS
 struct debug_fs_counter {
 	atomic_t caif_nr_socks;
+	atomic_t caif_sock_create;
 	atomic_t num_connect_req;
 	atomic_t num_connect_resp;
 	atomic_t num_connect_fail_resp;
@@ -59,11 +60,11 @@ struct debug_fs_counter {
 	atomic_t num_rx_flow_on;
 };
 static struct debug_fs_counter cnt;
-#define	dbfs_atomic_inc(v) atomic_inc(v)
-#define	dbfs_atomic_dec(v) atomic_dec(v)
+#define	dbfs_atomic_inc(v) atomic_inc_return(v)
+#define	dbfs_atomic_dec(v) atomic_dec_return(v)
 #else
-#define	dbfs_atomic_inc(v)
-#define	dbfs_atomic_dec(v)
+#define	dbfs_atomic_inc(v) 0
+#define	dbfs_atomic_dec(v) 0
 #endif
 
 struct caifsock {
@@ -155,9 +156,10 @@ static int caif_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 
 	if (atomic_read(&sk->sk_rmem_alloc) + skb->truesize >=
 		(unsigned)sk->sk_rcvbuf && rx_flow_is_on(cf_sk)) {
-		pr_debug("sending flow OFF (queue len = %d %d)\n",
-			atomic_read(&cf_sk->sk.sk_rmem_alloc),
-			sk_rcvbuf_lowwater(cf_sk));
+		if (net_ratelimit())
+			pr_debug("sending flow OFF (queue len = %d %d)\n",
+					atomic_read(&cf_sk->sk.sk_rmem_alloc),
+					sk_rcvbuf_lowwater(cf_sk));
 		set_rx_flow_off(cf_sk);
 		dbfs_atomic_inc(&cnt.num_rx_flow_off);
 		caif_flow_ctrl(sk, CAIF_MODEMCMD_FLOW_OFF_REQ);
@@ -168,7 +170,8 @@ static int caif_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 		return err;
 	if (!sk_rmem_schedule(sk, skb->truesize) && rx_flow_is_on(cf_sk)) {
 		set_rx_flow_off(cf_sk);
-		pr_debug("sending flow OFF due to rmem_schedule\n");
+		if (net_ratelimit())
+			pr_debug("sending flow OFF due to rmem_schedule\n");
 		dbfs_atomic_inc(&cnt.num_rx_flow_off);
 		caif_flow_ctrl(sk, CAIF_MODEMCMD_FLOW_OFF_REQ);
 	}
@@ -838,7 +841,7 @@ static int caif_connect(struct socket *sock, struct sockaddr *uaddr,
 	sock->state = SS_CONNECTING;
 	sk->sk_state = CAIF_CONNECTING;
 
-	/* Check priority value coming from socket */
+	/* Check priority value comming from socket */
 	/* if priority value is out of range it will be ajusted */
 	if (cf_sk->sk.sk_priority > CAIF_PRIO_MAX)
 		cf_sk->conn_req.priority = CAIF_PRIO_MAX;
@@ -942,6 +945,7 @@ static int caif_release(struct socket *sock)
 
 	dbfs_atomic_inc(&cnt.num_disconnect);
 
+	WARN_ON(IS_ERR(cf_sk->debugfs_socket_dir));
 	if (cf_sk->debugfs_socket_dir != NULL)
 		debugfs_remove_recursive(cf_sk->debugfs_socket_dir);
 
@@ -1047,7 +1051,7 @@ static void caif_sock_destructor(struct sock *sk)
 	caif_assert(sk_unhashed(sk));
 	caif_assert(!sk->sk_socket);
 	if (!sock_flag(sk, SOCK_DEAD)) {
-		pr_info("Attempt to release alive CAIF socket: %p\n", sk);
+		pr_debug("Attempt to release alive CAIF socket: %p\n", sk);
 		return;
 	}
 	sk_stream_kill_queues(&cf_sk->sk);
@@ -1058,6 +1062,7 @@ static void caif_sock_destructor(struct sock *sk)
 static int caif_create(struct net *net, struct socket *sock, int protocol,
 			int kern)
 {
+	int num;
 	struct sock *sk = NULL;
 	struct caifsock *cf_sk = NULL;
 	static struct proto prot = {.name = "PF_CAIF",
@@ -1120,14 +1125,16 @@ static int caif_create(struct net *net, struct socket *sock, int protocol,
 	cf_sk->conn_req.protocol = protocol;
 	/* Increase the number of sockets created. */
 	dbfs_atomic_inc(&cnt.caif_nr_socks);
+	num = dbfs_atomic_inc(&cnt.caif_sock_create);
 #ifdef CONFIG_DEBUG_FS
 	if (!IS_ERR(debugfsdir)) {
+
 		/* Fill in some information concerning the misc socket. */
-		snprintf(cf_sk->name, sizeof(cf_sk->name), "cfsk%d",
-				atomic_read(&cnt.caif_nr_socks));
+		snprintf(cf_sk->name, sizeof(cf_sk->name), "cfsk%d", num);
 
 		cf_sk->debugfs_socket_dir =
 			debugfs_create_dir(cf_sk->name, debugfsdir);
+
 		debugfs_create_u32("sk_state", S_IRUSR | S_IWUSR,
 				cf_sk->debugfs_socket_dir,
 				(u32 *) &cf_sk->sk.sk_state);
@@ -1171,6 +1178,9 @@ static int __init caif_sktinit_module(void)
 		debugfs_create_u32("num_sockets", S_IRUSR | S_IWUSR,
 				debugfsdir,
 				(u32 *) &cnt.caif_nr_socks);
+		debugfs_create_u32("num_create", S_IRUSR | S_IWUSR,
+				debugfsdir,
+				(u32 *) &cnt.caif_sock_create);
 		debugfs_create_u32("num_connect_req", S_IRUSR | S_IWUSR,
 				debugfsdir,
 				(u32 *) &cnt.num_connect_req);
-- 
1.7.1


^ permalink raw reply related

* [net-next-2.6 01/10] caif: Use rcu_read_lock in CAIF mux layer.
From: Sjur Brændeland @ 2011-05-13 12:43 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland
In-Reply-To: <1305290648-9613-1-git-send-email-sjur.brandeland@stericsson.com>

Replace spin_lock with rcu_read_lock when accessing lists to layers
and cache. While packets are in flight rcu_read_lock should not be held,
instead ref-counters are used in combination with RCU.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 include/net/caif/cffrml.h |    2 +
 net/caif/cffrml.c         |    8 +++
 net/caif/cfmuxl.c         |  119 ++++++++++++++++++++++++++++-----------------
 3 files changed, 85 insertions(+), 44 deletions(-)

diff --git a/include/net/caif/cffrml.h b/include/net/caif/cffrml.h
index 3f14d2e..4f126d7 100644
--- a/include/net/caif/cffrml.h
+++ b/include/net/caif/cffrml.h
@@ -12,5 +12,7 @@ struct cffrml;
 struct cflayer *cffrml_create(u16 phyid, bool DoFCS);
 void cffrml_set_uplayer(struct cflayer *this, struct cflayer *up);
 void cffrml_set_dnlayer(struct cflayer *this, struct cflayer *dn);
+void cffrml_put(struct cflayer *layr);
+void cffrml_hold(struct cflayer *layr);
 
 #endif /* CFFRML_H_ */
diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
index 2423fed..f4b8892 100644
--- a/net/caif/cffrml.c
+++ b/net/caif/cffrml.c
@@ -145,3 +145,11 @@ static void cffrml_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 	if (layr->up->ctrlcmd)
 		layr->up->ctrlcmd(layr->up, ctrl, layr->id);
 }
+
+void cffrml_put(struct cflayer *layr)
+{
+}
+
+void cffrml_hold(struct cflayer *layr)
+{
+}
diff --git a/net/caif/cfmuxl.c b/net/caif/cfmuxl.c
index fc24974..2a56df7 100644
--- a/net/caif/cfmuxl.c
+++ b/net/caif/cfmuxl.c
@@ -9,6 +9,7 @@
 #include <linux/stddef.h>
 #include <linux/spinlock.h>
 #include <linux/slab.h>
+#include <linux/rculist.h>
 #include <net/caif/cfpkt.h>
 #include <net/caif/cfmuxl.h>
 #include <net/caif/cfsrvl.h>
@@ -64,31 +65,31 @@ struct cflayer *cfmuxl_create(void)
 int cfmuxl_set_uplayer(struct cflayer *layr, struct cflayer *up, u8 linkid)
 {
 	struct cfmuxl *muxl = container_obj(layr);
-	spin_lock(&muxl->receive_lock);
-	cfsrvl_get(up);
-	list_add(&up->node, &muxl->srvl_list);
-	spin_unlock(&muxl->receive_lock);
+
+	spin_lock_bh(&muxl->receive_lock);
+	list_add_rcu(&up->node, &muxl->srvl_list);
+	spin_unlock_bh(&muxl->receive_lock);
 	return 0;
 }
 
 int cfmuxl_set_dnlayer(struct cflayer *layr, struct cflayer *dn, u8 phyid)
 {
 	struct cfmuxl *muxl = (struct cfmuxl *) layr;
-	spin_lock(&muxl->transmit_lock);
-	list_add(&dn->node, &muxl->frml_list);
-	spin_unlock(&muxl->transmit_lock);
+
+	spin_lock_bh(&muxl->transmit_lock);
+	list_add_rcu(&dn->node, &muxl->frml_list);
+	spin_unlock_bh(&muxl->transmit_lock);
 	return 0;
 }
 
 static struct cflayer *get_from_id(struct list_head *list, u16 id)
 {
-	struct list_head *node;
-	struct cflayer *layer;
-	list_for_each(node, list) {
-		layer = list_entry(node, struct cflayer, node);
-		if (layer->id == id)
-			return layer;
+	struct cflayer *lyr;
+	list_for_each_entry_rcu(lyr, list, node) {
+		if (lyr->id == id)
+			return lyr;
 	}
+
 	return NULL;
 }
 
@@ -96,41 +97,45 @@ struct cflayer *cfmuxl_remove_dnlayer(struct cflayer *layr, u8 phyid)
 {
 	struct cfmuxl *muxl = container_obj(layr);
 	struct cflayer *dn;
-	spin_lock(&muxl->transmit_lock);
-	memset(muxl->dn_cache, 0, sizeof(muxl->dn_cache));
+	int idx = phyid % DN_CACHE_SIZE;
+
+	spin_lock_bh(&muxl->transmit_lock);
+	rcu_assign_pointer(muxl->dn_cache[idx], NULL);
 	dn = get_from_id(&muxl->frml_list, phyid);
-	if (dn == NULL) {
-		spin_unlock(&muxl->transmit_lock);
-		return NULL;
-	}
-	list_del(&dn->node);
+	if (dn == NULL)
+		goto out;
+
+	list_del_rcu(&dn->node);
 	caif_assert(dn != NULL);
-	spin_unlock(&muxl->transmit_lock);
+out:
+	spin_unlock_bh(&muxl->transmit_lock);
 	return dn;
 }
 
-/* Invariant: lock is taken */
 static struct cflayer *get_up(struct cfmuxl *muxl, u16 id)
 {
 	struct cflayer *up;
 	int idx = id % UP_CACHE_SIZE;
-	up = muxl->up_cache[idx];
+	up = rcu_dereference(muxl->up_cache[idx]);
 	if (up == NULL || up->id != id) {
+		spin_lock_bh(&muxl->receive_lock);
 		up = get_from_id(&muxl->srvl_list, id);
-		muxl->up_cache[idx] = up;
+		rcu_assign_pointer(muxl->up_cache[idx], up);
+		spin_unlock_bh(&muxl->receive_lock);
 	}
 	return up;
 }
 
-/* Invariant: lock is taken */
 static struct cflayer *get_dn(struct cfmuxl *muxl, struct dev_info *dev_info)
 {
 	struct cflayer *dn;
 	int idx = dev_info->id % DN_CACHE_SIZE;
-	dn = muxl->dn_cache[idx];
+	dn = rcu_dereference(muxl->dn_cache[idx]);
 	if (dn == NULL || dn->id != dev_info->id) {
+		spin_lock_bh(&muxl->transmit_lock);
 		dn = get_from_id(&muxl->frml_list, dev_info->id);
-		muxl->dn_cache[idx] = dn;
+		rcu_assign_pointer(muxl->dn_cache[idx], dn);
+		spin_unlock_bh(&muxl->transmit_lock);
 	}
 	return dn;
 }
@@ -139,15 +144,17 @@ struct cflayer *cfmuxl_remove_uplayer(struct cflayer *layr, u8 id)
 {
 	struct cflayer *up;
 	struct cfmuxl *muxl = container_obj(layr);
-	spin_lock(&muxl->receive_lock);
-	up = get_up(muxl, id);
+	int idx = id % UP_CACHE_SIZE;
+
+	spin_lock_bh(&muxl->receive_lock);
+	up = get_from_id(&muxl->srvl_list, id);
 	if (up == NULL)
 		goto out;
-	memset(muxl->up_cache, 0, sizeof(muxl->up_cache));
-	list_del(&up->node);
-	cfsrvl_put(up);
+
+	rcu_assign_pointer(muxl->up_cache[idx], NULL);
+	list_del_rcu(&up->node);
 out:
-	spin_unlock(&muxl->receive_lock);
+	spin_unlock_bh(&muxl->receive_lock);
 	return up;
 }
 
@@ -162,22 +169,28 @@ static int cfmuxl_receive(struct cflayer *layr, struct cfpkt *pkt)
 		cfpkt_destroy(pkt);
 		return -EPROTO;
 	}
-
-	spin_lock(&muxl->receive_lock);
+	rcu_read_lock();
 	up = get_up(muxl, id);
-	spin_unlock(&muxl->receive_lock);
+
 	if (up == NULL) {
-		pr_info("Received data on unknown link ID = %d (0x%x) up == NULL",
-			id, id);
+		pr_debug("Received data on unknown link ID = %d (0x%x)"
+			" up == NULL", id, id);
 		cfpkt_destroy(pkt);
 		/*
 		 * Don't return ERROR, since modem misbehaves and sends out
 		 * flow on before linksetup response.
 		 */
+
+		rcu_read_unlock();
 		return /* CFGLU_EPROT; */ 0;
 	}
+
+	/* We can't hold rcu_lock during receive, so take a ref count instead */
 	cfsrvl_get(up);
+	rcu_read_unlock();
+
 	ret = up->receive(up, pkt);
+
 	cfsrvl_put(up);
 	return ret;
 }
@@ -185,31 +198,49 @@ static int cfmuxl_receive(struct cflayer *layr, struct cfpkt *pkt)
 static int cfmuxl_transmit(struct cflayer *layr, struct cfpkt *pkt)
 {
 	struct cfmuxl *muxl = container_obj(layr);
+	int err;
 	u8 linkid;
 	struct cflayer *dn;
 	struct caif_payload_info *info = cfpkt_info(pkt);
 	BUG_ON(!info);
+
+	rcu_read_lock();
+
 	dn = get_dn(muxl, info->dev_info);
 	if (dn == NULL) {
-		pr_warn("Send data on unknown phy ID = %d (0x%x)\n",
+		pr_debug("Send data on unknown phy ID = %d (0x%x)\n",
 			info->dev_info->id, info->dev_info->id);
+		rcu_read_unlock();
+		cfpkt_destroy(pkt);
 		return -ENOTCONN;
 	}
+
 	info->hdr_len += 1;
 	linkid = info->channel_id;
 	cfpkt_add_head(pkt, &linkid, 1);
-	return dn->transmit(dn, pkt);
+
+	/* We can't hold rcu_lock during receive, so take a ref count instead */
+	cffrml_hold(dn);
+
+	rcu_read_unlock();
+
+	err = dn->transmit(dn, pkt);
+
+	cffrml_put(dn);
+	return err;
 }
 
 static void cfmuxl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 				int phyid)
 {
 	struct cfmuxl *muxl = container_obj(layr);
-	struct list_head *node, *next;
 	struct cflayer *layer;
-	list_for_each_safe(node, next, &muxl->srvl_list) {
-		layer = list_entry(node, struct cflayer, node);
-		if (cfsrvl_phyid_match(layer, phyid))
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(layer, &muxl->srvl_list, node) {
+		if (cfsrvl_phyid_match(layer, phyid) && layer->ctrlcmd)
+			/* NOTE: ctrlcmd is not allowed to block */
 			layer->ctrlcmd(layer, ctrl, phyid);
 	}
+	rcu_read_unlock();
 }
-- 
1.7.1


^ permalink raw reply related

* [net-next-2.6 02/10] caif: Use RCU instead of spin-lock in caif_dev.c
From: Sjur Brændeland @ 2011-05-13 12:44 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland
In-Reply-To: <1305290648-9613-1-git-send-email-sjur.brandeland@stericsson.com>

RCU read_lock and refcount is used to protect in-flight packets.

Use RCU and counters to manage freeing lower part of the CAIF stack if
CAIF-link layer is removed. Old solution based on delaying removal of
device is removed.

When CAIF link layer goes down the use of CAIF link layer is disabled
(by calling caif_set_phy_state()), but removal and freeing of the
lower part of the CAIF stack is done when Link layer is unregistered.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 include/net/caif/cfcnfg.h |   10 ++
 net/caif/caif_dev.c       |  277 ++++++++++++++++++++++++++-------------------
 2 files changed, 169 insertions(+), 118 deletions(-)

diff --git a/include/net/caif/cfcnfg.h b/include/net/caif/cfcnfg.h
index f33d363..e0a1eb5 100644
--- a/include/net/caif/cfcnfg.h
+++ b/include/net/caif/cfcnfg.h
@@ -145,4 +145,14 @@ struct dev_info *cfcnfg_get_phyid(struct cfcnfg *cnfg,
  * @ifi:	ifindex obtained from socket.c bindtodevice.
  */
 int cfcnfg_get_id_from_ifi(struct cfcnfg *cnfg, int ifi);
+
+/**
+ * cfcnfg_set_phy_state() - Set the state of the physical interface device.
+ * @cnfg:	Configuration object
+ * @phy_layer:	Physical Layer representation
+ * @up:	State of device
+ */
+int cfcnfg_set_phy_state(struct cfcnfg *cnfg, struct cflayer *phy_layer,
+				bool up);
+
 #endif				/* CFCNFG_H_ */
diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index 75e00d5..6d1d86b 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -12,14 +12,11 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ":%s(): " fmt, __func__
 
 #include <linux/version.h>
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/if_arp.h>
 #include <linux/net.h>
 #include <linux/netdevice.h>
-#include <linux/skbuff.h>
-#include <linux/sched.h>
-#include <linux/wait.h>
+#include <linux/mutex.h>
 #include <net/netns/generic.h>
 #include <net/net_namespace.h>
 #include <net/pkt_sched.h>
@@ -30,23 +27,19 @@
 #include <net/caif/cfcnfg.h>
 
 MODULE_LICENSE("GPL");
-#define TIMEOUT (HZ*5)
 
 /* Used for local tracking of the CAIF net devices */
 struct caif_device_entry {
 	struct cflayer layer;
 	struct list_head list;
-	atomic_t in_use;
-	atomic_t state;
-	u16 phyid;
 	struct net_device *netdev;
-	wait_queue_head_t event;
+	int __percpu *pcpu_refcnt;
 };
 
 struct caif_device_entry_list {
 	struct list_head list;
 	/* Protects simulanous deletes in list */
-	spinlock_t lock;
+	struct mutex lock;
 };
 
 struct caif_net {
@@ -65,19 +58,39 @@ static struct caif_device_entry_list *caif_device_list(struct net *net)
 	return &caifn->caifdevs;
 }
 
+static void caifd_put(struct caif_device_entry *e)
+{
+	irqsafe_cpu_dec(*e->pcpu_refcnt);
+}
+
+static void caifd_hold(struct caif_device_entry *e)
+{
+	irqsafe_cpu_inc(*e->pcpu_refcnt);
+}
+
+static int caifd_refcnt_read(struct caif_device_entry *e)
+{
+	int i, refcnt = 0;
+	for_each_possible_cpu(i)
+		refcnt += *per_cpu_ptr(e->pcpu_refcnt, i);
+	return refcnt;
+}
+
 /* Allocate new CAIF device. */
 static struct caif_device_entry *caif_device_alloc(struct net_device *dev)
 {
 	struct caif_device_entry_list *caifdevs;
 	struct caif_device_entry *caifd;
+
 	caifdevs = caif_device_list(dev_net(dev));
 	BUG_ON(!caifdevs);
+
 	caifd = kzalloc(sizeof(*caifd), GFP_ATOMIC);
 	if (!caifd)
 		return NULL;
+	caifd->pcpu_refcnt = alloc_percpu(int);
 	caifd->netdev = dev;
-	list_add(&caifd->list, &caifdevs->list);
-	init_waitqueue_head(&caifd->event);
+	dev_hold(dev);
 	return caifd;
 }
 
@@ -87,35 +100,13 @@ static struct caif_device_entry *caif_get(struct net_device *dev)
 	    caif_device_list(dev_net(dev));
 	struct caif_device_entry *caifd;
 	BUG_ON(!caifdevs);
-	list_for_each_entry(caifd, &caifdevs->list, list) {
+	list_for_each_entry_rcu(caifd, &caifdevs->list, list) {
 		if (caifd->netdev == dev)
 			return caifd;
 	}
 	return NULL;
 }
 
-static void caif_device_destroy(struct net_device *dev)
-{
-	struct caif_device_entry_list *caifdevs =
-	    caif_device_list(dev_net(dev));
-	struct caif_device_entry *caifd;
-	ASSERT_RTNL();
-	if (dev->type != ARPHRD_CAIF)
-		return;
-
-	spin_lock_bh(&caifdevs->lock);
-	caifd = caif_get(dev);
-	if (caifd == NULL) {
-		spin_unlock_bh(&caifdevs->lock);
-		return;
-	}
-
-	list_del(&caifd->list);
-	spin_unlock_bh(&caifdevs->lock);
-
-	kfree(caifd);
-}
-
 static int transmit(struct cflayer *layer, struct cfpkt *pkt)
 {
 	struct caif_device_entry *caifd =
@@ -130,23 +121,8 @@ static int transmit(struct cflayer *layer, struct cfpkt *pkt)
 	return 0;
 }
 
-static int modemcmd(struct cflayer *layr, enum caif_modemcmd ctrl)
-{
-	struct caif_device_entry *caifd;
-	caifd = container_of(layr, struct caif_device_entry, layer);
-	if (ctrl == _CAIF_MODEMCMD_PHYIF_USEFULL) {
-		atomic_set(&caifd->in_use, 1);
-		wake_up_interruptible(&caifd->event);
-
-	} else if (ctrl == _CAIF_MODEMCMD_PHYIF_USELESS) {
-		atomic_set(&caifd->in_use, 0);
-		wake_up_interruptible(&caifd->event);
-	}
-	return 0;
-}
-
 /*
- * Stuff received packets to associated sockets.
+ * Stuff received packets into the CAIF stack.
  * On error, returns non-zero and releases the skb.
  */
 static int receive(struct sk_buff *skb, struct net_device *dev,
@@ -154,14 +130,27 @@ static int receive(struct sk_buff *skb, struct net_device *dev,
 {
 	struct cfpkt *pkt;
 	struct caif_device_entry *caifd;
+
 	pkt = cfpkt_fromnative(CAIF_DIR_IN, skb);
+
+	rcu_read_lock();
 	caifd = caif_get(dev);
-	if (!caifd || !caifd->layer.up || !caifd->layer.up->receive)
-		return NET_RX_DROP;
 
-	if (caifd->layer.up->receive(caifd->layer.up, pkt))
+	if (!caifd || !caifd->layer.up || !caifd->layer.up->receive ||
+			!netif_oper_up(caifd->netdev)) {
+		rcu_read_unlock();
+		kfree_skb(skb);
 		return NET_RX_DROP;
+	}
+
+	/* Hold reference to netdevice while using CAIF stack */
+	caifd_hold(caifd);
+	rcu_read_unlock();
 
+	caifd->layer.up->receive(caifd->layer.up, pkt);
+
+	/* Release reference to stack upwards */
+	caifd_put(caifd);
 	return 0;
 }
 
@@ -172,15 +161,25 @@ static struct packet_type caif_packet_type __read_mostly = {
 
 static void dev_flowctrl(struct net_device *dev, int on)
 {
-	struct caif_device_entry *caifd = caif_get(dev);
-	if (!caifd || !caifd->layer.up || !caifd->layer.up->ctrlcmd)
+	struct caif_device_entry *caifd;
+
+	rcu_read_lock();
+
+	caifd = caif_get(dev);
+	if (!caifd || !caifd->layer.up || !caifd->layer.up->ctrlcmd) {
+		rcu_read_unlock();
 		return;
+	}
+
+	caifd_hold(caifd);
+	rcu_read_unlock();
 
 	caifd->layer.up->ctrlcmd(caifd->layer.up,
 				 on ?
 				 _CAIF_CTRLCMD_PHYIF_FLOW_ON_IND :
 				 _CAIF_CTRLCMD_PHYIF_FLOW_OFF_IND,
 				 caifd->layer.id);
+	caifd_put(caifd);
 }
 
 /* notify Caif of device events */
@@ -192,34 +191,22 @@ static int caif_device_notify(struct notifier_block *me, unsigned long what,
 	struct caif_dev_common *caifdev;
 	enum cfcnfg_phy_preference pref;
 	enum cfcnfg_phy_type phy_type;
+	struct caif_device_entry_list *caifdevs =
+	    caif_device_list(dev_net(dev));
 
 	if (dev->type != ARPHRD_CAIF)
 		return 0;
 
 	switch (what) {
 	case NETDEV_REGISTER:
-		netdev_info(dev, "register\n");
 		caifd = caif_device_alloc(dev);
-		if (caifd == NULL)
-			break;
+		if (!caifd)
+			return 0;
+
 		caifdev = netdev_priv(dev);
 		caifdev->flowctrl = dev_flowctrl;
-		atomic_set(&caifd->state, what);
-		break;
 
-	case NETDEV_UP:
-		netdev_info(dev, "up\n");
-		caifd = caif_get(dev);
-		if (caifd == NULL)
-			break;
-		caifdev = netdev_priv(dev);
-		if (atomic_read(&caifd->state) == NETDEV_UP) {
-			netdev_info(dev, "already up\n");
-			break;
-		}
-		atomic_set(&caifd->state, what);
 		caifd->layer.transmit = transmit;
-		caifd->layer.modemcmd = modemcmd;
 
 		if (caifdev->use_frag)
 			phy_type = CFPHYTYPE_FRAG;
@@ -237,62 +224,95 @@ static int caif_device_notify(struct notifier_block *me, unsigned long what,
 			pref = CFPHYPREF_HIGH_BW;
 			break;
 		}
-		dev_hold(dev);
+		strncpy(caifd->layer.name, dev->name,
+			sizeof(caifd->layer.name) - 1);
+		caifd->layer.name[sizeof(caifd->layer.name) - 1] = 0;
+
+		mutex_lock(&caifdevs->lock);
+		list_add_rcu(&caifd->list, &caifdevs->list);
+
 		cfcnfg_add_phy_layer(cfg,
 				     phy_type,
 				     dev,
 				     &caifd->layer,
-				     &caifd->phyid,
+				     0,
 				     pref,
 				     caifdev->use_fcs,
 				     caifdev->use_stx);
-		strncpy(caifd->layer.name, dev->name,
-			sizeof(caifd->layer.name) - 1);
-		caifd->layer.name[sizeof(caifd->layer.name) - 1] = 0;
+		mutex_unlock(&caifdevs->lock);
 		break;
 
-	case NETDEV_GOING_DOWN:
+	case NETDEV_UP:
+		rcu_read_lock();
+
 		caifd = caif_get(dev);
-		if (caifd == NULL)
+		if (caifd == NULL) {
+			rcu_read_unlock();
 			break;
-		netdev_info(dev, "going down\n");
+		}
 
-		if (atomic_read(&caifd->state) == NETDEV_GOING_DOWN ||
-			atomic_read(&caifd->state) == NETDEV_DOWN)
-			break;
+		cfcnfg_set_phy_state(cfg, &caifd->layer, true);
+		rcu_read_unlock();
 
-		atomic_set(&caifd->state, what);
-		if (!caifd || !caifd->layer.up || !caifd->layer.up->ctrlcmd)
-			return -EINVAL;
-		caifd->layer.up->ctrlcmd(caifd->layer.up,
-					 _CAIF_CTRLCMD_PHYIF_DOWN_IND,
-					 caifd->layer.id);
-		might_sleep();
-		wait_event_interruptible_timeout(caifd->event,
-					atomic_read(&caifd->in_use) == 0,
-					TIMEOUT);
 		break;
 
 	case NETDEV_DOWN:
+		rcu_read_lock();
+
 		caifd = caif_get(dev);
-		if (caifd == NULL)
-			break;
-		netdev_info(dev, "down\n");
-		if (atomic_read(&caifd->in_use))
-			netdev_warn(dev,
-				    "Unregistering an active CAIF device\n");
-		cfcnfg_del_phy_layer(cfg, &caifd->layer);
-		dev_put(dev);
-		atomic_set(&caifd->state, what);
+		if (!caifd || !caifd->layer.up || !caifd->layer.up->ctrlcmd) {
+			rcu_read_unlock();
+			return -EINVAL;
+		}
+
+		cfcnfg_set_phy_state(cfg, &caifd->layer, false);
+		caifd_hold(caifd);
+		rcu_read_unlock();
+
+		caifd->layer.up->ctrlcmd(caifd->layer.up,
+					 _CAIF_CTRLCMD_PHYIF_DOWN_IND,
+					 caifd->layer.id);
+		caifd_put(caifd);
 		break;
 
 	case NETDEV_UNREGISTER:
+		mutex_lock(&caifdevs->lock);
+
 		caifd = caif_get(dev);
-		if (caifd == NULL)
+		if (caifd == NULL) {
+			mutex_unlock(&caifdevs->lock);
 			break;
-		netdev_info(dev, "unregister\n");
-		atomic_set(&caifd->state, what);
-		caif_device_destroy(dev);
+		}
+		list_del_rcu(&caifd->list);
+
+		/*
+		 * NETDEV_UNREGISTER is called repeatedly until all reference
+		 * counts for the net-device are released. If references to
+		 * caifd is taken, simply ignore NETDEV_UNREGISTER and wait for
+		 * the next call to NETDEV_UNREGISTER.
+		 *
+		 * If any packets are in flight down the CAIF Stack,
+		 * cfcnfg_del_phy_layer will return nonzero.
+		 * If no packets are in flight, the CAIF Stack associated
+		 * with the net-device un-registering is freed.
+		 */
+
+		if (caifd_refcnt_read(caifd) != 0 ||
+			cfcnfg_del_phy_layer(cfg, &caifd->layer) != 0) {
+
+			pr_info("Wait for device inuse\n");
+			/* Enrole device if CAIF Stack is still in use */
+			list_add_rcu(&caifd->list, &caifdevs->list);
+			mutex_unlock(&caifdevs->lock);
+			break;
+		}
+
+		synchronize_rcu();
+		dev_put(caifd->netdev);
+		free_percpu(caifd->pcpu_refcnt);
+		kfree(caifd);
+
+		mutex_unlock(&caifdevs->lock);
 		break;
 	}
 	return 0;
@@ -304,8 +324,8 @@ static struct notifier_block caif_device_notifier = {
 };
 
 int caif_connect_client(struct caif_connect_request *conn_req,
-			struct cflayer *client_layer, int *ifindex,
-			int *headroom, int *tailroom)
+		       struct cflayer *client_layer, int *ifindex,
+		       int *headroom, int *tailroom)
 {
 	struct cfctrl_link_param param;
 	int ret;
@@ -315,8 +335,8 @@ int caif_connect_client(struct caif_connect_request *conn_req,
 		return ret;
 	/* Hook up the adaptation layer. */
 	return cfcnfg_add_adaptation_layer(cfg, &param,
-					client_layer, ifindex,
-					headroom, tailroom);
+				       client_layer, ifindex,
+				       headroom, tailroom);
 }
 EXPORT_SYMBOL(caif_connect_client);
 
@@ -331,20 +351,40 @@ static int caif_init_net(struct net *net)
 {
 	struct caif_net *caifn = net_generic(net, caif_net_id);
 	INIT_LIST_HEAD(&caifn->caifdevs.list);
-	spin_lock_init(&caifn->caifdevs.lock);
+	mutex_init(&caifn->caifdevs.lock);
 	return 0;
 }
 
 static void caif_exit_net(struct net *net)
 {
-	struct net_device *dev;
+	struct caif_device_entry *caifd, *tmp;
+	struct caif_device_entry_list *caifdevs =
+	    caif_device_list(net);
+
 	rtnl_lock();
-	for_each_netdev(net, dev) {
-		if (dev->type != ARPHRD_CAIF)
-			continue;
-		dev_close(dev);
-		caif_device_destroy(dev);
+	mutex_lock(&caifdevs->lock);
+
+	list_for_each_entry_safe(caifd, tmp, &caifdevs->list, list) {
+		int i = 0;
+		list_del_rcu(&caifd->list);
+		cfcnfg_set_phy_state(cfg, &caifd->layer, false);
+
+		while (i < 10 &&
+			(caifd_refcnt_read(caifd) != 0 ||
+			cfcnfg_del_phy_layer(cfg, &caifd->layer) != 0)) {
+
+			pr_info("Wait for device inuse\n");
+			msleep(250);
+			i++;
+		}
+		synchronize_rcu();
+		dev_put(caifd->netdev);
+		free_percpu(caifd->pcpu_refcnt);
+		kfree(caifd);
 	}
+
+
+	mutex_unlock(&caifdevs->lock);
 	rtnl_unlock();
 }
 
@@ -359,6 +399,7 @@ static struct pernet_operations caif_net_ops = {
 static int __init caif_device_init(void)
 {
 	int result;
+
 	cfg = cfcnfg_create();
 	if (!cfg) {
 		pr_warn("can't create cfcnfg\n");
-- 
1.7.1


^ permalink raw reply related

* [net-next-2.6 10/10] caif: remove unesesarry exports
From: Sjur Brændeland @ 2011-05-13 12:44 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland
In-Reply-To: <1305290648-9613-1-git-send-email-sjur.brandeland@stericsson.com>

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 include/net/caif/cfpkt.h |    1 -
 net/caif/caif_socket.c   |    4 ++--
 net/caif/cfcnfg.c        |    1 -
 net/caif/cfpkt_skbuff.c  |   27 ++++++++-------------------
 net/caif/chnl_net.c      |    5 +++--
 5 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/include/net/caif/cfpkt.h b/include/net/caif/cfpkt.h
index 8b550f8..6bd200a 100644
--- a/include/net/caif/cfpkt.h
+++ b/include/net/caif/cfpkt.h
@@ -195,5 +195,4 @@ void *cfpkt_tonative(struct cfpkt *pkt);
  * @return Packet information
  */
 struct caif_payload_info *cfpkt_info(struct cfpkt *pkt);
-/*! @} */
 #endif				/* CFPKT_H_ */
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 5cf9c73..b840395 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -205,7 +205,7 @@ static int caif_sktrecv_cb(struct cflayer *layr, struct cfpkt *pkt)
 	skb = cfpkt_tonative(pkt);
 
 	if (unlikely(cf_sk->sk.sk_state != CAIF_CONNECTED)) {
-		cfpkt_destroy(pkt);
+		kfree_skb(skb);
 		return 0;
 	}
 	caif_queue_rcv_skb(&cf_sk->sk, skb);
@@ -537,7 +537,7 @@ static int transmit_skb(struct sk_buff *skb, struct caifsock *cf_sk,
 	struct cfpkt *pkt;
 
 	pkt = cfpkt_fromnative(CAIF_DIR_OUT, skb);
-	memset(cfpkt_info(pkt), 0, sizeof(struct caif_payload_info));
+	memset(skb->cb, 0, sizeof(struct caif_payload_info));
 
 	if (cf_sk->layer.dn == NULL)
 		return -EINVAL;
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index 4230099..351c2ca 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -117,7 +117,6 @@ out_of_mem:
 	kfree(this);
 	return NULL;
 }
-EXPORT_SYMBOL(cfcnfg_create);
 
 void cfcnfg_remove(struct cfcnfg *cfg)
 {
diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c
index 20c6cb3..75d4bfa 100644
--- a/net/caif/cfpkt_skbuff.c
+++ b/net/caif/cfpkt_skbuff.c
@@ -97,21 +97,20 @@ inline struct cfpkt *cfpkt_create(u16 len)
 {
 	return cfpkt_create_pfx(len + PKT_POSTFIX, PKT_PREFIX);
 }
-EXPORT_SYMBOL(cfpkt_create);
 
 void cfpkt_destroy(struct cfpkt *pkt)
 {
 	struct sk_buff *skb = pkt_to_skb(pkt);
 	kfree_skb(skb);
 }
-EXPORT_SYMBOL(cfpkt_destroy);
+
 
 inline bool cfpkt_more(struct cfpkt *pkt)
 {
 	struct sk_buff *skb = pkt_to_skb(pkt);
 	return skb->len > 0;
 }
-EXPORT_SYMBOL(cfpkt_more);
+
 
 int cfpkt_peek_head(struct cfpkt *pkt, void *data, u16 len)
 {
@@ -123,7 +122,6 @@ int cfpkt_peek_head(struct cfpkt *pkt, void *data, u16 len)
 	return !cfpkt_extr_head(pkt, data, len) &&
 	    !cfpkt_add_head(pkt, data, len);
 }
-EXPORT_SYMBOL(cfpkt_peek_head);
 
 int cfpkt_extr_head(struct cfpkt *pkt, void *data, u16 len)
 {
@@ -148,7 +146,6 @@ int cfpkt_extr_head(struct cfpkt *pkt, void *data, u16 len)
 	memcpy(data, from, len);
 	return 0;
 }
-EXPORT_SYMBOL(cfpkt_extr_head);
 
 int cfpkt_extr_trail(struct cfpkt *pkt, void *dta, u16 len)
 {
@@ -171,13 +168,13 @@ int cfpkt_extr_trail(struct cfpkt *pkt, void *dta, u16 len)
 	memcpy(data, from, len);
 	return 0;
 }
-EXPORT_SYMBOL(cfpkt_extr_trail);
+
 
 int cfpkt_pad_trail(struct cfpkt *pkt, u16 len)
 {
 	return cfpkt_add_body(pkt, NULL, len);
 }
-EXPORT_SYMBOL(cfpkt_pad_trail);
+
 
 int cfpkt_add_body(struct cfpkt *pkt, const void *data, u16 len)
 {
@@ -226,13 +223,11 @@ int cfpkt_add_body(struct cfpkt *pkt, const void *data, u16 len)
 		memcpy(to, data, len);
 	return 0;
 }
-EXPORT_SYMBOL(cfpkt_add_body);
 
 inline int cfpkt_addbdy(struct cfpkt *pkt, u8 data)
 {
 	return cfpkt_add_body(pkt, &data, 1);
 }
-EXPORT_SYMBOL(cfpkt_addbdy);
 
 int cfpkt_add_head(struct cfpkt *pkt, const void *data2, u16 len)
 {
@@ -259,20 +254,20 @@ int cfpkt_add_head(struct cfpkt *pkt, const void *data2, u16 len)
 	memcpy(to, data, len);
 	return 0;
 }
-EXPORT_SYMBOL(cfpkt_add_head);
+
 
 inline int cfpkt_add_trail(struct cfpkt *pkt, const void *data, u16 len)
 {
 	return cfpkt_add_body(pkt, data, len);
 }
-EXPORT_SYMBOL(cfpkt_add_trail);
+
 
 inline u16 cfpkt_getlen(struct cfpkt *pkt)
 {
 	struct sk_buff *skb = pkt_to_skb(pkt);
 	return skb->len;
 }
-EXPORT_SYMBOL(cfpkt_getlen);
+
 
 inline u16 cfpkt_iterate(struct cfpkt *pkt,
 			    u16 (*iter_func)(u16, void *, u16),
@@ -290,7 +285,7 @@ inline u16 cfpkt_iterate(struct cfpkt *pkt,
 	}
 	return iter_func(data, pkt->skb.data, cfpkt_getlen(pkt));
 }
-EXPORT_SYMBOL(cfpkt_iterate);
+
 
 int cfpkt_setlen(struct cfpkt *pkt, u16 len)
 {
@@ -315,7 +310,6 @@ int cfpkt_setlen(struct cfpkt *pkt, u16 len)
 
 	return cfpkt_getlen(pkt);
 }
-EXPORT_SYMBOL(cfpkt_setlen);
 
 struct cfpkt *cfpkt_append(struct cfpkt *dstpkt,
 			     struct cfpkt *addpkt,
@@ -357,7 +351,6 @@ struct cfpkt *cfpkt_append(struct cfpkt *dstpkt,
 	dst->len += addlen;
 	return skb_to_pkt(dst);
 }
-EXPORT_SYMBOL(cfpkt_append);
 
 struct cfpkt *cfpkt_split(struct cfpkt *pkt, u16 pos)
 {
@@ -395,17 +388,13 @@ struct cfpkt *cfpkt_split(struct cfpkt *pkt, u16 pos)
 	skb2->len += len2nd;
 	return skb_to_pkt(skb2);
 }
-EXPORT_SYMBOL(cfpkt_split);
 
 bool cfpkt_erroneous(struct cfpkt *pkt)
 {
 	return cfpkt_priv(pkt)->erronous;
 }
-EXPORT_SYMBOL(cfpkt_erroneous);
-
 
 struct caif_payload_info *cfpkt_info(struct cfpkt *pkt)
 {
 	return (struct caif_payload_info *)&pkt_to_skb(pkt)->cb;
 }
-EXPORT_SYMBOL(cfpkt_info);
diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 1a3398a..649ebac 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -83,10 +83,11 @@ static int chnl_recv_cb(struct cflayer *layr, struct cfpkt *pkt)
 	if (!priv)
 		return -EINVAL;
 
+	skb = (struct sk_buff *) cfpkt_tonative(pkt);
+
 	/* Get length of CAIF packet. */
-	pktlen = cfpkt_getlen(pkt);
+	pktlen = skb->len;
 
-	skb = (struct sk_buff *) cfpkt_tonative(pkt);
 	/* Pass some minimum information and
 	 * send the packet to the net stack.
 	 */
-- 
1.7.1


^ permalink raw reply related

* [net-next-2.6 06/10] caif: Protected in-flight packets using dev or sock refcont.
From: Sjur Brændeland @ 2011-05-13 12:44 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland
In-Reply-To: <1305290648-9613-1-git-send-email-sjur.brandeland@stericsson.com>

CAIF Socket Layer and ip-interface registers reference counters
in CAIF service layer. The functions sock_hold, sock_put and
dev_hold, dev_put are used by CAIF Stack to protect from freeing
memory while packets are in-flight.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 include/net/caif/caif_dev.h |   29 +++++++++++++++++++++++++++++
 net/caif/caif_socket.c      |   19 +++++++++++++++++--
 net/caif/chnl_net.c         |   28 ++++++++++++++++++++++++++--
 3 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/include/net/caif/caif_dev.h b/include/net/caif/caif_dev.h
index 7e3f7a6..6638435 100644
--- a/include/net/caif/caif_dev.h
+++ b/include/net/caif/caif_dev.h
@@ -74,6 +74,23 @@ int caif_connect_client(struct caif_connect_request *conn_req,
 int caif_disconnect_client(struct cflayer *client_layer);
 
 /**
+ * caif_client_register_refcnt - register ref-count functions provided by client.
+ *
+ * @adapt_layer: Client layer using CAIF Stack.
+ * @hold:	Function provided by client layer increasing ref-count
+ * @put:	Function provided by client layer decreasing ref-count
+ *
+ * Client of the CAIF Stack must register functions for reference counting.
+ * These functions are called by the CAIF Stack for every upstream packet,
+ * and must therefore be implemented efficiently.
+ *
+ * Client should call caif_free_client when reference count degrease to zero.
+ */
+
+void caif_client_register_refcnt(struct cflayer *adapt_layer,
+					void (*hold)(struct cflayer *lyr),
+					void (*put)(struct cflayer *lyr));
+/**
  * caif_connect_req_to_link_param - Translate configuration parameters
  *				    from socket format to internal format.
  * @cnfg:	Pointer to configuration handler
@@ -83,8 +100,20 @@ int caif_disconnect_client(struct cflayer *client_layer);
  *			 setting up channels.
  *
  */
+
 int caif_connect_req_to_link_param(struct cfcnfg *cnfg,
 				   struct caif_connect_request *con_req,
 				   struct cfctrl_link_param *setup_param);
 
+/**
+ * caif_free_client - Free memory used to manage the client in the CAIF Stack.
+ *
+ * @client_layer: Client layer to be removed.
+ *
+ * This function must be called from client layer in order to free memory.
+ * Caller must guarantee that no packets are in flight upstream when calling
+ * this function.
+ */
+void caif_free_client(struct cflayer *adap_layer);
+
 #endif /* CAIF_DEV_H_ */
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 2021242..01f612d 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -209,6 +209,18 @@ static int caif_sktrecv_cb(struct cflayer *layr, struct cfpkt *pkt)
 	return 0;
 }
 
+static void cfsk_hold(struct cflayer *layr)
+{
+	struct caifsock *cf_sk = container_of(layr, struct caifsock, layer);
+	sock_hold(&cf_sk->sk);
+}
+
+static void cfsk_put(struct cflayer *layr)
+{
+	struct caifsock *cf_sk = container_of(layr, struct caifsock, layer);
+	sock_put(&cf_sk->sk);
+}
+
 /* Packet Control Callback function called from CAIF */
 static void caif_ctrl_cb(struct cflayer *layr,
 				enum caif_ctrlcmd flow,
@@ -232,6 +244,8 @@ static void caif_ctrl_cb(struct cflayer *layr,
 
 	case CAIF_CTRLCMD_INIT_RSP:
 		/* We're now connected */
+		caif_client_register_refcnt(&cf_sk->layer,
+						cfsk_hold, cfsk_put);
 		dbfs_atomic_inc(&cnt.num_connect_resp);
 		cf_sk->sk.sk_state = CAIF_CONNECTED;
 		set_tx_flow_on(cf_sk);
@@ -242,7 +256,6 @@ static void caif_ctrl_cb(struct cflayer *layr,
 		/* We're now disconnected */
 		cf_sk->sk.sk_state = CAIF_DISCONNECTED;
 		cf_sk->sk.sk_state_change(&cf_sk->sk);
-		cfcnfg_release_adap_layer(&cf_sk->layer);
 		break;
 
 	case CAIF_CTRLCMD_INIT_FAIL_RSP:
@@ -837,8 +850,10 @@ static int caif_connect(struct socket *sock, struct sockaddr *uaddr,
 
 	dbfs_atomic_inc(&cnt.num_connect_req);
 	cf_sk->layer.receive = caif_sktrecv_cb;
+
 	err = caif_connect_client(&cf_sk->conn_req,
 				&cf_sk->layer, &ifindex, &headroom, &tailroom);
+
 	if (err < 0) {
 		cf_sk->sk.sk_socket->state = SS_UNCONNECTED;
 		cf_sk->sk.sk_state = CAIF_DISCONNECTED;
@@ -940,7 +955,6 @@ static int caif_release(struct socket *sock)
 	wake_up_interruptible_poll(sk_sleep(sk), POLLERR|POLLHUP);
 
 	sock_orphan(sk);
-	cf_sk->layer.dn = NULL;
 	sk_stream_kill_queues(&cf_sk->sk);
 	release_sock(sk);
 	sock_put(sk);
@@ -1036,6 +1050,7 @@ static void caif_sock_destructor(struct sock *sk)
 	}
 	sk_stream_kill_queues(&cf_sk->sk);
 	dbfs_atomic_dec(&cnt.caif_nr_socks);
+	caif_free_client(&cf_sk->layer);
 }
 
 static int caif_create(struct net *net, struct socket *sock, int protocol,
diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 6008d6d..9ef8f16 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -153,6 +153,18 @@ static void close_work(struct work_struct *work)
 }
 static DECLARE_WORK(close_worker, close_work);
 
+static void chnl_hold(struct cflayer *lyr)
+{
+	struct chnl_net *priv = container_of(lyr, struct chnl_net, chnl);
+	dev_hold(priv->netdev);
+}
+
+static void chnl_put(struct cflayer *lyr)
+{
+	struct chnl_net *priv = container_of(lyr, struct chnl_net, chnl);
+	dev_put(priv->netdev);
+}
+
 static void chnl_flowctrl_cb(struct cflayer *layr, enum caif_ctrlcmd flow,
 				int phyid)
 {
@@ -190,6 +202,7 @@ static void chnl_flowctrl_cb(struct cflayer *layr, enum caif_ctrlcmd flow,
 		netif_wake_queue(priv->netdev);
 		break;
 	case CAIF_CTRLCMD_INIT_RSP:
+		caif_client_register_refcnt(&priv->chnl, chnl_hold, chnl_put);
 		priv->state = CAIF_CONNECTED;
 		priv->flowenabled = true;
 		netif_wake_queue(priv->netdev);
@@ -373,11 +386,18 @@ static const struct net_device_ops netdev_ops = {
 	.ndo_start_xmit = chnl_net_start_xmit,
 };
 
+static void chnl_net_destructor(struct net_device *dev)
+{
+	struct chnl_net *priv = netdev_priv(dev);
+	caif_free_client(&priv->chnl);
+	free_netdev(dev);
+}
+
 static void ipcaif_net_setup(struct net_device *dev)
 {
 	struct chnl_net *priv;
 	dev->netdev_ops = &netdev_ops;
-	dev->destructor = free_netdev;
+	dev->destructor = chnl_net_destructor;
 	dev->flags |= IFF_NOARP;
 	dev->flags |= IFF_POINTOPOINT;
 	dev->mtu = GPRS_PDP_MTU;
@@ -391,7 +411,7 @@ static void ipcaif_net_setup(struct net_device *dev)
 	priv->conn_req.link_selector = CAIF_LINK_HIGH_BANDW;
 	priv->conn_req.priority = CAIF_PRIO_LOW;
 	/* Insert illegal value */
-	priv->conn_req.sockaddr.u.dgm.connection_id = -1;
+	priv->conn_req.sockaddr.u.dgm.connection_id = 0;
 	priv->flowenabled = false;
 
 	init_waitqueue_head(&priv->netmgmt_wq);
@@ -453,6 +473,10 @@ static int ipcaif_newlink(struct net *src_net, struct net_device *dev,
 		pr_warn("device rtml registration failed\n");
 	else
 		list_add(&caifdev->list_field, &chnl_net_list);
+
+	/* Take ifindex as connection-id if null */
+	if (caifdev->conn_req.sockaddr.u.dgm.connection_id == 0)
+		caifdev->conn_req.sockaddr.u.dgm.connection_id = dev->ifindex;
 	return ret;
 }
 
-- 
1.7.1


^ permalink raw reply related

* [net-next-2.6 05/10] caif: Move refcount from service layer to sock and dev.
From: Sjur Brændeland @ 2011-05-13 12:44 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland
In-Reply-To: <1305290648-9613-1-git-send-email-sjur.brandeland@stericsson.com>

Instead of having reference counts in caif service layers,
we hook into existing refcount handling in socket layer and netdevice.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 include/net/caif/cfsrvl.h |   29 ++++++++++++++++-------------
 net/caif/cfrfml.c         |    4 ++--
 net/caif/cfsrvl.c         |   35 +++++++++++++++++++++++++++++------
 3 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/include/net/caif/cfsrvl.h b/include/net/caif/cfsrvl.h
index 6c8279c..0f59052 100644
--- a/include/net/caif/cfsrvl.h
+++ b/include/net/caif/cfsrvl.h
@@ -10,6 +10,7 @@
 #include <linux/stddef.h>
 #include <linux/types.h>
 #include <linux/kref.h>
+#include <linux/rculist.h>
 
 struct cfsrvl {
 	struct cflayer layer;
@@ -17,9 +18,11 @@ struct cfsrvl {
 	bool phy_flow_on;
 	bool modem_flow_on;
 	bool supports_flowctrl;
-	void (*release)(struct kref *);
+	void (*release)(struct cflayer *layer);
 	struct dev_info dev_info;
-	struct kref ref;
+	void (*hold)(struct cflayer *lyr);
+	void (*put)(struct cflayer *lyr);
+	struct rcu_head rcu;
 };
 
 struct cflayer *cfvei_create(u8 linkid, struct dev_info *dev_info);
@@ -29,6 +32,10 @@ struct cflayer *cfvidl_create(u8 linkid, struct dev_info *dev_info);
 struct cflayer *cfrfml_create(u8 linkid, struct dev_info *dev_info,
 				int mtu_size);
 struct cflayer *cfdbgl_create(u8 linkid, struct dev_info *dev_info);
+
+void cfsrvl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
+		     int phyid);
+
 bool cfsrvl_phyid_match(struct cflayer *layer, int phyid);
 
 void cfsrvl_init(struct cfsrvl *service,
@@ -40,23 +47,19 @@ u8 cfsrvl_getphyid(struct cflayer *layer);
 
 static inline void cfsrvl_get(struct cflayer *layr)
 {
-	struct cfsrvl *s;
-	if (layr == NULL)
+	struct cfsrvl *s = container_of(layr, struct cfsrvl, layer);
+	if (layr == NULL || layr->up == NULL || s->hold == NULL)
 		return;
-	s = container_of(layr, struct cfsrvl, layer);
-	kref_get(&s->ref);
+
+	s->hold(layr->up);
 }
 
 static inline void cfsrvl_put(struct cflayer *layr)
 {
-	struct cfsrvl *s;
-	if (layr == NULL)
+	struct cfsrvl *s = container_of(layr, struct cfsrvl, layer);
+	if (layr == NULL || layr->up == NULL || s->hold == NULL)
 		return;
-	s = container_of(layr, struct cfsrvl, layer);
 
-	WARN_ON(!s->release);
-	if (s->release)
-		kref_put(&s->ref, s->release);
+	s->put(layr->up);
 }
-
 #endif				/* CFSRVL_H_ */
diff --git a/net/caif/cfrfml.c b/net/caif/cfrfml.c
index e2fb5fa..0deabb4 100644
--- a/net/caif/cfrfml.c
+++ b/net/caif/cfrfml.c
@@ -31,9 +31,9 @@ struct cfrfml {
 	spinlock_t sync;
 };
 
-static void cfrfml_release(struct kref *kref)
+static void cfrfml_release(struct cflayer *layer)
 {
-	struct cfsrvl *srvl = container_of(kref, struct cfsrvl, ref);
+	struct cfsrvl *srvl = container_of(layer, struct cfsrvl, layer);
 	struct cfrfml *rfml = container_obj(&srvl->layer);
 
 	if (rfml->incomplete_frm)
diff --git a/net/caif/cfsrvl.c b/net/caif/cfsrvl.c
index 24ba392..535a1e7 100644
--- a/net/caif/cfsrvl.c
+++ b/net/caif/cfsrvl.c
@@ -10,6 +10,7 @@
 #include <linux/types.h>
 #include <linux/errno.h>
 #include <linux/slab.h>
+#include <linux/module.h>
 #include <net/caif/caif_layer.h>
 #include <net/caif/cfsrvl.h>
 #include <net/caif/cfpkt.h>
@@ -27,8 +28,8 @@ static void cfservl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 {
 	struct cfsrvl *service = container_obj(layr);
 
-	caif_assert(layr->up != NULL);
-	caif_assert(layr->up->ctrlcmd != NULL);
+	if (layr->up == NULL || layr->up->ctrlcmd == NULL)
+		return;
 
 	switch (ctrl) {
 	case CAIF_CTRLCMD_INIT_RSP:
@@ -151,9 +152,9 @@ static int cfservl_modemcmd(struct cflayer *layr, enum caif_modemcmd ctrl)
 	return -EINVAL;
 }
 
-static void cfsrvl_release(struct kref *kref)
+static void cfsrvl_release(struct cflayer *layer)
 {
-	struct cfsrvl *service = container_of(kref, struct cfsrvl, ref);
+	struct cfsrvl *service = container_of(layer, struct cfsrvl, layer);
 	kfree(service);
 }
 
@@ -173,10 +174,8 @@ void cfsrvl_init(struct cfsrvl *service,
 	service->dev_info = *dev_info;
 	service->supports_flowctrl = supports_flowctrl;
 	service->release = cfsrvl_release;
-	kref_init(&service->ref);
 }
 
-
 bool cfsrvl_ready(struct cfsrvl *service, int *err)
 {
 	if (service->open && service->modem_flow_on && service->phy_flow_on)
@@ -189,6 +188,7 @@ bool cfsrvl_ready(struct cfsrvl *service, int *err)
 	*err = -EAGAIN;
 	return false;
 }
+
 u8 cfsrvl_getphyid(struct cflayer *layer)
 {
 	struct cfsrvl *servl = container_obj(layer);
@@ -200,3 +200,26 @@ bool cfsrvl_phyid_match(struct cflayer *layer, int phyid)
 	struct cfsrvl *servl = container_obj(layer);
 	return servl->dev_info.id == phyid;
 }
+
+void caif_free_client(struct cflayer *adap_layer)
+{
+	struct cfsrvl *servl;
+	if (adap_layer == NULL || adap_layer->dn == NULL)
+		return;
+	servl = container_obj(adap_layer->dn);
+	servl->release(&servl->layer);
+}
+EXPORT_SYMBOL(caif_free_client);
+
+void caif_client_register_refcnt(struct cflayer *adapt_layer,
+					void (*hold)(struct cflayer *lyr),
+					void (*put)(struct cflayer *lyr))
+{
+	struct cfsrvl *service;
+	service = container_of(adapt_layer->dn, struct cfsrvl, layer);
+
+	WARN_ON(adapt_layer == NULL || adapt_layer->dn == NULL);
+	service->hold = hold;
+	service->put = put;
+}
+EXPORT_SYMBOL(caif_client_register_refcnt);
-- 
1.7.1


^ permalink raw reply related

* [net-next-2.6 08/10] caif: Handle dev_queue_xmit errors.
From: Sjur Brændeland @ 2011-05-13 12:44 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland
In-Reply-To: <1305290648-9613-1-git-send-email-sjur.brandeland@stericsson.com>

Do proper handling of dev_queue_xmit errors in order to
avoid double free of skb and leaks in error conditions.
In cfctrl pending requests are removed when CAIF Link layer goes down.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 include/net/caif/cfctrl.h |    3 +-
 net/caif/caif_dev.c       |    7 ++-
 net/caif/caif_socket.c    |    8 ++-
 net/caif/cfcnfg.c         |    2 +-
 net/caif/cfctrl.c         |  121 +++++++++++++++++++++++++++++++-------------
 net/caif/cffrml.c         |   17 ++++++-
 net/caif/cfveil.c         |    8 ++-
 7 files changed, 119 insertions(+), 47 deletions(-)

diff --git a/include/net/caif/cfctrl.h b/include/net/caif/cfctrl.h
index d84416f..9e5425b 100644
--- a/include/net/caif/cfctrl.h
+++ b/include/net/caif/cfctrl.h
@@ -124,6 +124,7 @@ int  cfctrl_linkdown_req(struct cflayer *cfctrl, u8 linkid,
 
 struct cflayer *cfctrl_create(void);
 struct cfctrl_rsp *cfctrl_get_respfuncs(struct cflayer *layer);
-void cfctrl_cancel_req(struct cflayer *layr, struct cflayer *adap_layer);
+int cfctrl_cancel_req(struct cflayer *layr, struct cflayer *adap_layer);
+void cfctrl_remove(struct cflayer *layr);
 
 #endif				/* CFCTRL_H_ */
diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index 0e651cf..366ca0f 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -118,6 +118,7 @@ static struct caif_device_entry *caif_get(struct net_device *dev)
 
 static int transmit(struct cflayer *layer, struct cfpkt *pkt)
 {
+	int err;
 	struct caif_device_entry *caifd =
 	    container_of(layer, struct caif_device_entry, layer);
 	struct sk_buff *skb;
@@ -125,9 +126,11 @@ static int transmit(struct cflayer *layer, struct cfpkt *pkt)
 	skb = cfpkt_tonative(pkt);
 	skb->dev = caifd->netdev;
 
-	dev_queue_xmit(skb);
+	err = dev_queue_xmit(skb);
+	if (err > 0)
+		err = -EIO;
 
-	return 0;
+	return err;
 }
 
 /*
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 653db75..7baae11 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -604,7 +604,9 @@ static int caif_seqpkt_sendmsg(struct kiocb *kiocb, struct socket *sock,
 		goto err;
 	ret = transmit_skb(skb, cf_sk, noblock, timeo);
 	if (ret < 0)
-		goto err;
+		/* skb is already freed */
+		return ret;
+
 	return len;
 err:
 	kfree_skb(skb);
@@ -933,9 +935,9 @@ static int caif_release(struct socket *sock)
 	 * caif_queue_rcv_skb checks SOCK_DEAD holding the queue lock,
 	 * this ensures no packets when sock is dead.
 	 */
-	spin_lock(&sk->sk_receive_queue.lock);
+	spin_lock_bh(&sk->sk_receive_queue.lock);
 	sock_set_flag(sk, SOCK_DEAD);
-	spin_unlock(&sk->sk_receive_queue.lock);
+	spin_unlock_bh(&sk->sk_receive_queue.lock);
 	sock->sk = NULL;
 
 	dbfs_atomic_inc(&cnt.num_disconnect);
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index e857d89..4230099 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -126,7 +126,7 @@ void cfcnfg_remove(struct cfcnfg *cfg)
 		synchronize_rcu();
 
 		kfree(cfg->mux);
-		kfree(cfg->ctrl);
+		cfctrl_remove(cfg->ctrl);
 		kfree(cfg);
 	}
 }
diff --git a/net/caif/cfctrl.c b/net/caif/cfctrl.c
index 397a2c0..0c00a60 100644
--- a/net/caif/cfctrl.c
+++ b/net/caif/cfctrl.c
@@ -17,7 +17,6 @@
 #define UTILITY_NAME_LENGTH 16
 #define CFPKT_CTRL_PKT_LEN 20
 
-
 #ifdef CAIF_NO_LOOP
 static int handle_loop(struct cfctrl *ctrl,
 			      int cmd, struct cfpkt *pkt){
@@ -51,13 +50,29 @@ struct cflayer *cfctrl_create(void)
 	this->serv.layer.receive = cfctrl_recv;
 	sprintf(this->serv.layer.name, "ctrl");
 	this->serv.layer.ctrlcmd = cfctrl_ctrlcmd;
+#ifndef CAIF_NO_LOOP
 	spin_lock_init(&this->loop_linkid_lock);
+	this->loop_linkid = 1;
+#endif
 	spin_lock_init(&this->info_list_lock);
 	INIT_LIST_HEAD(&this->list);
-	this->loop_linkid = 1;
 	return &this->serv.layer;
 }
 
+void cfctrl_remove(struct cflayer *layer)
+{
+	struct cfctrl_request_info *p, *tmp;
+	struct cfctrl *ctrl = container_obj(layer);
+
+	spin_lock_bh(&ctrl->info_list_lock);
+	list_for_each_entry_safe(p, tmp, &ctrl->list, list) {
+		list_del(&p->list);
+		kfree(p);
+	}
+	spin_unlock_bh(&ctrl->info_list_lock);
+	kfree(layer);
+}
+
 static bool param_eq(const struct cfctrl_link_param *p1,
 			const struct cfctrl_link_param *p2)
 {
@@ -116,11 +131,11 @@ static bool cfctrl_req_eq(const struct cfctrl_request_info *r1,
 static void cfctrl_insert_req(struct cfctrl *ctrl,
 			      struct cfctrl_request_info *req)
 {
-	spin_lock(&ctrl->info_list_lock);
+	spin_lock_bh(&ctrl->info_list_lock);
 	atomic_inc(&ctrl->req_seq_no);
 	req->sequence_no = atomic_read(&ctrl->req_seq_no);
 	list_add_tail(&req->list, &ctrl->list);
-	spin_unlock(&ctrl->info_list_lock);
+	spin_unlock_bh(&ctrl->info_list_lock);
 }
 
 /* Compare and remove request */
@@ -129,7 +144,6 @@ static struct cfctrl_request_info *cfctrl_remove_req(struct cfctrl *ctrl,
 {
 	struct cfctrl_request_info *p, *tmp, *first;
 
-	spin_lock(&ctrl->info_list_lock);
 	first = list_first_entry(&ctrl->list, struct cfctrl_request_info, list);
 
 	list_for_each_entry_safe(p, tmp, &ctrl->list, list) {
@@ -145,7 +159,6 @@ static struct cfctrl_request_info *cfctrl_remove_req(struct cfctrl *ctrl,
 	}
 	p = NULL;
 out:
-	spin_unlock(&ctrl->info_list_lock);
 	return p;
 }
 
@@ -179,10 +192,6 @@ void cfctrl_enum_req(struct cflayer *layer, u8 physlinkid)
 	cfpkt_addbdy(pkt, physlinkid);
 	ret =
 	    cfctrl->serv.layer.dn->transmit(cfctrl->serv.layer.dn, pkt);
-	if (ret < 0) {
-		pr_err("Could not transmit enum message\n");
-		cfpkt_destroy(pkt);
-	}
 }
 
 int cfctrl_linkup_request(struct cflayer *layer,
@@ -196,14 +205,23 @@ int cfctrl_linkup_request(struct cflayer *layer,
 	struct cfctrl_request_info *req;
 	int ret;
 	char utility_name[16];
-	struct cfpkt *pkt = cfpkt_create(CFPKT_CTRL_PKT_LEN);
+	struct cfpkt *pkt;
+
+	if (cfctrl_cancel_req(layer, user_layer) > 0) {
+		/* Slight Paranoia, check if already connecting */
+		pr_err("Duplicate connect request for same client\n");
+		WARN_ON(1);
+		return -EALREADY;
+	}
+
+	pkt = cfpkt_create(CFPKT_CTRL_PKT_LEN);
 	if (!pkt) {
 		pr_warn("Out of memory\n");
 		return -ENOMEM;
 	}
 	cfpkt_addbdy(pkt, CFCTRL_CMD_LINK_SETUP);
-	cfpkt_addbdy(pkt, (param->chtype << 4) + param->linktype);
-	cfpkt_addbdy(pkt, (param->priority << 3) + param->phyid);
+	cfpkt_addbdy(pkt, (param->chtype << 4) | param->linktype);
+	cfpkt_addbdy(pkt, (param->priority << 3) | param->phyid);
 	cfpkt_addbdy(pkt, param->endpoint & 0x03);
 
 	switch (param->linktype) {
@@ -266,9 +284,13 @@ int cfctrl_linkup_request(struct cflayer *layer,
 	ret =
 	    cfctrl->serv.layer.dn->transmit(cfctrl->serv.layer.dn, pkt);
 	if (ret < 0) {
-		pr_err("Could not transmit linksetup request\n");
-		cfpkt_destroy(pkt);
-		return -ENODEV;
+		int count;
+
+		count = cfctrl_cancel_req(&cfctrl->serv.layer,
+						user_layer);
+		if (count != 1)
+			pr_err("Could not remove request (%d)", count);
+			return -ENODEV;
 	}
 	return 0;
 }
@@ -288,28 +310,29 @@ int cfctrl_linkdown_req(struct cflayer *layer, u8 channelid,
 	init_info(cfpkt_info(pkt), cfctrl);
 	ret =
 	    cfctrl->serv.layer.dn->transmit(cfctrl->serv.layer.dn, pkt);
-	if (ret < 0) {
-		pr_err("Could not transmit link-down request\n");
-		cfpkt_destroy(pkt);
-	}
+#ifndef CAIF_NO_LOOP
+	cfctrl->loop_linkused[channelid] = 0;
+#endif
 	return ret;
 }
 
-void cfctrl_cancel_req(struct cflayer *layr, struct cflayer *adap_layer)
+int cfctrl_cancel_req(struct cflayer *layr, struct cflayer *adap_layer)
 {
 	struct cfctrl_request_info *p, *tmp;
 	struct cfctrl *ctrl = container_obj(layr);
-	spin_lock(&ctrl->info_list_lock);
+	int found = 0;
+	spin_lock_bh(&ctrl->info_list_lock);
 
 	list_for_each_entry_safe(p, tmp, &ctrl->list, list) {
 		if (p->client_layer == adap_layer) {
-			pr_debug("cancel req :%d\n", p->sequence_no);
 			list_del(&p->list);
 			kfree(p);
+			found++;
 		}
 	}
 
-	spin_unlock(&ctrl->info_list_lock);
+	spin_unlock_bh(&ctrl->info_list_lock);
+	return found;
 }
 
 static int cfctrl_recv(struct cflayer *layer, struct cfpkt *pkt)
@@ -461,6 +484,7 @@ static int cfctrl_recv(struct cflayer *layer, struct cfpkt *pkt)
 
 			rsp.cmd = cmd;
 			rsp.param = linkparam;
+			spin_lock_bh(&cfctrl->info_list_lock);
 			req = cfctrl_remove_req(cfctrl, &rsp);
 
 			if (CFCTRL_ERR_BIT == (CFCTRL_ERR_BIT & cmdrsp) ||
@@ -480,6 +504,8 @@ static int cfctrl_recv(struct cflayer *layer, struct cfpkt *pkt)
 
 			if (req != NULL)
 				kfree(req);
+
+			spin_unlock_bh(&cfctrl->info_list_lock);
 		}
 		break;
 	case CFCTRL_CMD_LINK_DESTROY:
@@ -523,12 +549,29 @@ static void cfctrl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 	switch (ctrl) {
 	case _CAIF_CTRLCMD_PHYIF_FLOW_OFF_IND:
 	case CAIF_CTRLCMD_FLOW_OFF_IND:
-		spin_lock(&this->info_list_lock);
+		spin_lock_bh(&this->info_list_lock);
 		if (!list_empty(&this->list)) {
 			pr_debug("Received flow off in control layer\n");
 		}
-		spin_unlock(&this->info_list_lock);
+		spin_unlock_bh(&this->info_list_lock);
 		break;
+	case _CAIF_CTRLCMD_PHYIF_DOWN_IND: {
+		struct cfctrl_request_info *p, *tmp;
+
+		/* Find all connect request and report failure */
+		spin_lock_bh(&this->info_list_lock);
+		list_for_each_entry_safe(p, tmp, &this->list, list) {
+			if (p->param.phyid == phyid) {
+				list_del(&p->list);
+				p->client_layer->ctrlcmd(p->client_layer,
+						CAIF_CTRLCMD_INIT_FAIL_RSP,
+						phyid);
+				kfree(p);
+			}
+		}
+		spin_unlock_bh(&this->info_list_lock);
+		break;
+	}
 	default:
 		break;
 	}
@@ -538,27 +581,33 @@ static void cfctrl_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 static int handle_loop(struct cfctrl *ctrl, int cmd, struct cfpkt *pkt)
 {
 	static int last_linkid;
+	static int dec;
 	u8 linkid, linktype, tmp;
 	switch (cmd) {
 	case CFCTRL_CMD_LINK_SETUP:
-		spin_lock(&ctrl->loop_linkid_lock);
-		for (linkid = last_linkid + 1; linkid < 255; linkid++)
-			if (!ctrl->loop_linkused[linkid])
-				goto found;
+		spin_lock_bh(&ctrl->loop_linkid_lock);
+		if (!dec) {
+			for (linkid = last_linkid + 1; linkid < 255; linkid++)
+				if (!ctrl->loop_linkused[linkid])
+					goto found;
+		}
+		dec = 1;
 		for (linkid = last_linkid - 1; linkid > 0; linkid--)
 			if (!ctrl->loop_linkused[linkid])
 				goto found;
-		spin_unlock(&ctrl->loop_linkid_lock);
-		pr_err("Out of link-ids\n");
-		return -EINVAL;
+		spin_unlock_bh(&ctrl->loop_linkid_lock);
+
 found:
+		if (linkid < 10)
+			dec = 0;
+
 		if (!ctrl->loop_linkused[linkid])
 			ctrl->loop_linkused[linkid] = 1;
 
 		last_linkid = linkid;
 
 		cfpkt_add_trail(pkt, &linkid, 1);
-		spin_unlock(&ctrl->loop_linkid_lock);
+		spin_unlock_bh(&ctrl->loop_linkid_lock);
 		cfpkt_peek_head(pkt, &linktype, 1);
 		if (linktype ==  CFCTRL_SRV_UTIL) {
 			tmp = 0x01;
@@ -568,10 +617,10 @@ found:
 		break;
 
 	case CFCTRL_CMD_LINK_DESTROY:
-		spin_lock(&ctrl->loop_linkid_lock);
+		spin_lock_bh(&ctrl->loop_linkid_lock);
 		cfpkt_peek_head(pkt, &linkid, 1);
 		ctrl->loop_linkused[linkid] = 0;
-		spin_unlock(&ctrl->loop_linkid_lock);
+		spin_unlock_bh(&ctrl->loop_linkid_lock);
 		break;
 	default:
 		break;
diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
index 4f4f756..04204b2 100644
--- a/net/caif/cffrml.c
+++ b/net/caif/cffrml.c
@@ -33,7 +33,6 @@ static void cffrml_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 static u32 cffrml_rcv_error;
 static u32 cffrml_rcv_checsum_error;
 struct cflayer *cffrml_create(u16 phyid, bool use_fcs)
-
 {
 	struct cffrml *this = kmalloc(sizeof(struct cffrml), GFP_ATOMIC);
 	if (!this) {
@@ -128,6 +127,13 @@ static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt)
 		cfpkt_destroy(pkt);
 		return -EPROTO;
 	}
+
+	if (layr->up == NULL) {
+		pr_err("Layr up is missing!\n");
+		cfpkt_destroy(pkt);
+		return -EINVAL;
+	}
+
 	return layr->up->receive(layr->up, pkt);
 }
 
@@ -150,15 +156,22 @@ static int cffrml_transmit(struct cflayer *layr, struct cfpkt *pkt)
 	cfpkt_info(pkt)->hdr_len += 2;
 	if (cfpkt_erroneous(pkt)) {
 		pr_err("Packet is erroneous!\n");
+		cfpkt_destroy(pkt);
 		return -EPROTO;
 	}
+
+	if (layr->dn == NULL) {
+		cfpkt_destroy(pkt);
+		return -ENODEV;
+
+	}
 	return layr->dn->transmit(layr->dn, pkt);
 }
 
 static void cffrml_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 					int phyid)
 {
-	if (layr->up->ctrlcmd)
+	if (layr->up && layr->up->ctrlcmd)
 		layr->up->ctrlcmd(layr->up, ctrl, layr->id);
 }
 
diff --git a/net/caif/cfveil.c b/net/caif/cfveil.c
index 1a588cd..3ec83fb 100644
--- a/net/caif/cfveil.c
+++ b/net/caif/cfveil.c
@@ -82,13 +82,14 @@ static int cfvei_transmit(struct cflayer *layr, struct cfpkt *pkt)
 	int ret;
 	struct cfsrvl *service = container_obj(layr);
 	if (!cfsrvl_ready(service, &ret))
-		return ret;
+		goto err;
 	caif_assert(layr->dn != NULL);
 	caif_assert(layr->dn->transmit != NULL);
 
 	if (cfpkt_add_head(pkt, &tmp, 1) < 0) {
 		pr_err("Packet is erroneous!\n");
-		return -EPROTO;
+		ret = -EPROTO;
+		goto err;
 	}
 
 	/* Add info-> for MUX-layer to route the packet out. */
@@ -97,4 +98,7 @@ static int cfvei_transmit(struct cflayer *layr, struct cfpkt *pkt)
 	info->hdr_len = 1;
 	info->dev_info = &service->dev_info;
 	return layr->dn->transmit(layr->dn, pkt);
+err:
+	cfpkt_destroy(pkt);
+	return ret;
 }
-- 
1.7.1


^ permalink raw reply related

* [net-next-2.6 00/10] caif: rcu, refactoring and bugfixes
From: Sjur Brændeland @ 2011-05-13 12:43 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: Sjur Brændeland

This patch-set introduces RCU in the CAIF stack and
fixes problems found when removing CAIF Link layer during traffic.

The pattern used for RCU is mostly this:
	rcu_read_lock();
	p = get();
	hold(p);
	rcu_read_unlock();
	use(p);
	put(p);

And when freeing:
	synchronize_rcu();
	wait_refcnt(p);
	kfree(p);

Sjur Brændeland (10):
  caif: Use rcu_read_lock in CAIF mux layer.
  caif: Use RCU instead of spin-lock in caif_dev.c
  caif: Use RCU and lists in cfcnfg.c for managing caif link layers
  caif: Add ref-count to framing layer
  caif: Move refcount from service layer to sock and dev.
  caif: Protected in-flight packets using dev or sock refcont.
  caif: prepare support for namespaces
  caif: Handle dev_queue_xmit errors.
  caif: Bugfix debugfs directory name must be unique.
  caif: remove unesesarry exports

 include/net/caif/caif_dev.h |   43 +++--
 include/net/caif/cfcnfg.h   |   71 ++-----
 include/net/caif/cfctrl.h   |    3 +-
 include/net/caif/cffrml.h   |    7 +-
 include/net/caif/cfpkt.h    |    1 -
 include/net/caif/cfsrvl.h   |   29 ++--
 net/caif/Makefile           |    2 +-
 net/caif/caif_config_util.c |   99 ---------
 net/caif/caif_dev.c         |  349 +++++++++++++++++-------------
 net/caif/caif_socket.c      |   71 +++++--
 net/caif/cfcnfg.c           |  505 ++++++++++++++++++++++++++++---------------
 net/caif/cfctrl.c           |  121 ++++++++---
 net/caif/cffrml.c           |   54 +++++-
 net/caif/cfmuxl.c           |  119 +++++++----
 net/caif/cfpkt_skbuff.c     |   27 +--
 net/caif/cfrfml.c           |    4 +-
 net/caif/cfsrvl.c           |   35 +++-
 net/caif/cfveil.c           |    8 +-
 net/caif/chnl_net.c         |   45 +++-
 19 files changed, 929 insertions(+), 664 deletions(-)
 delete mode 100644 net/caif/caif_config_util.c


^ permalink raw reply


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