Netdev List
 help / color / mirror / Atom feed
* [RFC 2/3] RDMA/cma: Add support for netlink statistics export
From: Roland Dreier @ 2011-05-13 16:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1305303525-11113-1-git-send-email-roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

From: Nir Muchtar <nirm-smomgflXvOZWk0Htik3J/w@public.gmane.org>

[Dave please do not apply even if this ends up in netdev patchwork!]

Add callbacks and data types for statistics export of all current
devices/ids.  The schema for RDMA CM is a series of netlink messages.
Each one contains an rdma_cm_stat struct.  Additionally, two netlink
attributes are created for the addresses for each message (if
applicable).

Their types used are:
RDMA_NL_RDMA_CM_ATTR_SRC_ADDR (The source address for this ID)
RDMA_NL_RDMA_CM_ATTR_DST_ADDR (The destination address for this ID)
sockaddr_* structs are encapsulated within these attributes.

In other words, every transaction contains a series of messages like:

-------message 1-------
struct rdma_cm_id_stats {
       __u32 qp_num;
       __u32 bound_dev_if;
       __u32 port_space;
       __s32 pid;
       __u8 cm_state;
       __u8 node_type;
       __u8 port_num;
       __u8 reserved;
}
RDMA_NL_RDMA_CM_ATTR_SRC_ADDR attribute - contains the source address
RDMA_NL_RDMA_CM_ATTR_DST_ADDR attribute - contains the destination address
-------end 1-------
-------message 2-------
struct rdma_cm_id_stats
RDMA_NL_RDMA_CM_ATTR_SRC_ADDR attribute
RDMA_NL_RDMA_CM_ATTR_DST_ADDR attribute
-------end 2-------

NOT-Signed-off-by: Nir Muchtar <nirm-smomgflXvOZWk0Htik3J/w@public.gmane.org>
NOT-Signed-off-by: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
 drivers/infiniband/core/cma.c |   98 +++++++++++++++++++++++++++++++++++++++++
 include/rdma/rdma_netlink.h   |   28 ++++++++++++
 2 files changed, 126 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 05b55e4..d4701a8 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -47,6 +47,7 @@
 
 #include <rdma/rdma_cm.h>
 #include <rdma/rdma_cm_ib.h>
+#include <rdma/rdma_netlink.h>
 #include <rdma/ib_cache.h>
 #include <rdma/ib_cm.h>
 #include <rdma/ib_sa.h>
@@ -3247,6 +3248,98 @@ static void cma_remove_one(struct ib_device *device)
 	kfree(cma_dev);
 }
 
+static int cma_get_id_stats(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	struct nlmsghdr *nlh;
+	struct rdma_cm_id_stats *id_stats;
+	struct rdma_id_private *id_priv;
+	struct rdma_cm_id *id = NULL;
+	struct cma_device *cma_dev;
+	int i_dev = 0, i_id = 0;
+
+	/*
+	 * We export all of the IDs as a sequence of messages.  Each
+	 * ID gets its own netlink message.
+	 */
+	mutex_lock(&lock);
+
+	list_for_each_entry(cma_dev, &dev_list, list) {
+		if (i_dev < cb->args[0]) {
+			i_dev++;
+			continue;
+		}
+
+		i_id = 0;
+		list_for_each_entry(id_priv, &cma_dev->id_list, list) {
+			if (i_id < cb->args[1]) {
+				i_id++;
+				continue;
+			}
+
+			id_stats = ibnl_put_msg(skb, &nlh, cb->nlh->nlmsg_seq,
+						sizeof *id_stats, RDMA_NL_RDMA_CM,
+						RDMA_NL_RDMA_CM_ID_STATS);
+			if (!id_stats)
+				goto out;
+
+			memset(id_stats, 0, sizeof *id_stats);
+			id = &id_priv->id;
+			id_stats->node_type = id->route.addr.dev_addr.dev_type;
+			id_stats->port_num = id->port_num;
+			id_stats->bound_dev_if =
+				id->route.addr.dev_addr.bound_dev_if;
+
+			if (id->route.addr.src_addr.ss_family == AF_INET) {
+				if (ibnl_put_attr(skb, nlh,
+						  sizeof(struct sockaddr_in),
+						  &id->route.addr.src_addr,
+						  RDMA_NL_RDMA_CM_ATTR_SRC_ADDR)) {
+					goto out;
+				}
+				if (ibnl_put_attr(skb, nlh,
+						  sizeof(struct sockaddr_in),
+						  &id->route.addr.dst_addr,
+						  RDMA_NL_RDMA_CM_ATTR_DST_ADDR)) {
+					goto out;
+				}
+			} else if (id->route.addr.src_addr.ss_family == AF_INET6) {
+				if (ibnl_put_attr(skb, nlh,
+						  sizeof(struct sockaddr_in6),
+						  &id->route.addr.src_addr,
+						  RDMA_NL_RDMA_CM_ATTR_SRC_ADDR)) {
+					goto out;
+				}
+				if (ibnl_put_attr(skb, nlh,
+						  sizeof(struct sockaddr_in6),
+						  &id->route.addr.dst_addr,
+						  RDMA_NL_RDMA_CM_ATTR_DST_ADDR)) {
+					goto out;
+				}
+			}
+
+			id_stats->port_space = id->ps;
+			id_stats->cm_state = id_priv->state;
+			id_stats->qp_num = id_priv->qp_num;
+
+			i_id++;
+		}
+
+		cb->args[1] = 0;
+		i_dev++;
+	}
+
+out:
+	mutex_unlock(&lock);
+	cb->args[0] = i_dev;
+	cb->args[1] = i_id;
+
+	return skb->len;
+}
+
+static const struct ibnl_client_cbs cma_cb_table[] = {
+	[RDMA_NL_RDMA_CM_ID_STATS] = { .dump = cma_get_id_stats },
+};
+
 static int __init cma_init(void)
 {
 	int ret;
@@ -3262,6 +3355,10 @@ static int __init cma_init(void)
 	ret = ib_register_client(&cma_client);
 	if (ret)
 		goto err;
+
+	if (ibnl_add_client(RDMA_NL_RDMA_CM, RDMA_NL_RDMA_CM_NUM_OPS, cma_cb_table))
+		printk(KERN_WARNING "RDMA CMA: failed to add netlink callback\n");
+
 	return 0;
 
 err:
@@ -3274,6 +3371,7 @@ err:
 
 static void __exit cma_cleanup(void)
 {
+	ibnl_remove_client(RDMA_NL_RDMA_CM);
 	ib_unregister_client(&cma_client);
 	unregister_netdevice_notifier(&cma_nb);
 	rdma_addr_unregister_client(&addr_client);
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index c983a19..fa318af 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -1,10 +1,38 @@
 #ifndef _RDMA_NETLINK_H
 #define _RDMA_NETLINK_H
 
+#include <linux/types.h>
+
+enum {
+	RDMA_NL_RDMA_CM = 1
+};
+
 #define RDMA_NL_GET_CLIENT(type) ((type & (((1 << 6) - 1) << 10)) >> 10)
 #define RDMA_NL_GET_OP(type) (type & ((1 << 10) - 1))
 #define RDMA_NL_GET_TYPE(client, op) ((client << 10) + op)
 
+enum {
+	RDMA_NL_RDMA_CM_ID_STATS = 0,
+	RDMA_NL_RDMA_CM_NUM_OPS
+};
+
+enum {
+	RDMA_NL_RDMA_CM_ATTR_SRC_ADDR = 1,
+	RDMA_NL_RDMA_CM_ATTR_DST_ADDR,
+	RDMA_NL_RDMA_CM_NUM_ATTR,
+};
+
+struct rdma_cm_id_stats {
+	__u32	qp_num;
+	__u32	bound_dev_if;
+	__u32	port_space;
+	__s32	pid;
+	__u8	cm_state;
+	__u8	node_type;
+	__u8	port_num;
+	__u8	reserved;
+};
+
 #ifdef __KERNEL__
 
 #include <linux/netlink.h>
-- 
1.7.4.1

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

^ permalink raw reply related

* [RFC 0/3] RDMA: Add netlink infrastructure
From: Roland Dreier @ 2011-05-13 16:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA

From: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>

Hi,

Here are the highlights of a patch series adding support for getting
information on active RDMA connections via netlink.  I would really
appreciate it if netlink experts could take a look and make sure we
are doing this the right way and using a suitably extensible
structure.

I'm by no means particularly knowledgeable about netlink, so although
the RDMA side of things looks OK to me, I hope we can get some level
of review of the netlink side of things; the changelog for patch 2/3
tries to explain the messages we're using.

The full series based on 2.6.39-rc7 is also available at:

    git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git netlink

Thanks!
  Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

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

On Fri, May 13, 2011 at 12:11 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, May 13, 2011 at 7:56 AM, Andrew Lutomirski <luto@mit.edu> wrote:
>>
>> So what I really want is a fancy version of git bisect that makes no
>> assumptions about the relationship of good and bad commits in the
>> graph and just finds me a commit that is bad but for which all parents
>> are good or vice versa.
>
> Ehh. That's the "non-fancy" way of testing, I'm afraid: if you cannot
> make assumption about the relationship between good and bad commits,
> then you have to test _every_ commit.
>
> So yes, bisection has its problems. But they really do come from the
> fact that it's very efficient. When you have (on average) about ten
> thousand commits between releases, you have to make assumptions about
> the relationships. But once you do that, the efficiency also results
> in a certain fragility.
>
> Think of it as a compression method: it generates the smallest
> possible set of test points for you. But it's a "lossy" compression -
> you don't test everything. And it's extreme: it boils down 10k commit
> events to about 13 bisection events. If anything goes wrong (like the
> bug not being entirely repeatable, or the bug comes and goes), it will
> give the wrong answer.
>
> The good news is that _usually_ it works really well. And when the
> choice is between "works really well for 10k commits but can have
> problems" and "you need to test all 10k commits", the "can have
> problems" part turns out to be a pretty small downside ;)

In conclusion, I found the problem.  It's a clusterfuck and I think
there's no way that any bisection tool under any sane assumptions
could have found it.  Patch coming in a couple seconds b/c I think it
needs to go in to 2.6.39.

--Andy

>
>                                Linus
>

^ permalink raw reply

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

On Fri, May 13, 2011 at 7:56 AM, Andrew Lutomirski <luto@mit.edu> wrote:
>
> So what I really want is a fancy version of git bisect that makes no
> assumptions about the relationship of good and bad commits in the
> graph and just finds me a commit that is bad but for which all parents
> are good or vice versa.

Ehh. That's the "non-fancy" way of testing, I'm afraid: if you cannot
make assumption about the relationship between good and bad commits,
then you have to test _every_ commit.

So yes, bisection has its problems. But they really do come from the
fact that it's very efficient. When you have (on average) about ten
thousand commits between releases, you have to make assumptions about
the relationships. But once you do that, the efficiency also results
in a certain fragility.

Think of it as a compression method: it generates the smallest
possible set of test points for you. But it's a "lossy" compression -
you don't test everything. And it's extreme: it boils down 10k commit
events to about 13 bisection events. If anything goes wrong (like the
bug not being entirely repeatable, or the bug comes and goes), it will
give the wrong answer.

The good news is that _usually_ it works really well. And when the
choice is between "works really well for 10k commits but can have
problems" and "you need to test all 10k commits", the "can have
problems" part turns out to be a pretty small downside ;)

                                Linus

^ permalink raw reply

* dl2k/stmmac patches (was Re: [PATCH 2/2] net/dl2k: Don't reconfigure link @100Mbps when disabling autoneg @1Gbps)
From: David Decotigny @ 2011-05-13 15:54 UTC (permalink / raw)
  To: netdev
  Cc: Florian Weimer, Giuseppe Cavallaro, David S. Miller, Joe Perches,
	Stanislaw Gruszka, linux-kernel
In-Reply-To: <82y62dk1qz.fsf@mid.bfk.de>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Hi all,

An update on the patch series I posted earlier (dl2k/stmmac autoneg
minor changes: see my posts on 05/09/11 17:19 PST)...

I'd suggest dropping the patch concerning dl2k, as I don't have any way
to claim it's any good at all (though I'm pretty confident it doesn't
make things any worse).

However, for the other patch of the same series (stmmac), please
consider for inclusion the new version prepared by Giuseppe instead of
my initial patch, for his version readily integrates my patch and has
been tested on real hardware:
  stmmac: don't go through ethtool to start auto-negotiation
  stmmac: fix autoneg in set_pauseparam

Regards,
Thank you!

On 05/11/11 02:47, Florian Weimer wrote:
> * David Decotigny:
> 
>> Tested: module compiling, NOT tested on real hardware.
> 
> To my knowledge, dl2k is broken.  Some sort of synchronization
> primitives are missing.  Under load, the NIC's notion of ring buffer
> status diverges from the host's view. 8-(
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk3NVCYACgkQld7vhusVrCHUEACggeKyCmoEHy9AzYec/aW8cDjU
GyAAoIiESxUAFWuKBmCOA31X/V0fvC+Y
=6hXY
-----END PGP SIGNATURE-----

^ permalink raw reply

* Re: [PATCH 03/16] add driver for C64x+ ethernet driver
From: Ben Hutchings @ 2011-05-13 15:30 UTC (permalink / raw)
  To: Mark Salter; +Cc: linux-kernel, netdev
In-Reply-To: <1305144843-5058-4-git-send-email-msalter@redhat.com>

Please send network drivers to netdev for review.

I haven't looked at this thoroughly, but I noticed a few things:

On Wed, 2011-05-11 at 16:13 -0400, Mark Salter wrote:
> From: Aurelien Jacquiot <a-jacquiot@ti.com>
> 
> This patch adds a network driver to support the ethernet interface found on
> several Texas Instruments C64X+ based System on Chips. In particular, this
> driver has been tested on the TMS320C6455, TMS320C6457, TMS320C6472, and
> TMS320C6474 parts.
[...]
> --- /dev/null
> +++ b/drivers/net/c6x_gemac.c
[...]
> +static int __init get_mac_addr_from_cmdline(char *str)
> +{
> +	const char *start = (const char *) str;
> +	const char *end;
> +	int count;
> +
> +	for (count = 0; count < 6; count++) {
> +		config.enetaddr[count] = _hex_strtoul(start, &end);
> +		if (end == start)
> +			return 0;
> +		if ((*end != ':') && (count != 5))
> +			return 0;
> +		start = end + 1;
> +	}
> +	return 1;
> +}
> +
> +__setup("emac_addr=", get_mac_addr_from_cmdline);
> +
> +static int __init set_emac_shared(char *str)
> +{
> +	emac_shared = 1;
> +	return 1;
> +}
> +
> +__setup("emac_shared", set_emac_shared);

These parameters should be provided by platform data.

> +/*
> + * Get the device statistic
> + */
> +static struct net_device_stats *emac_get_stats(struct net_device *dev)
> +{
> +	struct emac_private *ep = netdev_priv(dev);
> +	unsigned int reg;
> +	unsigned int dummy;
> +	int i;
> +
> +	emac_set_stat(dev->stats.multicast,	    EMAC_RXMCASTFRAMES);
> +	emac_set_stat(dev->stats.collisions,	    EMAC_TXCOLLISION);
> +	emac_set_stat(dev->stats.rx_length_errors,  EMAC_RXOVERSIZED);
> +	emac_set_stat(dev->stats.rx_length_errors,  EMAC_RXUNDERSIZED);
> +	emac_set_stat(dev->stats.rx_crc_errors,     EMAC_RXCRCERRORS);
> +	emac_set_stat(dev->stats.rx_frame_errors,   EMAC_RXALIGNCODEERRORS);
> +	emac_set_stat(dev->stats.tx_carrier_errors, EMAC_TXCARRIERSLOSS);
> +	emac_set_stat(dev->stats.tx_fifo_errors,    EMAC_TXUNDERRUN);
> +	emac_set_stat(dev->stats.tx_window_errors,  EMAC_TXLATECOLL);
> +
> +	/* ACK statistic by write-to-decrement */
> +	reg = EMAC_RXGOODFRAMES;
> +	for (i = 0; i < EMAC_NUM_STATREGS; i++) {
> +		dummy = emac_get_reg(reg);
> +		emac_set_reg(reg, dummy);
> +		reg += 4;
> +	}

This looks wrong.  Surely you should be decrementing stats by the values
that were read above, not the 'dummy' value read later.

> +	return &dev->stats;
> +}
> +
> +/*
> + * Receive packets
> + */
> +static int emac_rx(struct net_device *dev, struct emac_desc *desc_ack)
> +{
[...]
> +			/* Give back old sk_buff */
> +			netif_rx(skb_old);
> +			dev->last_rx = jiffies;

No need to set last_rx; the networking core does it.

> +			/* Fill statistic */
> +			dev->stats.rx_packets++;
> +			dev->stats.rx_bytes += pkt_len;
> +		} else {
> +			printk(KERN_WARNING "%s: Memory squeeze, dropping packet.\n", dev->name);

Should be rate-limited or controlled by message-level (see netif_info()
and related macros in netdevice.h).

[...]
> +static int emac_start_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct emac_private	  *ep =  netdev_priv(dev);
> +	volatile struct emac_desc *desc;
> +	volatile struct emac_desc *prev_desc = NULL;
> +	ushort pkt_len = skb->len;
> +	unsigned long flags;
> +
> +	if (ep->tx_full) {
> +		printk(KERN_WARNING "%s: tx queue full\n", dev->name);

Definitely don't log this by default.

> +		dev->stats.tx_dropped++;

No, the packet is not dropped in this case.

> +		return NETDEV_TX_BUSY;
> +	}
> +
> +	/* Pad short frame */
> +	if (unlikely(pkt_len < ETH_ZLEN)) {
> +		if (skb_padto(skb, ETH_ZLEN)) {
> +			netif_wake_queue(dev);

The queue is already awake!

And in this case you *are* dropping the packet, so you could increment
tx_dropped.

> +			return NETDEV_TX_OK;
> +		}
> +		pkt_len = ETH_ZLEN;
> +	}
[...]
> +static void emac_handle_host_interrupt(struct net_device *dev)
> +{
> +	struct emac_private *ep = netdev_priv(dev);
> +	unsigned long	     status;
> +
> +	/* Handle host error */
> +	status = emac_get_reg(EMAC_MACSTATUS);
> +
> +	/* Check if the problem occurs on our channel when we are slave */
> +	if ((ep->slave)
> +	    && (((status & EMAC_M_RXERRCH) >> EMAC_S_RXERRCH) != IDX_TO_CHAN(emac_idx))
> +	    && (((status & EMAC_M_TXERRCH) >> EMAC_S_TXERRCH) != IDX_TO_CHAN(emac_idx)))
> +		return;
> +
> +	if ((status & EMAC_M_RXERRCODE) == (EMAC_V_OWNERSHIP <<  EMAC_S_RXERRCODE)) {
> +		printk(KERN_ERR "%s: EMAC rx ring full\n", dev->name);

Should be rate-limited or controlled by message-level.

> +		dev->stats.rx_fifo_errors++;
> +	} else
> +		printk(KERN_ERR "%s: EMAC fatal host error 0x%lx\n",
> +		       dev->name, status);
> +
> +	DPRINTK(KERN_ERR "%s: Error head=%p desc=%p dirty=%p skb_tx_dirty=%ld count=%ld\n",
> +		dev->name, ep->head_tx, ep->cur_tx,
> +		ep->dirty_tx, ep->skb_tx_dirty, ep->count_tx);
> +
> +	if (!ep->slave) {
> +		/* Reconfigure the device */
> +		ep->fatal_error = 1;
> +		emac_reconfigure_device(dev);
> +	}
> +}
[...]
> +#ifdef EMAC_HAS_ALE_SUPPORT
[...]
> +static void emac_set_rx_mode(struct net_device *dev)
[...]
> +		/* Set mcast from a list */
> +		for (i = 0; i < dev->mc_count; i++, dmi = dmi->next) {
> +			u8 *p_dmi = dmi->dmi_addr;

This is the old multicast list format; the code won't even compile now.

It needs to be updated like the !EMAC_HAS_ALE_SUPPORT version has been.

[...]
> +#ifdef EMAC_TIMER_TICK_MDIO
> +/*
> + * This function should be called for each device in the system on a
> + * periodic basis of 100mS (10 times a second). It is used to check the
> + * status of the EMAC and MDIO device.
> + */
> +static void emac_timer_tick(unsigned long data)

That seems excessive!  It might be worth checking whether this takes
significant CPU time, and increasing the period when all links are in a
steady state.

[...]
> +static const struct ethtool_ops gemac_ethtool_ops = {
> +};

We (that is, netdev regulars) should make more of the default ethtool
operations work without dev->ethtool_ops set, so you don't have to do
this!

[...]
> +#ifdef EMAC_ARCH_HAS_MAC_ADDR
> +	/* SoC or board hw has MAC address */
> +	if (config.enetaddr[0] == 0 && config.enetaddr[1] == 0 &&
> +	    config.enetaddr[2] == 0 && config.enetaddr[3] == 0 &&
> +	    config.enetaddr[4] == 0 && config.enetaddr[5] == 0) {
[...]

is_zero_ether_addr(config.enetaddr)

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH] ethtool: FLOW_EXT may confuse drivers
From: Ben Hutchings @ 2011-05-13 15:01 UTC (permalink / raw)
  To: Sebastian.Poehn; +Cc: netdev
In-Reply-To: <OFDA6F039C.B25E3B43-ON8525788F.004EA678-8525788F.004EA67B@BeldenCDT.com>

Please send ethtool patches directly to me as well as to netdev.

On Fri, 2011-05-13 at 10:19 -0400, Sebastian.Poehn@Belden.com wrote:
> The FLOW_EXT bit must be masked out. Otherwise if e.g. vlan is set a
> driver receiving the ntuple may not detect the flow_type correctly!
> 
> Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>
> 
> diff --git a/ethtool.c b/ethtool.c
> index 34fe107..0b7ec05 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -3134,6 +3134,9 @@ static int flow_spec_to_ntuple(struct ethtool_rx_flow_spec *fsp,
>                  (u64)ntohl(~fsp->m_ext.data[1]);
>          }
>      }
> +    
> +    /*Mask out the extended bit, because ntuple does not know it!*/
> +    ntuple->flow_type &= ~FLOW_EXT;
>  
>      return 0;
>  }
> 
> DISCLAIMER:
> 
> Privileged and/or Confidential information may be contained in this
> message. If you are not the addressee of this message, you may not
> copy, use or deliver this message to anyone. In such event, you
> should destroy the message and kindly notify the sender by reply
> e-mail. It is understood that opinions or conclusions that do not
> relate to the official business of the company are neither given
> nor endorsed by the company.

I am not the addressee, so I won't apply this (even though it looks
right).  Please get rid of this disclaimer on messages to public mailing
lists.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

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

On Fri, May 13, 2011 at 9:38 AM, Andrew Lutomirski <luto@mit.edu> wrote:
> On Fri, May 13, 2011 at 4:20 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> 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/
>
> No, it was this:
>
> http://stackoverflow.com/questions/5638211/how-do-you-get-git-bisect-to-ignore-merged-branches
>

Sadly even that's not enough.  I finished the bisection (by
standard-ish techniques but with a bit of overriding of git bisect's
choices and occasional merging of newer versions of -linus to get
something that would boot) and it pointed to a commit that wasn't the
culprit.

The problem is that 2.6.39-rc7 is bad, 2.6.38 (and 2.6.38.{5,6}) is
good, but 2.6.38-rc5 is bad and fails identically to 2.6.39-rc7.  I
think that git bisect makes the assumption that ancestors of a good
commit are good, and that just isn't true for this bug.

So what I really want is a fancy version of git bisect that makes no
assumptions about the relationship of good and bad commits in the
graph and just finds me a commit that is bad but for which all parents
are good or vice versa.

I'm currently bisecting the other way to find the commit before 2.6.38
that fixed the bug, since there's presumably less churn there than in
the early bits of 2.6.39.

--Andy

^ permalink raw reply

* Re: question about UFO behavior for bridge device
From: Herbert Xu @ 2011-05-13 14:36 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Shan Wei, netdev
In-Reply-To: <1305292012.4065.441.camel@localhost>

On Fri, May 13, 2011 at 02:06:52PM +0100, Ben Hutchings wrote:
>
> > 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?
> 
> I assume that packet capturing is handled earlier in the transmit path.

Yes it is.  It's the same with TSO.

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

^ permalink raw reply

* [PATCH] ethtool: FLOW_EXT may confuse drivers
From: Sebastian.Poehn @ 2011-05-13 14:19 UTC (permalink / raw)
  To: netdev

The FLOW_EXT bit must be masked out. Otherwise if e.g. vlan is set a driver receiving the ntuple may not detect the flow_type correctly!

Signed-off-by: Sebastian Poehn <sebastian.poehn@belden.com>

diff --git a/ethtool.c b/ethtool.c
index 34fe107..0b7ec05 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -3134,6 +3134,9 @@ static int flow_spec_to_ntuple(struct ethtool_rx_flow_spec *fsp,
                 (u64)ntohl(~fsp->m_ext.data[1]);
         }
     }
+    
+    /*Mask out the extended bit, because ntuple does not know it!*/
+    ntuple->flow_type &= ~FLOW_EXT;
 
     return 0;
 }

DISCLAIMER:

Privileged and/or Confidential information may be contained in this
message. If you are not the addressee of this message, you may not
copy, use or deliver this message to anyone. In such event, you
should destroy the message and kindly notify the sender by reply
e-mail. It is understood that opinions or conclusions that do not
relate to the official business of the company are neither given
nor endorsed by the company.

Thank You.


^ permalink raw reply related

* [PATCH] drivers/isdn/hisax: Drop unused list
From: Julia Lawall @ 2011-05-13 14:15 UTC (permalink / raw)
  To: Karsten Keil; +Cc: kernel-janitors, netdev, linux-kernel

The file st5481_init.c locally defines and initializes the adapter_list
variable, but does not use it for anything.  Removing the list makes it
possible to remove the list field from the st5481_adapter data structure.
In the function probe_st5481, it also makes it possible to free the locally
allocated adapter value on an error exit.

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/isdn/hisax/st5481.h      |    1 -
 drivers/isdn/hisax/st5481_init.c |    6 +-----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/isdn/hisax/st5481.h b/drivers/isdn/hisax/st5481.h
index 64f78a8..b9054cb 100644
--- a/drivers/isdn/hisax/st5481.h
+++ b/drivers/isdn/hisax/st5481.h
@@ -377,7 +377,6 @@ struct st5481_bcs {
 };
 
 struct st5481_adapter {
-	struct list_head list;
 	int number_of_leds;
 	struct usb_device *usb_dev;
 	struct hisax_d_if hisax_d_if;
diff --git a/drivers/isdn/hisax/st5481_init.c b/drivers/isdn/hisax/st5481_init.c
index 1375123..9f7fd18 100644
--- a/drivers/isdn/hisax/st5481_init.c
+++ b/drivers/isdn/hisax/st5481_init.c
@@ -46,8 +46,6 @@ module_param(debug, int, 0);
 #endif
 int st5481_debug;
 
-static LIST_HEAD(adapter_list);
-
 /* ======================================================================
  * registration/deregistration with the USB layer
  */
@@ -86,7 +84,6 @@ static int probe_st5481(struct usb_interface *intf,
 		adapter->bcs[i].b_if.ifc.priv = &adapter->bcs[i];
 		adapter->bcs[i].b_if.ifc.l2l1 = st5481_b_l2l1;
 	}
-	list_add(&adapter->list, &adapter_list);
 
 	retval = st5481_setup_usb(adapter);
 	if (retval < 0)
@@ -125,6 +122,7 @@ static int probe_st5481(struct usb_interface *intf,
  err_usb:
 	st5481_release_usb(adapter);
  err:
+	kfree(adapter);
 	return -EIO;
 }
 
@@ -142,8 +140,6 @@ static void disconnect_st5481(struct usb_interface *intf)
 	if (!adapter)
 		return;
 	
-	list_del(&adapter->list);
-
 	st5481_stop(adapter);
 	st5481_release_b(&adapter->bcs[1]);
 	st5481_release_b(&adapter->bcs[0]);

^ permalink raw reply related

* Re: [PATCH 2/3] net/rfkill/core.c: Avoid leaving freed data in a list
From: Johannes Berg @ 2011-05-13 13:55 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, John W. Linville, David S. Miller,
	linux-wireless, netdev, linux-kernel
In-Reply-To: <1305294731-12127-2-git-send-email-julia@diku.dk>

On Fri, 2011-05-13 at 15:52 +0200, Julia Lawall wrote:
> The list_for_each_entry loop can fail, in which case the list element is
> not removed from the list rfkill_fds.  Since this list is not accessed by
> the loop, the addition of &data->list into the list is just moved after the
> loop.
> 
> The sematic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression E,E1,E2;
> identifier l;
> @@
> 
> *list_add(&E->l,E1);
> ... when != E1
>     when != list_del(&E->l)
>     when != list_del_init(&E->l)
>     when != E = E2
> *kfree(E);// </smpl>
> 
> Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> ---
> I have only verified that rfkill_fds is not accessed by the loop by
> inspecting the code.  If this analysis is not correct, the other solution
> would be to leave the list_add where it is and delete the element from the
> list explicitly.

Looks right to me, thanks!

johannes

^ permalink raw reply

* [PATCH 2/3] net/rfkill/core.c: Avoid leaving freed data in a list
From: Julia Lawall @ 2011-05-13 13:52 UTC (permalink / raw)
  To: Johannes Berg
  Cc: kernel-janitors, John W. Linville, David S. Miller,
	linux-wireless, netdev, linux-kernel

The list_for_each_entry loop can fail, in which case the list element is
not removed from the list rfkill_fds.  Since this list is not accessed by
the loop, the addition of &data->list into the list is just moved after the
loop.

The sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression E,E1,E2;
identifier l;
@@

*list_add(&E->l,E1);
... when != E1
    when != list_del(&E->l)
    when != list_del_init(&E->l)
    when != E = E2
*kfree(E);// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
I have only verified that rfkill_fds is not accessed by the loop by
inspecting the code.  If this analysis is not correct, the other solution
would be to leave the list_add where it is and delete the element from the
list explicitly.

 net/rfkill/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 0198191..be90640 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -1024,7 +1024,6 @@ static int rfkill_fop_open(struct inode *inode, struct file *file)
 	 * start getting events from elsewhere but hold mtx to get
 	 * startup events added first
 	 */
-	list_add(&data->list, &rfkill_fds);
 
 	list_for_each_entry(rfkill, &rfkill_list, node) {
 		ev = kzalloc(sizeof(*ev), GFP_KERNEL);
@@ -1033,6 +1032,7 @@ static int rfkill_fop_open(struct inode *inode, struct file *file)
 		rfkill_fill_event(&ev->ev, rfkill, RFKILL_OP_ADD);
 		list_add_tail(&ev->list, &data->events);
 	}
+	list_add(&data->list, &rfkill_fds);
 	mutex_unlock(&data->mtx);
 	mutex_unlock(&rfkill_global_mutex);
 


^ permalink raw reply related

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

[resend because the Android gmail client apparently generates HTML
emails even for plain text]

On Thu, May 12, 2011 at 1:37 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, May 12, 2011 at 10:15 AM, 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?
>
> Nope, that doesn't look very helpful.
>
>> 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.
>
> That's the _point_ of bisection. It jumps around. You can start off
> trying to pick points on my development tree, but I only have a
> hundred merges or so. You're going to start delving into the actual
> development versions very quickly. And if you don't do it early,
> bisection is going to be much much slower, because it's not going to
> pick half-way points.
>
> So bisection works so well exactly because it picks points that are
> far away from each other, and you should just totally ignore the
> version number. It's meaningless. Looking at it just confuses you.
> Don't do it.
>

I actually had better results looking at the version number, saying
"blech", and running git merge v2.6.38.  (git bisect good gets a
little confused if I feed it the merge result, but I can just lie.)

Anyway, I must have made a mistake somewhere.  The regression is in
drm (presumably i915) and it has a new thread now.

--Andy

^ permalink raw reply

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

On Fri, May 13, 2011 at 4:20 AM, Christian Couder
<christian.couder@gmail.com> wrote:
> 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/

No, it was this:

http://stackoverflow.com/questions/5638211/how-do-you-get-git-bisect-to-ignore-merged-branches

--Andy

>
> ?
>
> Thanks,
> Christian.
>

^ permalink raw reply

* Re: question about drivers/isdn/hisax/st5481_init.c
From: Julia Lawall @ 2011-05-13 13:35 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Karsten Keil, netdev, linux-kernel
In-Reply-To: <1305292826.4065.443.camel@localhost>

On Fri, 13 May 2011, Ben Hutchings wrote:

> On Fri, 2011-05-13 at 07:53 +0200, Julia Lawall wrote:
> > 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?
> 
> Some older drivers have these device lists hanging around from before
> the device model has implemented.  If the list isn't being used now then
> it's probably fine to remove.

OK, I'll propose a patch in that direction.  Thanks.

julia

^ permalink raw reply

* Re: [PATCH] Add libertas_disablemesh module parameter to disable mesh interface
From: Sascha Silbe @ 2011-05-13 13:26 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: linux-wireless, devel, Dan Williams,
	John-i2JScjCrrBUtjPN9IyLgGT5Lv7v+73c5YPYVAmT7z5s,
	W.Linville-i2JScjCrrBUtjPN9IyLgGT5Lv7v+73c5YPYVAmT7z5s,
	linville-2XuSBdqkA4R54TAoqtyWWQ, libertas-dev, netdev,
	linux-kernel
In-Reply-To: <20110511100030.cad34893.rdunlap-/UHa2rfvQTnk1uMJSBkQmQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

Excerpts from Randy Dunlap's message of Wed May 11 19:00:30 +0200 2011:
> On Wed, 11 May 2011 14:52:34 +0200 Sascha Silbe wrote:
> 
> > This allows individual users and deployments to disable mesh support at
> > runtime, i.e. without having to build and maintain a custom kernel.
> 
> I guess a user could want to do this on a per-driver basis, but ISTM that
> it would be better to be something like a sysctl that applies to all (wireless)
> drivers.

AFAIK libertas is the only driver with a mesh implementation based on an
outdated 802.11s draft. To clarify, we could rename the parameter to
lbs_disable_olpcmesh (or just disable_olpcmesh).
Disabling 802.11s (latest draft) based mesh support is a separate
matter.

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 494 bytes --]

^ permalink raw reply

* Re: question about drivers/isdn/hisax/st5481_init.c
From: Ben Hutchings @ 2011-05-13 13:20 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Karsten Keil, netdev, linux-kernel
In-Reply-To: <Pine.LNX.4.64.1105130748060.9190@ask.diku.dk>

On Fri, 2011-05-13 at 07:53 +0200, Julia Lawall wrote:
> 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?

Some older drivers have these device lists hanging around from before
the device model has implemented.  If the list isn't being used now then
it's probably fine to remove.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH] Add libertas_disablemesh module parameter to disable mesh interface
From: Sascha Silbe @ 2011-05-13 13:16 UTC (permalink / raw)
  To: Dan Williams
  Cc: libertas-dev, netdev, John, linux-wireless, linville,
	linux-kernel, W.Linville, devel
In-Reply-To: <1305169898.8054.19.camel@dcbw.foobar.com>


[-- Attachment #1.1: Type: text/plain, Size: 1672 bytes --]

Excerpts from Dan Williams's message of Thu May 12 05:11:36 +0200 2011:
> On Wed, 2011-05-11 at 14:52 +0200, Sascha Silbe wrote:
> > This allows individual users and deployments to disable mesh support at
> > runtime, i.e. without having to build and maintain a custom kernel.

> Does the mesh interface somehow cause problems, even when nothing is
> using it?

Some people suspect it does, but there's no hard data showing that. But
then the problems are often hard to reproduce in the first place, so
proving a correlation with mesh is even harder.

The hardware based mesh support is based on an outdated draft of
802.11s and not interoperable with any other device AFAIK. For most
users Ad-hoc networks are the better option. Disabling mesh support as
low-level as possible makes it less likely that any remains are causing
trouble. With at least four layers (firmware, kernel, NM, Sugar)
involved in managing connectivity and one of the (firmware) being closed
source, I prefer to simplify things by eliminating three layers for
functionality we don't intend to use. It makes debugging (and
blaming ;) ) a lot easier.

In the field, mesh support is currently disabled using
/sys/class/net/eth0/lbs_mesh. However, it comes back after resume
(possibly only if powercycled) and needs to be disabled again by
post-resume hacks. Race conditions with NM are possible.

A user space option would be to teach NM to disable mesh support (at
runtime - we don't want to ship a custom NM package). I'd expect the
patch to be much more invasive than the one posted for libertas.

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 494 bytes --]

[-- Attachment #2: Type: text/plain, Size: 129 bytes --]

_______________________________________________
Devel mailing list
Devel@lists.laptop.org
http://lists.laptop.org/listinfo/devel

^ permalink raw reply

* [net-next-2.6 04/10] caif: Add ref-count to framing layer
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>

Introduce Per-cpu reference for lower part of CAIF Stack.
Before freeing payload is disabled, synchronize_rcu() is called,
and then ref-count verified to be zero.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 include/net/caif/cffrml.h |    5 ++++-
 net/caif/cfcnfg.c         |    9 ++++++++-
 net/caif/cffrml.c         |   31 +++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/include/net/caif/cffrml.h b/include/net/caif/cffrml.h
index 4f126d7..afac1a4 100644
--- a/include/net/caif/cffrml.h
+++ b/include/net/caif/cffrml.h
@@ -7,12 +7,15 @@
 #ifndef CFFRML_H_
 #define CFFRML_H_
 #include <net/caif/caif_layer.h>
+#include <linux/netdevice.h>
 
 struct cffrml;
-struct cflayer *cffrml_create(u16 phyid, bool DoFCS);
+struct cflayer *cffrml_create(u16 phyid, bool use_fcs);
+void cffrml_free(struct cflayer *layr);
 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);
+int cffrml_refcnt_read(struct cflayer *layr);
 
 #endif /* CFFRML_H_ */
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index 7892cc0..3f4f31f 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -519,6 +519,13 @@ int cfcnfg_del_phy_layer(struct cfcnfg *cnfg, struct cflayer *phy_layer)
 	caif_assert(phy_layer->id == phyid);
 	caif_assert(phyinfo->frm_layer->id == phyid);
 
+	/* Fail if reference count is not zero */
+	if (cffrml_refcnt_read(phyinfo->frm_layer) != 0) {
+		pr_info("Wait for device inuse\n");
+		mutex_unlock(&cnfg->lock);
+		return -EAGAIN;
+	}
+
 	list_del_rcu(&phyinfo->node);
 	synchronize_rcu();
 
@@ -537,7 +544,7 @@ int cfcnfg_del_phy_layer(struct cfcnfg *cnfg, struct cflayer *phy_layer)
 	if (phyinfo->phy_layer != frml_dn)
 		kfree(frml_dn);
 
-	kfree(frml);
+	cffrml_free(frml);
 	kfree(phyinfo);
 	mutex_unlock(&cnfg->lock);
 
diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
index f4b8892..4f4f756 100644
--- a/net/caif/cffrml.c
+++ b/net/caif/cffrml.c
@@ -12,6 +12,7 @@
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 #include <linux/crc-ccitt.h>
+#include <linux/netdevice.h>
 #include <net/caif/caif_layer.h>
 #include <net/caif/cfpkt.h>
 #include <net/caif/cffrml.h>
@@ -21,6 +22,7 @@
 struct cffrml {
 	struct cflayer layer;
 	bool dofcs;		/* !< FCS active */
+	int __percpu		*pcpu_refcnt;
 };
 
 static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt);
@@ -31,12 +33,19 @@ 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) {
 		pr_warn("Out of memory\n");
 		return NULL;
 	}
+	this->pcpu_refcnt = alloc_percpu(int);
+	if (this->pcpu_refcnt == NULL) {
+		kfree(this);
+		return NULL;
+	}
+
 	caif_assert(offsetof(struct cffrml, layer) == 0);
 
 	memset(this, 0, sizeof(struct cflayer));
@@ -49,6 +58,13 @@ struct cflayer *cffrml_create(u16 phyid, bool use_fcs)
 	return (struct cflayer *) this;
 }
 
+void cffrml_free(struct cflayer *layer)
+{
+	struct cffrml *this = container_obj(layer);
+	free_percpu(this->pcpu_refcnt);
+	kfree(layer);
+}
+
 void cffrml_set_uplayer(struct cflayer *this, struct cflayer *up)
 {
 	this->up = up;
@@ -148,8 +164,23 @@ static void cffrml_ctrlcmd(struct cflayer *layr, enum caif_ctrlcmd ctrl,
 
 void cffrml_put(struct cflayer *layr)
 {
+	struct cffrml *this = container_obj(layr);
+	if (layr != NULL && this->pcpu_refcnt != NULL)
+		irqsafe_cpu_dec(*this->pcpu_refcnt);
 }
 
 void cffrml_hold(struct cflayer *layr)
 {
+	struct cffrml *this = container_obj(layr);
+	if (layr != NULL && this->pcpu_refcnt != NULL)
+		irqsafe_cpu_inc(*this->pcpu_refcnt);
+}
+
+int cffrml_refcnt_read(struct cflayer *layr)
+{
+	int i, refcnt = 0;
+	struct cffrml *this = container_obj(layr);
+	for_each_possible_cpu(i)
+		refcnt += *per_cpu_ptr(this->pcpu_refcnt, i);
+	return refcnt;
 }
-- 
1.7.1


^ permalink raw reply related

* [net-next-2.6 07/10] caif: prepare support for namespaces
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>

Use struct net to reference CAIF configuration object instead of static variables.
Refactor functions caif_connect_client, caif_disconnect_client and squach
files cfcnfg.c and caif_config_utils.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 include/net/caif/caif_dev.h |   24 ++-----
 include/net/caif/cfcnfg.h   |   71 +++------------------
 net/caif/Makefile           |    2 +-
 net/caif/caif_config_util.c |   99 -----------------------------
 net/caif/caif_dev.c         |   77 +++++++++++------------
 net/caif/caif_socket.c      |    6 +-
 net/caif/cfcnfg.c           |  146 ++++++++++++++++++++++++++++++++++---------
 net/caif/chnl_net.c         |   12 ++--
 8 files changed, 176 insertions(+), 261 deletions(-)
 delete mode 100644 net/caif/caif_config_util.c

diff --git a/include/net/caif/caif_dev.h b/include/net/caif/caif_dev.h
index 6638435..c011281 100644
--- a/include/net/caif/caif_dev.h
+++ b/include/net/caif/caif_dev.h
@@ -11,6 +11,7 @@
 #include <net/caif/cfcnfg.h>
 #include <linux/caif/caif_socket.h>
 #include <linux/if.h>
+#include <linux/net.h>
 
 /**
  * struct caif_param - CAIF parameters.
@@ -62,16 +63,18 @@ struct caif_connect_request {
  * E.g. CAIF Socket will call this function for each socket it connects
  * and have one client_layer instance for each socket.
  */
-int caif_connect_client(struct caif_connect_request *conn_req,
+int caif_connect_client(struct net *net,
+			struct caif_connect_request *conn_req,
 			struct cflayer *client_layer, int *ifindex,
 			int *headroom, int *tailroom);
 
 /**
  * caif_disconnect_client - Disconnects a client from the CAIF stack.
  *
- * @client_layer: Client layer to be removed.
+ * @client_layer: Client layer to be disconnected.
  */
-int caif_disconnect_client(struct cflayer *client_layer);
+int caif_disconnect_client(struct net *net, struct cflayer *client_layer);
+
 
 /**
  * caif_client_register_refcnt - register ref-count functions provided by client.
@@ -91,21 +94,6 @@ 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
- * @con_req:	Configuration parameters supplied in function
- *		caif_connect_client
- * @channel_setup_param: Parameters supplied to the CAIF Core stack for
- *			 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.
diff --git a/include/net/caif/cfcnfg.h b/include/net/caif/cfcnfg.h
index e0a1eb5..3e93a4a 100644
--- a/include/net/caif/cfcnfg.h
+++ b/include/net/caif/cfcnfg.h
@@ -46,6 +46,12 @@ enum cfcnfg_phy_preference {
 };
 
 /**
+ * cfcnfg_create() - Get the CAIF configuration object given network.
+ * @net:	Network for the CAIF configuration object.
+ */
+struct cfcnfg *get_cfcnfg(struct net *net);
+
+/**
  * cfcnfg_create() - Create the CAIF configuration object.
  */
 struct cfcnfg *cfcnfg_create(void);
@@ -65,17 +71,15 @@ void cfcnfg_remove(struct cfcnfg *cfg);
  * @dev:	Pointer to link layer device
  * @phy_layer:	Specify the physical layer. The transmit function
  *		MUST be set in the structure.
- * @phyid:	The assigned physical ID for this layer, used in
- *		cfcnfg_add_adapt_layer to specify PHY for the link.
  * @pref:	The phy (link layer) preference.
  * @fcs:	Specify if checksum is used in CAIF Framing Layer.
- * @stx:	Specify if Start Of Frame extension is used.
+ * @stx:	Specify if Start Of Frame eXtention is used.
  */
 
 void
 cfcnfg_add_phy_layer(struct cfcnfg *cnfg, enum cfcnfg_phy_type phy_type,
 		     struct net_device *dev, struct cflayer *phy_layer,
-		     u16 *phyid, enum cfcnfg_phy_preference pref,
+		     enum cfcnfg_phy_preference pref,
 		     bool fcs, bool stx);
 
 /**
@@ -88,65 +92,6 @@ cfcnfg_add_phy_layer(struct cfcnfg *cnfg, enum cfcnfg_phy_type phy_type,
 int cfcnfg_del_phy_layer(struct cfcnfg *cnfg, struct cflayer *phy_layer);
 
 /**
- * cfcnfg_disconn_adapt_layer - Disconnects an adaptation layer.
- *
- * @cnfg:	Pointer to a CAIF configuration object, created by
- *		cfcnfg_create().
- * @adap_layer: Adaptation layer to be removed.
- */
-int cfcnfg_disconn_adapt_layer(struct cfcnfg *cnfg,
-			struct cflayer *adap_layer);
-
-/**
- * cfcnfg_release_adap_layer - Used by client to release the adaptation layer.
- *
- * @adap_layer: Adaptation layer.
- */
-void cfcnfg_release_adap_layer(struct cflayer *adap_layer);
-
-/**
- * cfcnfg_add_adaptation_layer - Add an adaptation layer to the CAIF stack.
- *
- * The adaptation Layer is where the interface to application or higher-level
- * driver functionality is implemented.
- *
- * @cnfg:		Pointer to a CAIF configuration object, created by
- *			cfcnfg_create().
- * @param:		Link setup parameters.
- * @adap_layer:		Specify the adaptation layer; the receive and
- *			flow-control functions MUST be set in the structure.
- * @ifindex:		Link layer interface index used for this connection.
- * @proto_head:		Protocol head-space needed by CAIF protocol,
- *			excluding link layer.
- * @proto_tail:		Protocol tail-space needed by CAIF protocol,
- *			excluding link layer.
- */
-int cfcnfg_add_adaptation_layer(struct cfcnfg *cnfg,
-			    struct cfctrl_link_param *param,
-			    struct cflayer *adap_layer,
-			    int *ifindex,
-			    int *proto_head,
-			    int *proto_tail);
-
-/**
- * cfcnfg_get_phyid() - Get physical ID, given type.
- * Returns one of the physical interfaces matching the given type.
- * Zero if no match is found.
- * @cnfg:	Configuration object
- * @phy_pref:	Caif Link Layer preference
- */
-struct dev_info *cfcnfg_get_phyid(struct cfcnfg *cnfg,
-		     enum cfcnfg_phy_preference phy_pref);
-
-/**
- * cfcnfg_get_id_from_ifi() - Get the Physical Identifier of ifindex,
- * 			it matches caif physical id with the kernel interface id.
- * @cnfg:	Configuration object
- * @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
diff --git a/net/caif/Makefile b/net/caif/Makefile
index 9d38e40..ebcd4e7 100644
--- a/net/caif/Makefile
+++ b/net/caif/Makefile
@@ -5,7 +5,7 @@ caif-y := caif_dev.o \
 	cffrml.o cfveil.o cfdbgl.o\
 	cfserl.o cfdgml.o  \
 	cfrfml.o cfvidl.o cfutill.o \
-	cfsrvl.o cfpkt_skbuff.o caif_config_util.o
+	cfsrvl.o cfpkt_skbuff.o
 
 obj-$(CONFIG_CAIF) += caif.o
 obj-$(CONFIG_CAIF_NETDEV) += chnl_net.o
diff --git a/net/caif/caif_config_util.c b/net/caif/caif_config_util.c
deleted file mode 100644
index 9b63e4e..0000000
--- a/net/caif/caif_config_util.c
+++ /dev/null
@@ -1,99 +0,0 @@
-/*
- * Copyright (C) ST-Ericsson AB 2010
- * Author:	Sjur Brendeland sjur.brandeland@stericsson.com
- * License terms: GNU General Public License (GPL) version 2
- */
-
-#include <linux/module.h>
-#include <linux/spinlock.h>
-#include <net/caif/cfctrl.h>
-#include <net/caif/cfcnfg.h>
-#include <net/caif/caif_dev.h>
-
-int caif_connect_req_to_link_param(struct cfcnfg *cnfg,
-				   struct caif_connect_request *s,
-				   struct cfctrl_link_param *l)
-{
-	struct dev_info *dev_info;
-	enum cfcnfg_phy_preference pref;
-	int res;
-
-	memset(l, 0, sizeof(*l));
-	/* In caif protocol low value is high priority */
-	l->priority = CAIF_PRIO_MAX - s->priority + 1;
-
-	if (s->ifindex != 0){
-		res = cfcnfg_get_id_from_ifi(cnfg, s->ifindex);
-		if (res < 0)
-			return res;
-		l->phyid = res;
-	}
-	else {
-		switch (s->link_selector) {
-		case CAIF_LINK_HIGH_BANDW:
-			pref = CFPHYPREF_HIGH_BW;
-			break;
-		case CAIF_LINK_LOW_LATENCY:
-			pref = CFPHYPREF_LOW_LAT;
-			break;
-		default:
-			return -EINVAL;
-		}
-		dev_info = cfcnfg_get_phyid(cnfg, pref);
-		if (dev_info == NULL)
-			return -ENODEV;
-		l->phyid = dev_info->id;
-	}
-	switch (s->protocol) {
-	case CAIFPROTO_AT:
-		l->linktype = CFCTRL_SRV_VEI;
-		if (s->sockaddr.u.at.type == CAIF_ATTYPE_PLAIN)
-			l->chtype = 0x02;
-		else
-			l->chtype = s->sockaddr.u.at.type;
-		l->endpoint = 0x00;
-		break;
-	case CAIFPROTO_DATAGRAM:
-		l->linktype = CFCTRL_SRV_DATAGRAM;
-		l->chtype = 0x00;
-		l->u.datagram.connid = s->sockaddr.u.dgm.connection_id;
-		break;
-	case CAIFPROTO_DATAGRAM_LOOP:
-		l->linktype = CFCTRL_SRV_DATAGRAM;
-		l->chtype = 0x03;
-		l->endpoint = 0x00;
-		l->u.datagram.connid = s->sockaddr.u.dgm.connection_id;
-		break;
-	case CAIFPROTO_RFM:
-		l->linktype = CFCTRL_SRV_RFM;
-		l->u.datagram.connid = s->sockaddr.u.rfm.connection_id;
-		strncpy(l->u.rfm.volume, s->sockaddr.u.rfm.volume,
-			sizeof(l->u.rfm.volume)-1);
-		l->u.rfm.volume[sizeof(l->u.rfm.volume)-1] = 0;
-		break;
-	case CAIFPROTO_UTIL:
-		l->linktype = CFCTRL_SRV_UTIL;
-		l->endpoint = 0x00;
-		l->chtype = 0x00;
-		strncpy(l->u.utility.name, s->sockaddr.u.util.service,
-			sizeof(l->u.utility.name)-1);
-		l->u.utility.name[sizeof(l->u.utility.name)-1] = 0;
-		caif_assert(sizeof(l->u.utility.name) > 10);
-		l->u.utility.paramlen = s->param.size;
-		if (l->u.utility.paramlen > sizeof(l->u.utility.params))
-			l->u.utility.paramlen = sizeof(l->u.utility.params);
-
-		memcpy(l->u.utility.params, s->param.data,
-		       l->u.utility.paramlen);
-
-		break;
-	case CAIFPROTO_DEBUG:
-		l->linktype = CFCTRL_SRV_DBG;
-		l->endpoint = s->sockaddr.u.dbg.service;
-		l->chtype = s->sockaddr.u.dbg.type;
-		break;
-	default:
-		return -EINVAL;
-	}
-	return 0;
-}
diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index 6d1d86b..0e651cf 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -21,7 +21,6 @@
 #include <net/net_namespace.h>
 #include <net/pkt_sched.h>
 #include <net/caif/caif_device.h>
-#include <net/caif/caif_dev.h>
 #include <net/caif/caif_layer.h>
 #include <net/caif/cfpkt.h>
 #include <net/caif/cfcnfg.h>
@@ -43,11 +42,21 @@ struct caif_device_entry_list {
 };
 
 struct caif_net {
+	struct cfcnfg *cfg;
 	struct caif_device_entry_list caifdevs;
 };
 
 static int caif_net_id;
-static struct cfcnfg *cfg;
+
+struct cfcnfg *get_cfcnfg(struct net *net)
+{
+	struct caif_net *caifn;
+	BUG_ON(!net);
+	caifn = net_generic(net, caif_net_id);
+	BUG_ON(!caifn);
+	return caifn->cfg;
+}
+EXPORT_SYMBOL(get_cfcnfg);
 
 static struct caif_device_entry_list *caif_device_list(struct net *net)
 {
@@ -191,12 +200,17 @@ 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 cfcnfg *cfg;
 	struct caif_device_entry_list *caifdevs =
 	    caif_device_list(dev_net(dev));
 
 	if (dev->type != ARPHRD_CAIF)
 		return 0;
 
+	cfg = get_cfcnfg(dev_net(dev));
+	if (cfg == NULL)
+		return 0;
+
 	switch (what) {
 	case NETDEV_REGISTER:
 		caifd = caif_device_alloc(dev);
@@ -235,7 +249,6 @@ static int caif_device_notify(struct notifier_block *me, unsigned long what,
 				     phy_type,
 				     dev,
 				     &caifd->layer,
-				     0,
 				     pref,
 				     caifdev->use_fcs,
 				     caifdev->use_stx);
@@ -323,35 +336,20 @@ static struct notifier_block caif_device_notifier = {
 	.priority = 0,
 };
 
-int caif_connect_client(struct caif_connect_request *conn_req,
-		       struct cflayer *client_layer, int *ifindex,
-		       int *headroom, int *tailroom)
-{
-	struct cfctrl_link_param param;
-	int ret;
-
-	ret = caif_connect_req_to_link_param(cfg, conn_req, &param);
-	if (ret)
-		return ret;
-	/* Hook up the adaptation layer. */
-	return cfcnfg_add_adaptation_layer(cfg, &param,
-				       client_layer, ifindex,
-				       headroom, tailroom);
-}
-EXPORT_SYMBOL(caif_connect_client);
-
-int caif_disconnect_client(struct cflayer *adap_layer)
-{
-	return cfcnfg_disconn_adapt_layer(cfg, adap_layer);
-}
-EXPORT_SYMBOL(caif_disconnect_client);
-
 /* Per-namespace Caif devices handling */
 static int caif_init_net(struct net *net)
 {
 	struct caif_net *caifn = net_generic(net, caif_net_id);
+	BUG_ON(!caifn);
 	INIT_LIST_HEAD(&caifn->caifdevs.list);
 	mutex_init(&caifn->caifdevs.lock);
+
+	caifn->cfg = cfcnfg_create();
+	if (!caifn->cfg) {
+		pr_warn("can't create cfcnfg\n");
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 
@@ -360,10 +358,17 @@ static void caif_exit_net(struct net *net)
 	struct caif_device_entry *caifd, *tmp;
 	struct caif_device_entry_list *caifdevs =
 	    caif_device_list(net);
+	struct cfcnfg *cfg;
 
 	rtnl_lock();
 	mutex_lock(&caifdevs->lock);
 
+	cfg = get_cfcnfg(net);
+	if (cfg == NULL) {
+		mutex_unlock(&caifdevs->lock);
+		return;
+	}
+
 	list_for_each_entry_safe(caifd, tmp, &caifdevs->list, list) {
 		int i = 0;
 		list_del_rcu(&caifd->list);
@@ -382,7 +387,7 @@ static void caif_exit_net(struct net *net)
 		free_percpu(caifd->pcpu_refcnt);
 		kfree(caifd);
 	}
-
+	cfcnfg_remove(cfg);
 
 	mutex_unlock(&caifdevs->lock);
 	rtnl_unlock();
@@ -400,32 +405,22 @@ static int __init caif_device_init(void)
 {
 	int result;
 
-	cfg = cfcnfg_create();
-	if (!cfg) {
-		pr_warn("can't create cfcnfg\n");
-		goto err_cfcnfg_create_failed;
-	}
 	result = register_pernet_device(&caif_net_ops);
 
-	if (result) {
-		kfree(cfg);
-		cfg = NULL;
+	if (result)
 		return result;
-	}
-	dev_add_pack(&caif_packet_type);
+
 	register_netdevice_notifier(&caif_device_notifier);
+	dev_add_pack(&caif_packet_type);
 
 	return result;
-err_cfcnfg_create_failed:
-	return -ENODEV;
 }
 
 static void __exit caif_device_exit(void)
 {
-	dev_remove_pack(&caif_packet_type);
 	unregister_pernet_device(&caif_net_ops);
 	unregister_netdevice_notifier(&caif_device_notifier);
-	cfcnfg_remove(cfg);
+	dev_remove_pack(&caif_packet_type);
 }
 
 module_init(caif_device_init);
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 01f612d..653db75 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -810,7 +810,7 @@ static int caif_connect(struct socket *sock, struct sockaddr *uaddr,
 				sk->sk_state == CAIF_DISCONNECTED);
 		if (sk->sk_shutdown & SHUTDOWN_MASK) {
 			/* Allow re-connect after SHUTDOWN_IND */
-			caif_disconnect_client(&cf_sk->layer);
+			caif_disconnect_client(sock_net(sk), &cf_sk->layer);
 			break;
 		}
 		/* No reconnect on a seqpacket socket */
@@ -851,7 +851,7 @@ 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,
+	err = caif_connect_client(sock_net(sk), &cf_sk->conn_req,
 				&cf_sk->layer, &ifindex, &headroom, &tailroom);
 
 	if (err < 0) {
@@ -949,7 +949,7 @@ static int caif_release(struct socket *sock)
 
 	if (cf_sk->sk.sk_socket->state == SS_CONNECTED ||
 		cf_sk->sk.sk_socket->state == SS_CONNECTING)
-		res = caif_disconnect_client(&cf_sk->layer);
+		res = caif_disconnect_client(sock_net(sk), &cf_sk->layer);
 
 	cf_sk->sk.sk_socket->state = SS_DISCONNECTING;
 	wake_up_interruptible_poll(sk_sleep(sk), POLLERR|POLLHUP);
diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index 3f4f31f..e857d89 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -150,7 +150,7 @@ static void cfctrl_enum_resp(void)
 {
 }
 
-struct dev_info *cfcnfg_get_phyid(struct cfcnfg *cnfg,
+static struct dev_info *cfcnfg_get_phyid(struct cfcnfg *cnfg,
 				  enum cfcnfg_phy_preference phy_pref)
 {
 	/* Try to match with specified preference */
@@ -171,7 +171,7 @@ struct dev_info *cfcnfg_get_phyid(struct cfcnfg *cnfg,
 	return NULL;
 }
 
-int cfcnfg_get_id_from_ifi(struct cfcnfg *cnfg, int ifi)
+static int cfcnfg_get_id_from_ifi(struct cfcnfg *cnfg, int ifi)
 {
 	struct cfcnfg_phyinfo *phy;
 
@@ -181,11 +181,12 @@ int cfcnfg_get_id_from_ifi(struct cfcnfg *cnfg, int ifi)
 	return -ENODEV;
 }
 
-int cfcnfg_disconn_adapt_layer(struct cfcnfg *cfg, struct cflayer *adap_layer)
+int caif_disconnect_client(struct net *net, struct cflayer *adap_layer)
 {
 	u8 channel_id = 0;
 	int ret = 0;
 	struct cflayer *servl = NULL;
+	struct cfcnfg *cfg = get_cfcnfg(net);
 
 	caif_assert(adap_layer != NULL);
 
@@ -217,14 +218,7 @@ end:
 	return ret;
 
 }
-EXPORT_SYMBOL(cfcnfg_disconn_adapt_layer);
-
-void cfcnfg_release_adap_layer(struct cflayer *adap_layer)
-{
-	if (adap_layer->dn)
-		cfsrvl_put(adap_layer->dn);
-}
-EXPORT_SYMBOL(cfcnfg_release_adap_layer);
+EXPORT_SYMBOL(caif_disconnect_client);
 
 static void cfcnfg_linkdestroy_rsp(struct cflayer *layer, u8 channel_id)
 {
@@ -238,19 +232,109 @@ static const int protohead[CFCTRL_SRV_MASK] = {
 	[CFCTRL_SRV_DBG] = 3,
 };
 
-int cfcnfg_add_adaptation_layer(struct cfcnfg *cnfg,
-				struct cfctrl_link_param *param,
-				struct cflayer *adap_layer,
-				int *ifindex,
+
+static int caif_connect_req_to_link_param(struct cfcnfg *cnfg,
+				   struct caif_connect_request *s,
+				   struct cfctrl_link_param *l)
+{
+	struct dev_info *dev_info;
+	enum cfcnfg_phy_preference pref;
+	int res;
+
+	memset(l, 0, sizeof(*l));
+	/* In caif protocol low value is high priority */
+	l->priority = CAIF_PRIO_MAX - s->priority + 1;
+
+	if (s->ifindex != 0) {
+		res = cfcnfg_get_id_from_ifi(cnfg, s->ifindex);
+		if (res < 0)
+			return res;
+		l->phyid = res;
+	} else {
+		switch (s->link_selector) {
+		case CAIF_LINK_HIGH_BANDW:
+			pref = CFPHYPREF_HIGH_BW;
+			break;
+		case CAIF_LINK_LOW_LATENCY:
+			pref = CFPHYPREF_LOW_LAT;
+			break;
+		default:
+			return -EINVAL;
+		}
+		dev_info = cfcnfg_get_phyid(cnfg, pref);
+		if (dev_info == NULL)
+			return -ENODEV;
+		l->phyid = dev_info->id;
+	}
+	switch (s->protocol) {
+	case CAIFPROTO_AT:
+		l->linktype = CFCTRL_SRV_VEI;
+		l->endpoint = (s->sockaddr.u.at.type >> 2) & 0x3;
+		l->chtype = s->sockaddr.u.at.type & 0x3;
+		break;
+	case CAIFPROTO_DATAGRAM:
+		l->linktype = CFCTRL_SRV_DATAGRAM;
+		l->chtype = 0x00;
+		l->u.datagram.connid = s->sockaddr.u.dgm.connection_id;
+		break;
+	case CAIFPROTO_DATAGRAM_LOOP:
+		l->linktype = CFCTRL_SRV_DATAGRAM;
+		l->chtype = 0x03;
+		l->endpoint = 0x00;
+		l->u.datagram.connid = s->sockaddr.u.dgm.connection_id;
+		break;
+	case CAIFPROTO_RFM:
+		l->linktype = CFCTRL_SRV_RFM;
+		l->u.datagram.connid = s->sockaddr.u.rfm.connection_id;
+		strncpy(l->u.rfm.volume, s->sockaddr.u.rfm.volume,
+			sizeof(l->u.rfm.volume)-1);
+		l->u.rfm.volume[sizeof(l->u.rfm.volume)-1] = 0;
+		break;
+	case CAIFPROTO_UTIL:
+		l->linktype = CFCTRL_SRV_UTIL;
+		l->endpoint = 0x00;
+		l->chtype = 0x00;
+		strncpy(l->u.utility.name, s->sockaddr.u.util.service,
+			sizeof(l->u.utility.name)-1);
+		l->u.utility.name[sizeof(l->u.utility.name)-1] = 0;
+		caif_assert(sizeof(l->u.utility.name) > 10);
+		l->u.utility.paramlen = s->param.size;
+		if (l->u.utility.paramlen > sizeof(l->u.utility.params))
+			l->u.utility.paramlen = sizeof(l->u.utility.params);
+
+		memcpy(l->u.utility.params, s->param.data,
+		       l->u.utility.paramlen);
+
+		break;
+	case CAIFPROTO_DEBUG:
+		l->linktype = CFCTRL_SRV_DBG;
+		l->endpoint = s->sockaddr.u.dbg.service;
+		l->chtype = s->sockaddr.u.dbg.type;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+int caif_connect_client(struct net *net, struct caif_connect_request *conn_req,
+			struct cflayer *adap_layer, int *ifindex,
 				int *proto_head,
 				int *proto_tail)
 {
 	struct cflayer *frml;
 	struct cfcnfg_phyinfo *phy;
 	int err;
+	struct cfctrl_link_param param;
+	struct cfcnfg *cfg = get_cfcnfg(net);
+	caif_assert(cfg != NULL);
 
 	rcu_read_lock();
-	phy = cfcnfg_get_phyinfo_rcu(cnfg, param->phyid);
+	err = caif_connect_req_to_link_param(cfg, conn_req, &param);
+	if (err)
+		goto unlock;
+
+	phy = cfcnfg_get_phyinfo_rcu(cfg, param.phyid);
 	if (!phy) {
 		err = -ENODEV;
 		goto unlock;
@@ -276,28 +360,29 @@ int cfcnfg_add_adaptation_layer(struct cfcnfg *cnfg,
 		pr_err("Specified PHY type does not exist!\n");
 		goto unlock;
 	}
-	caif_assert(param->phyid == phy->id);
+	caif_assert(param.phyid == phy->id);
 	caif_assert(phy->frm_layer->id ==
-		     param->phyid);
+		     param.phyid);
 	caif_assert(phy->phy_layer->id ==
-		     param->phyid);
+		     param.phyid);
 
 	*ifindex = phy->ifindex;
 	*proto_tail = 2;
 	*proto_head =
-		protohead[param->linktype] + (phy->use_stx ? 1 : 0);
+
+	protohead[param.linktype] + (phy->use_stx ? 1 : 0);
 
 	rcu_read_unlock();
 
 	/* FIXME: ENUMERATE INITIALLY WHEN ACTIVATING PHYSICAL INTERFACE */
-	cfctrl_enum_req(cnfg->ctrl, param->phyid);
-	return cfctrl_linkup_request(cnfg->ctrl, param, adap_layer);
+	cfctrl_enum_req(cfg->ctrl, param.phyid);
+	return cfctrl_linkup_request(cfg->ctrl, &param, adap_layer);
 
 unlock:
 	rcu_read_unlock();
 	return err;
 }
-EXPORT_SYMBOL(cfcnfg_add_adaptation_layer);
+EXPORT_SYMBOL(caif_connect_client);
 
 static void cfcnfg_reject_rsp(struct cflayer *layer, u8 channel_id,
 			     struct cflayer *adapt_layer)
@@ -389,7 +474,7 @@ unlock:
 void
 cfcnfg_add_phy_layer(struct cfcnfg *cnfg, enum cfcnfg_phy_type phy_type,
 		     struct net_device *dev, struct cflayer *phy_layer,
-		     u16 *phy_id, enum cfcnfg_phy_preference pref,
+		     enum cfcnfg_phy_preference pref,
 		     bool fcs, bool stx)
 {
 	struct cflayer *frml;
@@ -512,23 +597,26 @@ int cfcnfg_del_phy_layer(struct cfcnfg *cnfg, struct cflayer *phy_layer)
 	phyid = phy_layer->id;
 	phyinfo = cfcnfg_get_phyinfo_rcu(cnfg, phyid);
 
-	if (phyinfo == NULL)
+	if (phyinfo == NULL) {
+		mutex_unlock(&cnfg->lock);
 		return 0;
+	}
 	caif_assert(phyid == phyinfo->id);
 	caif_assert(phy_layer == phyinfo->phy_layer);
 	caif_assert(phy_layer->id == phyid);
 	caif_assert(phyinfo->frm_layer->id == phyid);
 
+	list_del_rcu(&phyinfo->node);
+	synchronize_rcu();
+
 	/* Fail if reference count is not zero */
 	if (cffrml_refcnt_read(phyinfo->frm_layer) != 0) {
 		pr_info("Wait for device inuse\n");
+		list_add_rcu(&phyinfo->node, &cnfg->phys);
 		mutex_unlock(&cnfg->lock);
 		return -EAGAIN;
 	}
 
-	list_del_rcu(&phyinfo->node);
-	synchronize_rcu();
-
 	frml = phyinfo->frm_layer;
 	frml_dn = frml->dn;
 	cffrml_set_uplayer(frml, NULL);
@@ -539,8 +627,6 @@ int cfcnfg_del_phy_layer(struct cfcnfg *cnfg, struct cflayer *phy_layer)
 	}
 	layer_set_up(phy_layer, NULL);
 
-
-
 	if (phyinfo->phy_layer != frml_dn)
 		kfree(frml_dn);
 
diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 9ef8f16..1a3398a 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -20,7 +20,6 @@
 #include <linux/caif/if_caif.h>
 #include <net/rtnetlink.h>
 #include <net/caif/caif_layer.h>
-#include <net/caif/cfcnfg.h>
 #include <net/caif/cfpkt.h>
 #include <net/caif/caif_dev.h>
 
@@ -270,8 +269,9 @@ static int chnl_net_open(struct net_device *dev)
 
 	if (priv->state != CAIF_CONNECTING) {
 		priv->state = CAIF_CONNECTING;
-		result = caif_connect_client(&priv->conn_req, &priv->chnl,
-					&llifindex, &headroom, &tailroom);
+		result = caif_connect_client(dev_net(dev), &priv->conn_req,
+						&priv->chnl, &llifindex,
+						&headroom, &tailroom);
 		if (result != 0) {
 				pr_debug("err: "
 					 "Unable to register and open device,"
@@ -327,7 +327,7 @@ static int chnl_net_open(struct net_device *dev)
 
 	if (result == 0) {
 		pr_debug("connect timeout\n");
-		caif_disconnect_client(&priv->chnl);
+		caif_disconnect_client(dev_net(dev), &priv->chnl);
 		priv->state = CAIF_DISCONNECTED;
 		pr_debug("state disconnected\n");
 		result = -ETIMEDOUT;
@@ -343,7 +343,7 @@ static int chnl_net_open(struct net_device *dev)
 	return 0;
 
 error:
-	caif_disconnect_client(&priv->chnl);
+	caif_disconnect_client(dev_net(dev), &priv->chnl);
 	priv->state = CAIF_DISCONNECTED;
 	pr_debug("state disconnected\n");
 	return result;
@@ -357,7 +357,7 @@ static int chnl_net_stop(struct net_device *dev)
 	ASSERT_RTNL();
 	priv = netdev_priv(dev);
 	priv->state = CAIF_DISCONNECTED;
-	caif_disconnect_client(&priv->chnl);
+	caif_disconnect_client(dev_net(dev), &priv->chnl);
 	return 0;
 }
 
-- 
1.7.1


^ permalink raw reply related

* [net-next-2.6 03/10] caif: Use RCU and lists in cfcnfg.c for managing caif link layers
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 lists are used for handling the link layers instead of array.
When generating CAIF phy-id, ifindex is used as base. Legal range is 1-6.
Introduced set_phy_state() for managing CAIF Link layer state.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/cfcnfg.c |  373 ++++++++++++++++++++++++++++++-----------------------
 1 files changed, 213 insertions(+), 160 deletions(-)

diff --git a/net/caif/cfcnfg.c b/net/caif/cfcnfg.c
index 25c0b19..7892cc0 100644
--- a/net/caif/cfcnfg.c
+++ b/net/caif/cfcnfg.c
@@ -10,6 +10,7 @@
 #include <linux/stddef.h>
 #include <linux/slab.h>
 #include <linux/netdevice.h>
+#include <linux/module.h>
 #include <net/caif/caif_layer.h>
 #include <net/caif/cfpkt.h>
 #include <net/caif/cfcnfg.h>
@@ -18,11 +19,7 @@
 #include <net/caif/cffrml.h>
 #include <net/caif/cfserl.h>
 #include <net/caif/cfsrvl.h>
-
-#include <linux/module.h>
-#include <asm/atomic.h>
-
-#define MAX_PHY_LAYERS 7
+#include <net/caif/caif_dev.h>
 
 #define container_obj(layr) container_of(layr, struct cfcnfg, layer)
 
@@ -30,6 +27,9 @@
  * to manage physical interfaces
  */
 struct cfcnfg_phyinfo {
+	struct list_head node;
+	bool up;
+
 	/* Pointer to the layer below the MUX (framing layer) */
 	struct cflayer *frm_layer;
 	/* Pointer to the lowest actual physical layer */
@@ -39,9 +39,6 @@ struct cfcnfg_phyinfo {
 	/* Preference of the physical in interface */
 	enum cfcnfg_phy_preference pref;
 
-	/* Reference count, number of channels using the device */
-	int phy_ref_count;
-
 	/* Information about the physical device */
 	struct dev_info dev_info;
 
@@ -59,8 +56,8 @@ struct cfcnfg {
 	struct cflayer layer;
 	struct cflayer *ctrl;
 	struct cflayer *mux;
-	u8 last_phyid;
-	struct cfcnfg_phyinfo phy_layers[MAX_PHY_LAYERS];
+	struct list_head phys;
+	struct mutex lock;
 };
 
 static void cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id,
@@ -76,6 +73,9 @@ struct cfcnfg *cfcnfg_create(void)
 {
 	struct cfcnfg *this;
 	struct cfctrl_rsp *resp;
+
+	might_sleep();
+
 	/* Initiate this layer */
 	this = kzalloc(sizeof(struct cfcnfg), GFP_ATOMIC);
 	if (!this) {
@@ -99,15 +99,19 @@ struct cfcnfg *cfcnfg_create(void)
 	resp->radioset_rsp = cfctrl_resp_func;
 	resp->linksetup_rsp = cfcnfg_linkup_rsp;
 	resp->reject_rsp = cfcnfg_reject_rsp;
-
-	this->last_phyid = 1;
+	INIT_LIST_HEAD(&this->phys);
 
 	cfmuxl_set_uplayer(this->mux, this->ctrl, 0);
 	layer_set_dn(this->ctrl, this->mux);
 	layer_set_up(this->ctrl, this);
+	mutex_init(&this->lock);
+
 	return this;
 out_of_mem:
 	pr_warn("Out of memory\n");
+
+	synchronize_rcu();
+
 	kfree(this->mux);
 	kfree(this->ctrl);
 	kfree(this);
@@ -117,7 +121,10 @@ EXPORT_SYMBOL(cfcnfg_create);
 
 void cfcnfg_remove(struct cfcnfg *cfg)
 {
+	might_sleep();
 	if (cfg) {
+		synchronize_rcu();
+
 		kfree(cfg->mux);
 		kfree(cfg->ctrl);
 		kfree(cfg);
@@ -128,6 +135,17 @@ static void cfctrl_resp_func(void)
 {
 }
 
+static struct cfcnfg_phyinfo *cfcnfg_get_phyinfo_rcu(struct cfcnfg *cnfg,
+							u8 phyid)
+{
+	struct cfcnfg_phyinfo *phy;
+
+	list_for_each_entry_rcu(phy, &cnfg->phys, node)
+		if (phy->id == phyid)
+			return phy;
+	return NULL;
+}
+
 static void cfctrl_enum_resp(void)
 {
 }
@@ -135,106 +153,65 @@ static void cfctrl_enum_resp(void)
 struct dev_info *cfcnfg_get_phyid(struct cfcnfg *cnfg,
 				  enum cfcnfg_phy_preference phy_pref)
 {
-	u16 i;
-
 	/* Try to match with specified preference */
-	for (i = 1; i < MAX_PHY_LAYERS; i++) {
-		if (cnfg->phy_layers[i].id == i &&
-		     cnfg->phy_layers[i].pref == phy_pref &&
-		     cnfg->phy_layers[i].frm_layer != NULL) {
-			caif_assert(cnfg->phy_layers != NULL);
-			caif_assert(cnfg->phy_layers[i].id == i);
-			return &cnfg->phy_layers[i].dev_info;
-		}
-	}
-	/* Otherwise just return something */
-	for (i = 1; i < MAX_PHY_LAYERS; i++) {
-		if (cnfg->phy_layers[i].id == i) {
-			caif_assert(cnfg->phy_layers != NULL);
-			caif_assert(cnfg->phy_layers[i].id == i);
-			return &cnfg->phy_layers[i].dev_info;
-		}
+	struct cfcnfg_phyinfo *phy;
+
+	list_for_each_entry_rcu(phy, &cnfg->phys, node) {
+		if (phy->up && phy->pref == phy_pref &&
+				phy->frm_layer != NULL)
+
+			return &phy->dev_info;
 	}
 
-	return NULL;
-}
+	/* Otherwise just return something */
+	list_for_each_entry_rcu(phy, &cnfg->phys, node)
+		if (phy->up)
+			return &phy->dev_info;
 
-static struct cfcnfg_phyinfo *cfcnfg_get_phyinfo(struct cfcnfg *cnfg,
-							u8 phyid)
-{
-	int i;
-	/* Try to match with specified preference */
-	for (i = 0; i < MAX_PHY_LAYERS; i++)
-		if (cnfg->phy_layers[i].frm_layer != NULL &&
-		    cnfg->phy_layers[i].id == phyid)
-			return &cnfg->phy_layers[i];
 	return NULL;
 }
 
-
 int cfcnfg_get_id_from_ifi(struct cfcnfg *cnfg, int ifi)
 {
-	int i;
-	for (i = 0; i < MAX_PHY_LAYERS; i++)
-		if (cnfg->phy_layers[i].frm_layer != NULL &&
-				cnfg->phy_layers[i].ifindex == ifi)
-			return i;
+	struct cfcnfg_phyinfo *phy;
+
+	list_for_each_entry_rcu(phy, &cnfg->phys, node)
+		if (phy->ifindex == ifi && phy->up)
+			return phy->id;
 	return -ENODEV;
 }
 
-int cfcnfg_disconn_adapt_layer(struct cfcnfg *cnfg, struct cflayer *adap_layer)
+int cfcnfg_disconn_adapt_layer(struct cfcnfg *cfg, struct cflayer *adap_layer)
 {
 	u8 channel_id = 0;
 	int ret = 0;
 	struct cflayer *servl = NULL;
-	struct cfcnfg_phyinfo *phyinfo = NULL;
-	u8 phyid = 0;
 
 	caif_assert(adap_layer != NULL);
+
 	channel_id = adap_layer->id;
 	if (adap_layer->dn == NULL || channel_id == 0) {
 		pr_err("adap_layer->dn == NULL or adap_layer->id is 0\n");
 		ret = -ENOTCONN;
 		goto end;
 	}
-	servl = cfmuxl_remove_uplayer(cnfg->mux, channel_id);
+
+	servl = cfmuxl_remove_uplayer(cfg->mux, channel_id);
 	if (servl == NULL) {
-		pr_err("PROTOCOL ERROR - Error removing service_layer Channel_Id(%d)",
-		       channel_id);
+		pr_err("PROTOCOL ERROR - "
+				"Error removing service_layer Channel_Id(%d)",
+				channel_id);
 		ret = -EINVAL;
 		goto end;
 	}
-	layer_set_up(servl, NULL);
-	ret = cfctrl_linkdown_req(cnfg->ctrl, channel_id, adap_layer);
-	if (ret)
-		goto end;
-	caif_assert(channel_id == servl->id);
-	if (adap_layer->dn != NULL) {
-		phyid = cfsrvl_getphyid(adap_layer->dn);
-
-		phyinfo = cfcnfg_get_phyinfo(cnfg, phyid);
-		if (phyinfo == NULL) {
-			pr_warn("No interface to send disconnect to\n");
-			ret = -ENODEV;
-			goto end;
-		}
-		if (phyinfo->id != phyid ||
-			phyinfo->phy_layer->id != phyid ||
-			phyinfo->frm_layer->id != phyid) {
-			pr_err("Inconsistency in phy registration\n");
-			ret = -EINVAL;
-			goto end;
-		}
-	}
-	if (phyinfo != NULL && --phyinfo->phy_ref_count == 0 &&
-		phyinfo->phy_layer != NULL &&
-		phyinfo->phy_layer->modemcmd != NULL) {
-		phyinfo->phy_layer->modemcmd(phyinfo->phy_layer,
-					     _CAIF_MODEMCMD_PHYIF_USELESS);
-	}
+
+	ret = cfctrl_linkdown_req(cfg->ctrl, channel_id, adap_layer);
+
 end:
-	cfsrvl_put(servl);
-	cfctrl_cancel_req(cnfg->ctrl, adap_layer);
+	cfctrl_cancel_req(cfg->ctrl, adap_layer);
+
+	/* Do RCU sync before initiating cleanup */
+	synchronize_rcu();
 	if (adap_layer->ctrlcmd != NULL)
 		adap_layer->ctrlcmd(adap_layer, CAIF_CTRLCMD_DEINIT_RSP, 0);
 	return ret;
@@ -269,39 +246,56 @@ int cfcnfg_add_adaptation_layer(struct cfcnfg *cnfg,
 				int *proto_tail)
 {
 	struct cflayer *frml;
+	struct cfcnfg_phyinfo *phy;
+	int err;
+
+	rcu_read_lock();
+	phy = cfcnfg_get_phyinfo_rcu(cnfg, param->phyid);
+	if (!phy) {
+		err = -ENODEV;
+		goto unlock;
+	}
+	err = -EINVAL;
+
 	if (adap_layer == NULL) {
 		pr_err("adap_layer is zero\n");
-		return -EINVAL;
+		goto unlock;
 	}
 	if (adap_layer->receive == NULL) {
 		pr_err("adap_layer->receive is NULL\n");
-		return -EINVAL;
+		goto unlock;
 	}
 	if (adap_layer->ctrlcmd == NULL) {
 		pr_err("adap_layer->ctrlcmd == NULL\n");
-		return -EINVAL;
+		goto unlock;
 	}
-	frml = cnfg->phy_layers[param->phyid].frm_layer;
+
+	err = -ENODEV;
+	frml = phy->frm_layer;
 	if (frml == NULL) {
 		pr_err("Specified PHY type does not exist!\n");
-		return -ENODEV;
+		goto unlock;
 	}
-	caif_assert(param->phyid == cnfg->phy_layers[param->phyid].id);
-	caif_assert(cnfg->phy_layers[param->phyid].frm_layer->id ==
+	caif_assert(param->phyid == phy->id);
+	caif_assert(phy->frm_layer->id ==
 		     param->phyid);
-	caif_assert(cnfg->phy_layers[param->phyid].phy_layer->id ==
+	caif_assert(phy->phy_layer->id ==
 		     param->phyid);
 
-	*ifindex = cnfg->phy_layers[param->phyid].ifindex;
+	*ifindex = phy->ifindex;
+	*proto_tail = 2;
 	*proto_head =
-		protohead[param->linktype]+
-		(cnfg->phy_layers[param->phyid].use_stx ? 1 : 0);
+		protohead[param->linktype] + (phy->use_stx ? 1 : 0);
 
-	*proto_tail = 2;
+	rcu_read_unlock();
 
 	/* FIXME: ENUMERATE INITIALLY WHEN ACTIVATING PHYSICAL INTERFACE */
 	cfctrl_enum_req(cnfg->ctrl, param->phyid);
 	return cfctrl_linkup_request(cnfg->ctrl, param, adap_layer);
+
+unlock:
+	rcu_read_unlock();
+	return err;
 }
 EXPORT_SYMBOL(cfcnfg_add_adaptation_layer);
 
@@ -315,32 +309,37 @@ static void cfcnfg_reject_rsp(struct cflayer *layer, u8 channel_id,
 
 static void
 cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
-		 u8 phyid, struct cflayer *adapt_layer)
+		  u8 phyid, struct cflayer *adapt_layer)
 {
 	struct cfcnfg *cnfg = container_obj(layer);
 	struct cflayer *servicel = NULL;
 	struct cfcnfg_phyinfo *phyinfo;
 	struct net_device *netdev;
 
+	rcu_read_lock();
+
 	if (adapt_layer == NULL) {
-		pr_debug("link setup response but no client exist, send linkdown back\n");
+		pr_debug("link setup response but no client exist,"
+				"send linkdown back\n");
 		cfctrl_linkdown_req(cnfg->ctrl, channel_id, NULL);
-		return;
+		goto unlock;
 	}
 
 	caif_assert(cnfg != NULL);
 	caif_assert(phyid != 0);
-	phyinfo = &cnfg->phy_layers[phyid];
+
+	phyinfo = cfcnfg_get_phyinfo_rcu(cnfg, phyid);
+	if (phyinfo == NULL) {
+		pr_err("ERROR: Link Layer Device dissapeared"
+				"while connecting\n");
+		goto unlock;
+	}
+
+	caif_assert(phyinfo != NULL);
 	caif_assert(phyinfo->id == phyid);
 	caif_assert(phyinfo->phy_layer != NULL);
 	caif_assert(phyinfo->phy_layer->id == phyid);
 
-	phyinfo->phy_ref_count++;
-	if (phyinfo->phy_ref_count == 1 &&
-	    phyinfo->phy_layer->modemcmd != NULL) {
-		phyinfo->phy_layer->modemcmd(phyinfo->phy_layer,
-					     _CAIF_MODEMCMD_PHYIF_USEFULL);
-	}
 	adapt_layer->id = channel_id;
 
 	switch (serv) {
@@ -348,7 +347,8 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
 		servicel = cfvei_create(channel_id, &phyinfo->dev_info);
 		break;
 	case CFCTRL_SRV_DATAGRAM:
-		servicel = cfdgml_create(channel_id, &phyinfo->dev_info);
+		servicel = cfdgml_create(channel_id,
+					&phyinfo->dev_info);
 		break;
 	case CFCTRL_SRV_RFM:
 		netdev = phyinfo->dev_info.dev;
@@ -365,94 +365,93 @@ cfcnfg_linkup_rsp(struct cflayer *layer, u8 channel_id, enum cfctrl_srv serv,
 		servicel = cfdbgl_create(channel_id, &phyinfo->dev_info);
 		break;
 	default:
-		pr_err("Protocol error. Link setup response - unknown channel type\n");
-		return;
+		pr_err("Protocol error. Link setup response "
+				"- unknown channel type\n");
+		goto unlock;
 	}
 	if (!servicel) {
 		pr_warn("Out of memory\n");
-		return;
+		goto unlock;
 	}
 	layer_set_dn(servicel, cnfg->mux);
 	cfmuxl_set_uplayer(cnfg->mux, servicel, channel_id);
 	layer_set_up(servicel, adapt_layer);
 	layer_set_dn(adapt_layer, servicel);
-	cfsrvl_get(servicel);
+
+	rcu_read_unlock();
+
 	servicel->ctrlcmd(servicel, CAIF_CTRLCMD_INIT_RSP, 0);
+	return;
+unlock:
+	rcu_read_unlock();
 }
 
 void
 cfcnfg_add_phy_layer(struct cfcnfg *cnfg, enum cfcnfg_phy_type phy_type,
 		     struct net_device *dev, struct cflayer *phy_layer,
-		     u16 *phyid, enum cfcnfg_phy_preference pref,
+		     u16 *phy_id, enum cfcnfg_phy_preference pref,
 		     bool fcs, bool stx)
 {
 	struct cflayer *frml;
 	struct cflayer *phy_driver = NULL;
+	struct cfcnfg_phyinfo *phyinfo;
 	int i;
+	u8 phyid;
 
+	mutex_lock(&cnfg->lock);
 
-	if (cnfg->phy_layers[cnfg->last_phyid].frm_layer == NULL) {
-		*phyid = cnfg->last_phyid;
-
-		/* range: * 1..(MAX_PHY_LAYERS-1) */
-		cnfg->last_phyid =
-		    (cnfg->last_phyid % (MAX_PHY_LAYERS - 1)) + 1;
-	} else {
-		*phyid = 0;
-		for (i = 1; i < MAX_PHY_LAYERS; i++) {
-			if (cnfg->phy_layers[i].frm_layer == NULL) {
-				*phyid = i;
-				break;
-			}
-		}
-	}
-	if (*phyid == 0) {
-		pr_err("No Available PHY ID\n");
-		return;
+	/* CAIF protocol allow maximum 6 link-layers */
+	for (i = 0; i < 7; i++) {
+		phyid = (dev->ifindex + i) & 0x7;
+		if (phyid == 0)
+			continue;
+		if (cfcnfg_get_phyinfo_rcu(cnfg, phyid) == NULL)
+			goto got_phyid;
 	}
+	pr_warn("Too many CAIF Link Layers (max 6)\n");
+	goto out;
+
+got_phyid:
+	phyinfo = kzalloc(sizeof(struct cfcnfg_phyinfo), GFP_ATOMIC);
 
 	switch (phy_type) {
 	case CFPHYTYPE_FRAG:
 		phy_driver =
-		    cfserl_create(CFPHYTYPE_FRAG, *phyid, stx);
+		    cfserl_create(CFPHYTYPE_FRAG, phyid, stx);
 		if (!phy_driver) {
 			pr_warn("Out of memory\n");
-			return;
+			goto out;
 		}
-
 		break;
 	case CFPHYTYPE_CAIF:
 		phy_driver = NULL;
 		break;
 	default:
-		pr_err("%d\n", phy_type);
-		return;
-		break;
+		goto out;
 	}
-
-	phy_layer->id = *phyid;
-	cnfg->phy_layers[*phyid].pref = pref;
-	cnfg->phy_layers[*phyid].id = *phyid;
-	cnfg->phy_layers[*phyid].dev_info.id = *phyid;
-	cnfg->phy_layers[*phyid].dev_info.dev = dev;
-	cnfg->phy_layers[*phyid].phy_layer = phy_layer;
-	cnfg->phy_layers[*phyid].phy_ref_count = 0;
-	cnfg->phy_layers[*phyid].ifindex = dev->ifindex;
-	cnfg->phy_layers[*phyid].use_stx = stx;
-	cnfg->phy_layers[*phyid].use_fcs = fcs;
+	phy_layer->id = phyid;
+	phyinfo->pref = pref;
+	phyinfo->id = phyid;
+	phyinfo->dev_info.id = phyid;
+	phyinfo->dev_info.dev = dev;
+	phyinfo->phy_layer = phy_layer;
+	phyinfo->ifindex = dev->ifindex;
+	phyinfo->use_stx = stx;
+	phyinfo->use_fcs = fcs;
 
 	phy_layer->type = phy_type;
-	frml = cffrml_create(*phyid, fcs);
+	frml = cffrml_create(phyid, fcs);
+
 	if (!frml) {
 		pr_warn("Out of memory\n");
-		return;
+		kfree(phyinfo);
+		goto out;
 	}
-	cnfg->phy_layers[*phyid].frm_layer = frml;
-	cfmuxl_set_dnlayer(cnfg->mux, frml, *phyid);
+	phyinfo->frm_layer = frml;
 	layer_set_up(frml, cnfg->mux);
 
 	if (phy_driver != NULL) {
-		phy_driver->id = *phyid;
+		phy_driver->id = phyid;
 		layer_set_dn(frml, phy_driver);
 		layer_set_up(phy_driver, frml);
 		layer_set_dn(phy_driver, phy_layer);
@@ -461,33 +460,87 @@ cfcnfg_add_phy_layer(struct cfcnfg *cnfg, enum cfcnfg_phy_type phy_type,
 		layer_set_dn(frml, phy_layer);
 		layer_set_up(phy_layer, frml);
 	}
+
+	list_add_rcu(&phyinfo->node, &cnfg->phys);
+out:
+	mutex_unlock(&cnfg->lock);
 }
 EXPORT_SYMBOL(cfcnfg_add_phy_layer);
 
+int cfcnfg_set_phy_state(struct cfcnfg *cnfg, struct cflayer *phy_layer,
+		bool up)
+{
+	struct cfcnfg_phyinfo *phyinfo;
+
+	rcu_read_lock();
+	phyinfo = cfcnfg_get_phyinfo_rcu(cnfg, phy_layer->id);
+	if (phyinfo == NULL) {
+		rcu_read_unlock();
+		return -ENODEV;
+	}
+
+	if (phyinfo->up == up) {
+		rcu_read_unlock();
+		return 0;
+	}
+	phyinfo->up = up;
+
+	if (up) {
+		cffrml_hold(phyinfo->frm_layer);
+		cfmuxl_set_dnlayer(cnfg->mux, phyinfo->frm_layer,
+					phy_layer->id);
+	} else {
+		cfmuxl_remove_dnlayer(cnfg->mux, phy_layer->id);
+		cffrml_put(phyinfo->frm_layer);
+	}
+
+	rcu_read_unlock();
+	return 0;
+}
+EXPORT_SYMBOL(cfcnfg_set_phy_state);
+
 int cfcnfg_del_phy_layer(struct cfcnfg *cnfg, struct cflayer *phy_layer)
 {
 	struct cflayer *frml, *frml_dn;
 	u16 phyid;
+	struct cfcnfg_phyinfo *phyinfo;
+
+	might_sleep();
+
+	mutex_lock(&cnfg->lock);
+
 	phyid = phy_layer->id;
-	caif_assert(phyid == cnfg->phy_layers[phyid].id);
-	caif_assert(phy_layer == cnfg->phy_layers[phyid].phy_layer);
+	phyinfo = cfcnfg_get_phyinfo_rcu(cnfg, phyid);
+
+	if (phyinfo == NULL)
+		return 0;
+	caif_assert(phyid == phyinfo->id);
+	caif_assert(phy_layer == phyinfo->phy_layer);
 	caif_assert(phy_layer->id == phyid);
-	caif_assert(cnfg->phy_layers[phyid].frm_layer->id == phyid);
+	caif_assert(phyinfo->frm_layer->id == phyid);
+
+	list_del_rcu(&phyinfo->node);
+	synchronize_rcu();
 
-	memset(&cnfg->phy_layers[phy_layer->id], 0,
-	       sizeof(struct cfcnfg_phyinfo));
-	frml = cfmuxl_remove_dnlayer(cnfg->mux, phy_layer->id);
+	frml = phyinfo->frm_layer;
 	frml_dn = frml->dn;
 	cffrml_set_uplayer(frml, NULL);
 	cffrml_set_dnlayer(frml, NULL);
-	kfree(frml);
-
 	if (phy_layer != frml_dn) {
 		layer_set_up(frml_dn, NULL);
 		layer_set_dn(frml_dn, NULL);
-		kfree(frml_dn);
 	}
 	layer_set_up(phy_layer, NULL);
+
+
+
+	if (phyinfo->phy_layer != frml_dn)
+		kfree(frml_dn);
+
+	kfree(frml);
+	kfree(phyinfo);
+	mutex_unlock(&cnfg->lock);
+
 	return 0;
 }
 EXPORT_SYMBOL(cfcnfg_del_phy_layer);
-- 
1.7.1


^ permalink raw reply related

* Re: question about UFO behavior for bridge device
From: Ben Hutchings @ 2011-05-13 13:06 UTC (permalink / raw)
  To: Shan Wei; +Cc: Herbert Xu, netdev
In-Reply-To: <4DCCF803.5020805@cn.fujitsu.com>

On Fri, 2011-05-13 at 17:21 +0800, Shan Wei wrote:
> 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.

Right, it does that just before calling the driver's ndo_start_xmit().

> 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?

I assume that packet capturing is handled earlier in the transmit path.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* [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

* [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


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