Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 3/7] netfilter: nf_ct_helper: implement variable length helper private data
From: Jan Engelhardt @ 2012-06-04 13:06 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1338812485-4232-4-git-send-email-pablo@netfilter.org>

On Monday 2012-06-04 14:21, pablo@netfilter.org wrote:

>+static inline void *nfct_help_data(const struct nf_conn *ct)
>+{
>+	struct nf_conn_help *help;
>+
>+	help = nf_ct_ext_find(ct, NF_CT_EXT_HELPER);
>+
>+	return (void *)&help->data;
>+}

I think you wanted

	return help->data;

here. Remember that help->data is of type char[0] which is
convertible to char* - which is what you want,
while adding an extra & would turn that into the undesired (char[0])*.



>@@ -89,12 +59,13 @@ struct nf_conn_help {
> 	/* Helper. if any */
> 	struct nf_conntrack_helper __rcu *helper;
> 
>-	union nf_conntrack_help help;
>-
> 	struct hlist_head expectations;
> 
> 	/* Current number of expected connections */
> 	u8 expecting[NF_CT_MAX_EXPECT_CLASSES];
>+
>+	/* private helper information. */
>+	char data[0];

There is a now-standardized notation:

	char data[];



>@@ -218,13 +221,13 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl,
> 	}
> 
> 	if (help == NULL) {
>-		help = nf_ct_helper_ext_add(ct, flags);
>+		help = nf_ct_helper_ext_add(ct, helper, flags);
> 		if (help == NULL) {
> 			ret = -ENOMEM;
> 			goto out;
> 		}
> 	} else {
>-		memset(&help->help, 0, sizeof(help->help));
>+		memset(&help->data, 0, sizeof(helper->data_len));
> 	}

memset(help->data, 0, sizeof(helper->data_len));

>index 6f4b00a..30f5e12 100644
>--- a/net/netfilter/nf_conntrack_netlink.c
>+++ b/net/netfilter/nf_conntrack_netlink.c
>@@ -1218,7 +1218,7 @@ ctnetlink_change_helper(struct nf_conn *ct, const struct nlattr * const cda[])
> 		if (help->helper)
> 			return -EBUSY;
> 		/* need to zero data of old helper */
>-		memset(&help->help, 0, sizeof(help->help));
>+		memset(&help->data, 0, help->helper->data_len);

Here too.. memset(help->data,...

^ permalink raw reply

* Re: [PATCH net-next] net: netdev_alloc_skb() use build_skb()
From: Eric Dumazet @ 2012-06-04 13:06 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Willy Tarreau, David Miller, netdev
In-Reply-To: <20120604123738.GA28992@redhat.com>

On Mon, 2012-06-04 at 15:37 +0300, Michael S. Tsirkin wrote:
> On Thu, May 17, 2012 at 07:34:16PM +0200, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > Please note I havent tested yet this patch, lacking hardware for this.
> > 
> > (tg3/bnx2/bnx2x use build_skb, r8169 does a copy of incoming frames,
> > ixgbe uses fragments...)
> 
> virtio-net uses netdev_alloc_skb but maybe it should call
> build_skb instead?
> 
> Also, it's not uncommon for drivers to copy short packets out to be able
> to reuse pages.  virtio does this but I am guessing the logic is not
> really virtio specific.
> 
> We could do
> 	if (len < GOOD_COPY_LEN)
> 		netdev_alloc_skb
> 		memmov
> 	else
> 		build_skb
> 
> but maybe it makes sense to put this logic in build_skb?
> 
> 

I am not sure to understand the question.

If virtio-net uses netdev_alloc_skb(), all is good, you have nothing to
change.

build_skb() is for drivers that allocate the memory to hold frame, and
wait for NIC completion before allocating/populating the skb itself.

^ permalink raw reply

* Re: [PATCH 3/7] netfilter: nf_ct_helper: implement variable length helper private data
From: Joe Perches @ 2012-06-04 13:09 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: pablo, netfilter-devel, netdev
In-Reply-To: <alpine.LNX.2.01.1206041456480.16684@frira.zrqbmnf.qr>

On Mon, 2012-06-04 at 15:06 +0200, Jan Engelhardt wrote:
> On Monday 2012-06-04 14:21, pablo@netfilter.org wrote:

> >@@ -218,13 +221,13 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl,
> > 	}
> > 
> > 	if (help == NULL) {
> >-		help = nf_ct_helper_ext_add(ct, flags);
> >+		help = nf_ct_helper_ext_add(ct, helper, flags);
> > 		if (help == NULL) {
> > 			ret = -ENOMEM;
> > 			goto out;
> > 		}
> > 	} else {
> >-		memset(&help->help, 0, sizeof(help->help));
> >+		memset(&help->data, 0, sizeof(helper->data_len));
> > 	}
> 
> memset(help->data, 0, sizeof(helper->data_len));

	memset(help->data, 0, helper->data_len);



^ permalink raw reply

* Re: [PATCH 3/7] netfilter: nf_ct_helper: implement variable length helper private data
From: Jan Engelhardt @ 2012-06-04 13:16 UTC (permalink / raw)
  To: Joe Perches; +Cc: pablo, netfilter-devel, netdev
In-Reply-To: <1338815399.8574.10.camel@joe2Laptop>

On Monday 2012-06-04 15:09, Joe Perches wrote:

>On Mon, 2012-06-04 at 15:06 +0200, Jan Engelhardt wrote:
>> On Monday 2012-06-04 14:21, pablo@netfilter.org wrote:
>
>> >@@ -218,13 +221,13 @@ int __nf_ct_try_assign_helper(struct nf_conn *ct, struct nf_conn *tmpl,
>> > 	}
>> > 
>> > 	if (help == NULL) {
>> >-		help = nf_ct_helper_ext_add(ct, flags);
>> >+		help = nf_ct_helper_ext_add(ct, helper, flags);
>> > 		if (help == NULL) {
>> > 			ret = -ENOMEM;
>> > 			goto out;
>> > 		}
>> > 	} else {
>> >-		memset(&help->help, 0, sizeof(help->help));
>> >+		memset(&help->data, 0, sizeof(helper->data_len));
>> > 	}
>> 
>> memset(help->data, 0, sizeof(helper->data_len));
>
>	memset(help->data, 0, helper->data_len);

I knew this looked suspect. With so many "sizeof"s, this spot was 
starting to look like a "mine is bigger" competition.

^ permalink raw reply

* [PATCH RFC] c_can_pci: generic module for c_can on PCI
From: Federico Vaga @ 2012-06-04 13:32 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde
  Cc: Federico Vaga, Giancarlo Asnaghi, Alan Cox, Alessandro Rubini,
	linux-can, netdev, linux-kernel
In-Reply-To: <1338816766-7089-1-git-send-email-federico.vaga@gmail.com>

Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
Cc: Alan Cox <alan@linux.intel.com>
---
 drivers/net/can/c_can/Kconfig     |   11 +-
 drivers/net/can/c_can/Makefile    |    1 +
 drivers/net/can/c_can/c_can_pci.c |  221 +++++++++++++++++++++++++++++++++++++
 3 files changed, 230 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/can/c_can/c_can_pci.c

diff --git a/drivers/net/can/c_can/Kconfig b/drivers/net/can/c_can/Kconfig
index ffb9773..74ef97d 100644
--- a/drivers/net/can/c_can/Kconfig
+++ b/drivers/net/can/c_can/Kconfig
@@ -2,14 +2,19 @@ menuconfig CAN_C_CAN
 	tristate "Bosch C_CAN devices"
 	depends on CAN_DEV && HAS_IOMEM
 
-if CAN_C_CAN
-
 config CAN_C_CAN_PLATFORM
 	tristate "Generic Platform Bus based C_CAN driver"
+	depends on CAN_C_CAN
 	---help---
 	  This driver adds support for the C_CAN chips connected to
 	  the "platform bus" (Linux abstraction for directly to the
 	  processor attached devices) which can be found on various
 	  boards from ST Microelectronics (http://www.st.com)
 	  like the SPEAr1310 and SPEAr320 evaluation boards.
-endif
+
+config CAN_C_CAN_PCI
+	tristate "Generic PCI Bus based C_CAN driver"
+	depends on CAN_C_CAN
+	---help---
+	  This driver adds support for the C_CAN chips connected to
+	  the PCI bus.
diff --git a/drivers/net/can/c_can/Makefile b/drivers/net/can/c_can/Makefile
index 9273f6d..ad1cc84 100644
--- a/drivers/net/can/c_can/Makefile
+++ b/drivers/net/can/c_can/Makefile
@@ -4,5 +4,6 @@
 
 obj-$(CONFIG_CAN_C_CAN) += c_can.o
 obj-$(CONFIG_CAN_C_CAN_PLATFORM) += c_can_platform.o
+obj-$(CONFIG_CAN_C_CAN_PCI) += c_can_pci.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/c_can/c_can_pci.c b/drivers/net/can/c_can/c_can_pci.c
new file mode 100644
index 0000000..b635375
--- /dev/null
+++ b/drivers/net/can/c_can/c_can_pci.c
@@ -0,0 +1,221 @@
+/*
+ * Platform CAN bus driver for Bosch C_CAN controller
+ *
+ * Copyright (C) 2012 Federico Vaga <federico.vaga@gmail.com>
+  *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/clk.h>
+#include <linux/pci.h>
+#include <linux/can/dev.h>
+
+#include "c_can.h"
+
+enum c_can_pci_reg_align {
+	C_CAN_REG_ALIGN_16,
+	C_CAN_REG_ALIGN_32,
+};
+
+struct c_can_pci_data {
+	unsigned int reg_align;	/* Set the register alignment in the memory */
+	unsigned int freq;	/* Set the frequency if clk is not usable */
+};
+
+/*
+ * 16-bit c_can registers can be arranged differently in the memory
+ * architecture of different implementations. For example: 16-bit
+ * registers can be aligned to a 16-bit boundary or 32-bit boundary etc.
+ * Handle the same by providing a common read/write interface.
+ */
+static u16 c_can_pci_read_reg_aligned_to_16bit(struct c_can_priv *priv,
+						void *reg)
+{
+	return readw(reg);
+}
+
+static void c_can_pci_write_reg_aligned_to_16bit(struct c_can_priv *priv,
+						void *reg, u16 val)
+{
+	writew(val, reg);
+}
+
+static u16 c_can_pci_read_reg_aligned_to_32bit(struct c_can_priv *priv,
+						void *reg)
+{
+	return readw(reg + (long)reg - (long)priv->regs);
+}
+
+static void c_can_pci_write_reg_aligned_to_32bit(struct c_can_priv *priv,
+						void *reg, u16 val)
+{
+	writew(val, reg + (long)reg - (long)priv->regs);
+}
+
+static int __devinit c_can_pci_probe(struct pci_dev *pdev,
+				     const struct pci_device_id *ent)
+{
+	struct c_can_pci_data *c_can_pci_data = (void *)ent->driver_data;
+	struct c_can_priv *priv;
+	struct net_device *dev;
+	void __iomem *addr;
+	struct clk *clk;
+	int ret;
+
+	ret = pci_enable_device(pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "pci_enable_device FAILED\n");
+		goto out;
+	}
+
+	ret = pci_request_regions(pdev, KBUILD_MODNAME);
+	if (ret) {
+		dev_err(&pdev->dev, "pci_request_regions FAILED\n");
+		goto out_disable_device;
+	}
+
+	pci_set_master(pdev);
+	pci_enable_msi(pdev);
+
+	addr = pci_iomap(pdev, 0, pci_resource_len(pdev, 0));
+	if (!addr) {
+		dev_err(&pdev->dev,
+			"device has no PCI memory resources, "
+			"failing adapter\n");
+		ret = -ENOMEM;
+		goto out_release_regions;
+	}
+
+	/* allocate the c_can device */
+	dev = alloc_c_can_dev();
+	if (!dev) {
+		ret = -ENOMEM;
+		goto out_iounmap;
+	}
+
+	priv = netdev_priv(dev);
+	pci_set_drvdata(pdev, dev);
+	SET_NETDEV_DEV(dev, &pdev->dev);
+
+	dev->irq = pdev->irq;
+	priv->regs = addr;
+
+	if (!c_can_pci_data->freq) {
+		/* get the appropriate clk */
+		clk = clk_get(&pdev->dev, NULL);
+		if (IS_ERR(clk)) {
+			dev_err(&pdev->dev, "no clock defined\n");
+			ret = -ENODEV;
+			goto out_free_c_can;
+		}
+		priv->can.clock.freq = clk_get_rate(clk);
+		priv->priv = clk;
+	} else {
+		priv->can.clock.freq = c_can_pci_data->freq;
+		priv->priv = NULL;
+	}
+
+	switch (c_can_pci_data->reg_align) {
+	case C_CAN_REG_ALIGN_32:
+		priv->read_reg = c_can_pci_read_reg_aligned_to_32bit;
+		priv->write_reg = c_can_pci_write_reg_aligned_to_32bit;
+		break;
+	case C_CAN_REG_ALIGN_16:
+	default:
+		priv->read_reg = c_can_pci_read_reg_aligned_to_16bit;
+		priv->write_reg = c_can_pci_write_reg_aligned_to_16bit;
+		break;
+	}
+
+	ret = register_c_can_dev(dev);
+	if (ret) {
+		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
+			KBUILD_MODNAME, ret);
+		goto out_free_clock;
+	}
+
+	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
+		 KBUILD_MODNAME, priv->regs, dev->irq);
+
+	return 0;
+
+out_free_clock:
+	if (!priv->priv)
+		clk_put(priv->priv);
+out_free_c_can:
+	pci_set_drvdata(pdev, NULL);
+	free_c_can_dev(dev);
+out_iounmap:
+	pci_iounmap(pdev, addr);
+out_release_regions:
+	pci_disable_msi(pdev);
+	pci_clear_master(pdev);
+	pci_release_regions(pdev);
+out_disable_device:
+	/*
+	 * do not call pci_disable_device on sta2x11 because it
+	 * break all other Bus masters on this EP
+	 */
+	if(pdev->vendor == PCI_VENDOR_ID_STMICRO &&
+	   pdev->device == PCI_DEVICE_ID_STMICRO_CAN)
+		goto out;
+	pci_disable_device(pdev);
+out:
+	return ret;
+}
+
+static void __devexit c_can_pci_remove(struct pci_dev *pdev)
+{
+	struct net_device *dev = pci_get_drvdata(pdev);
+	struct c_can_priv *priv = netdev_priv(dev);
+
+	pci_set_drvdata(pdev, NULL);
+	free_c_can_dev(dev);
+	if (!priv->priv)
+		clk_put(priv->priv);
+	pci_iounmap(pdev, priv->regs);
+	pci_disable_msi(pdev);
+	pci_clear_master(pdev);
+	pci_release_regions(pdev);
+	/*
+	 * do not call pci_disable_device on sta2x11 because it
+	 * break all other Bus masters on this EP
+	 */
+	if(pdev->vendor == PCI_VENDOR_ID_STMICRO &&
+	   pdev->device == PCI_DEVICE_ID_STMICRO_CAN)
+		return;
+	pci_disable_device(pdev);
+}
+
+static struct c_can_pci_data c_can_sta2x11= {
+	.reg_align = C_CAN_REG_ALIGN_32,
+	.freq = 52000000, /* 52 Mhz */
+};
+
+#define C_CAN_ID(_vend, _dev, _driverdata) {		\
+	PCI_DEVICE(_vend, _dev),			\
+	.driver_data = (unsigned long)&_driverdata,	\
+}
+DEFINE_PCI_DEVICE_TABLE(c_can_pci_tbl) = {
+	C_CAN_ID(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_CAN,
+		 c_can_sta2x11),
+	{},
+};
+static struct pci_driver sta2x11_pci_driver = {
+	.name = KBUILD_MODNAME,
+	.id_table = c_can_pci_tbl,
+	.probe = c_can_pci_probe,
+	.remove = __devexit_p(c_can_pci_remove),
+};
+
+module_pci_driver(sta2x11_pci_driver);
+
+MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
+MODULE_LICENSE("GPL V2");
+MODULE_DESCRIPTION("PCI CAN bus driver for Bosch C_CAN controller");
+MODULE_DEVICE_TABLE(pci, c_can_pci_tbl);
-- 
1.7.10.2

^ permalink raw reply related

* generic module for c-can on pci
From: Federico Vaga @ 2012-06-04 13:32 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde
  Cc: Federico Vaga, Giancarlo Asnaghi, Alan Cox, Alessandro Rubini,
	linux-can, netdev, linux-kernel
In-Reply-To: <4FC135C6.5030206@grandegger.com>


As suggested I developed a generic module for C-CAN
on PCI. Probably I will do some changes about our
specific board, but I think that the module is generic
enough.

^ permalink raw reply

* Re: [PATCH 4/7] netfilter: add glue code to integrate nfnetlink_queue and ctnetlink
From: Jan Engelhardt @ 2012-06-04 13:38 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1338812485-4232-5-git-send-email-pablo@netfilter.org>


On Monday 2012-06-04 14:21, pablo@netfilter.org wrote:
>+static int
>+ctnetlink_nfqueue_parse(const struct nlattr *attr, struct nf_conn *ct)
>+{
>+	const struct nlattr * const cda[CTA_MAX+1];

I suppose you wrote that because the same appears in function
headers/signatures

	void foo(const struct nlattr *const tb[]) { ... }

But there, it is actually equal to

	void foo(const struct nlattr *const *tb) { ... }

In either case, tb is writable. IMHO, [] should be avoided in
signatures to avoid self-confusion, as it seemed to occur in your
case, where cda is - unlike tb - really marked const.
You likely wanted

	const struct nlattr *cda[CTA_MAX+1];

>+       nla_parse_nested((struct nlattr **)cda, CTA_MAX, attr, ct_nla_policy);



^ permalink raw reply

* Re: [PATCH net-next] net: netdev_alloc_skb() use build_skb()
From: Michael S. Tsirkin @ 2012-06-04 13:41 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Willy Tarreau, David Miller, netdev
In-Reply-To: <1338815213.2760.1806.camel@edumazet-glaptop>

On Mon, Jun 04, 2012 at 03:06:53PM +0200, Eric Dumazet wrote:
> On Mon, 2012-06-04 at 15:37 +0300, Michael S. Tsirkin wrote:
> > On Thu, May 17, 2012 at 07:34:16PM +0200, Eric Dumazet wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > > 
> > > Please note I havent tested yet this patch, lacking hardware for this.
> > > 
> > > (tg3/bnx2/bnx2x use build_skb, r8169 does a copy of incoming frames,
> > > ixgbe uses fragments...)
> > 
> > virtio-net uses netdev_alloc_skb but maybe it should call
> > build_skb instead?
> > 
> > Also, it's not uncommon for drivers to copy short packets out to be able
> > to reuse pages.  virtio does this but I am guessing the logic is not
> > really virtio specific.
> > 
> > We could do
> > 	if (len < GOOD_COPY_LEN)
> > 		netdev_alloc_skb
> > 		memmov
> > 	else
> > 		build_skb
> > 
> > but maybe it makes sense to put this logic in build_skb?
> > 
> > 
> 
> I am not sure to understand the question.
> 
> If virtio-net uses netdev_alloc_skb(), all is good, you have nothing to
> change.
> 
> build_skb() is for drivers that allocate the memory to hold frame, and
> wait for NIC completion before allocating/populating the skb itself.
> 


This is generally what virtio does, take a look:
page_to_skb fills the first fragment and receive_mergeable fills the
rest (other modes are for legacy hardware).

The way hypervisor now works is this (we call it mergeable buffers):

- pages are passed to hardware
- hypervisor puts virtio specific stuff in first 12 bytes
  on first page
- following this, the rest of the first page and all following
  pages have data

The driver gets the 1st page, allocates the skb, copies out the 12 byte
header and copies the first 128 bytes of data into skb.
The rest if any is populated by the pages.

So I guess I'm asking for advice, would it make sense to switch to build_skb
and how best to handle the data copying above? Maybe it would help
if we changed the hypervisor to write the 12 bytes separately?
  


-- 
MST

^ permalink raw reply

* [PATCH net-next] sock_diag: add SK_MEMINFO_BACKLOG
From: Eric Dumazet @ 2012-06-04 13:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

Adding socket backlog len in INET_DIAG_SKMEMINFO is really useful to
diagnose various TCP problems.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/sock_diag.h |    1 +
 net/core/sock_diag.c      |    1 +
 2 files changed, 2 insertions(+)

diff --git a/include/linux/sock_diag.h b/include/linux/sock_diag.h
index db4bae7..6793fac 100644
--- a/include/linux/sock_diag.h
+++ b/include/linux/sock_diag.h
@@ -18,6 +18,7 @@ enum {
 	SK_MEMINFO_FWD_ALLOC,
 	SK_MEMINFO_WMEM_QUEUED,
 	SK_MEMINFO_OPTMEM,
+	SK_MEMINFO_BACKLOG,
 
 	SK_MEMINFO_VARS,
 };
diff --git a/net/core/sock_diag.c b/net/core/sock_diag.c
index 5fd1467..0d934ce 100644
--- a/net/core/sock_diag.c
+++ b/net/core/sock_diag.c
@@ -46,6 +46,7 @@ int sock_diag_put_meminfo(struct sock *sk, struct sk_buff *skb, int attrtype)
 	mem[SK_MEMINFO_FWD_ALLOC] = sk->sk_forward_alloc;
 	mem[SK_MEMINFO_WMEM_QUEUED] = sk->sk_wmem_queued;
 	mem[SK_MEMINFO_OPTMEM] = atomic_read(&sk->sk_omem_alloc);
+	mem[SK_MEMINFO_BACKLOG] = sk->sk_backlog.len;
 
 	return 0;
 

^ permalink raw reply related

* Re: [PATCH net-next] net: netdev_alloc_skb() use build_skb()
From: Eric Dumazet @ 2012-06-04 14:01 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Willy Tarreau, David Miller, netdev
In-Reply-To: <20120604134138.GA29814@redhat.com>

On Mon, 2012-06-04 at 16:41 +0300, Michael S. Tsirkin wrote:

> This is generally what virtio does, take a look:
> page_to_skb fills the first fragment and receive_mergeable fills the
> rest (other modes are for legacy hardware).
> 
> The way hypervisor now works is this (we call it mergeable buffers):
> 
> - pages are passed to hardware
> - hypervisor puts virtio specific stuff in first 12 bytes
>   on first page
> - following this, the rest of the first page and all following
>   pages have data
> 
> The driver gets the 1st page, allocates the skb, copies out the 12 byte
> header and copies the first 128 bytes of data into skb.
> The rest if any is populated by the pages.
> 
> So I guess I'm asking for advice, would it make sense to switch to build_skb
> and how best to handle the data copying above? Maybe it would help
> if we changed the hypervisor to write the 12 bytes separately?
>   

Thanks for these details.

Not sure 12 bytes of headroom would be enough (instead of the
NET_SKB_PAD reserved in netdev_alloc_skb_ip_align(), but what could be
done indeed is to use the first page as the skb->head, so using
build_skb() indeed, removing one fragment, one (small) copy and one
{put|get}_page() pair.

^ permalink raw reply

* Re: [PATCH 7/7] netfilter: add user-space connection tracking helper infrastructure
From: Jan Engelhardt @ 2012-06-04 14:04 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1338812485-4232-8-git-send-email-pablo@netfilter.org>


On Monday 2012-06-04 14:21, pablo@netfilter.org wrote:
>+static int
>+nfnl_cthelper_from_nlattr(struct nlattr *attr, struct nf_conn *ct)
>+{
>+	const struct nf_conn_help *help = nfct_help(ct);
>+
>+	if (help->helper->data_len == 0)
>+		return -EINVAL;
>+
>+	memcpy(&help->data, nla_data(attr), help->helper->data_len);

memcpy(help->data, ...)

>+static int
>+nfnl_cthelper_to_nlattr(struct sk_buff *skb, const struct nf_conn *ct)
>+{
>+	const struct nf_conn_help *help = nfct_help(ct);
>+
>+	if (help->helper->data_len &&
>+	    nla_put(skb, CTA_HELP_INFO, help->helper->data_len, &help->data))
>+		goto nla_put_failure;

help->data

^ permalink raw reply

* Re: [PATCH RFC] c_can_pci: generic module for c_can on PCI
From: Marc Kleine-Budde @ 2012-06-04 14:04 UTC (permalink / raw)
  To: Federico Vaga
  Cc: Wolfgang Grandegger, Giancarlo Asnaghi, Alan Cox,
	Alessandro Rubini, linux-can, netdev, linux-kernel
In-Reply-To: <1338816766-7089-2-git-send-email-federico.vaga@gmail.com>

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

On 06/04/2012 03:32 PM, Federico Vaga wrote:
> Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
> Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
> Cc: Alan Cox <alan@linux.intel.com>

Please port you driver to the recent c_can changes. Use the c_can branch
of the linux-can-next repo[1] as base for your work. You have to rework
the register access function. Please have a look if there are devm_
variants for the registration/mapping of the pci and clock.

[1] https://gitorious.org/linux-can/linux-can-next

More comments inline. Marc

> ---
>  drivers/net/can/c_can/Kconfig     |   11 +-
>  drivers/net/can/c_can/Makefile    |    1 +
>  drivers/net/can/c_can/c_can_pci.c |  221 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 230 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/net/can/c_can/c_can_pci.c
> 
> diff --git a/drivers/net/can/c_can/Kconfig b/drivers/net/can/c_can/Kconfig
> index ffb9773..74ef97d 100644
> --- a/drivers/net/can/c_can/Kconfig
> +++ b/drivers/net/can/c_can/Kconfig
> @@ -2,14 +2,19 @@ menuconfig CAN_C_CAN
>  	tristate "Bosch C_CAN devices"
>  	depends on CAN_DEV && HAS_IOMEM
>  
> -if CAN_C_CAN

please keep the if CAN_C_CAN...

> -
>  config CAN_C_CAN_PLATFORM
>  	tristate "Generic Platform Bus based C_CAN driver"
> +	depends on CAN_C_CAN

...then you don't have to add the depends on here.

>  	---help---
>  	  This driver adds support for the C_CAN chips connected to
>  	  the "platform bus" (Linux abstraction for directly to the
>  	  processor attached devices) which can be found on various
>  	  boards from ST Microelectronics (http://www.st.com)
>  	  like the SPEAr1310 and SPEAr320 evaluation boards.
> -endif

... Just move you pci driver inside the if...endif block...
> +
> +config CAN_C_CAN_PCI
> +	tristate "Generic PCI Bus based C_CAN driver"
> +	depends on CAN_C_CAN

...and remove the depends on CAN_C_CAN. You probably have to add a
depends on PCI.

> +	---help---
> +	  This driver adds support for the C_CAN chips connected to
> +	  the PCI bus.
> diff --git a/drivers/net/can/c_can/Makefile b/drivers/net/can/c_can/Makefile
> index 9273f6d..ad1cc84 100644
> --- a/drivers/net/can/c_can/Makefile
> +++ b/drivers/net/can/c_can/Makefile
> @@ -4,5 +4,6 @@
>  
>  obj-$(CONFIG_CAN_C_CAN) += c_can.o
>  obj-$(CONFIG_CAN_C_CAN_PLATFORM) += c_can_platform.o
> +obj-$(CONFIG_CAN_C_CAN_PCI) += c_can_pci.o
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/c_can/c_can_pci.c b/drivers/net/can/c_can/c_can_pci.c
> new file mode 100644
> index 0000000..b635375
> --- /dev/null
> +++ b/drivers/net/can/c_can/c_can_pci.c
> @@ -0,0 +1,221 @@
> +/*
> + * Platform CAN bus driver for Bosch C_CAN controller
> + *
> + * Copyright (C) 2012 Federico Vaga <federico.vaga@gmail.com>
> +  *
   ^^^ double space :)

> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/clk.h>
> +#include <linux/pci.h>
> +#include <linux/can/dev.h>
> +
> +#include "c_can.h"
> +
> +enum c_can_pci_reg_align {
> +	C_CAN_REG_ALIGN_16,
> +	C_CAN_REG_ALIGN_32,
> +};
> +
> +struct c_can_pci_data {
> +	unsigned int reg_align;	/* Set the register alignment in the memory */
        ^^^^^^^^^^^^
use the enum you defined above.

> +	unsigned int freq;	/* Set the frequency if clk is not usable */
> +};
> +
> +/*
> + * 16-bit c_can registers can be arranged differently in the memory
> + * architecture of different implementations. For example: 16-bit
> + * registers can be aligned to a 16-bit boundary or 32-bit boundary etc.
> + * Handle the same by providing a common read/write interface.
> + */
> +static u16 c_can_pci_read_reg_aligned_to_16bit(struct c_can_priv *priv,
> +						void *reg)
> +{
> +	return readw(reg);
> +}
> +
> +static void c_can_pci_write_reg_aligned_to_16bit(struct c_can_priv *priv,
> +						void *reg, u16 val)
> +{
> +	writew(val, reg);
> +}
> +
> +static u16 c_can_pci_read_reg_aligned_to_32bit(struct c_can_priv *priv,
> +						void *reg)
> +{
> +	return readw(reg + (long)reg - (long)priv->regs);
> +}
> +
> +static void c_can_pci_write_reg_aligned_to_32bit(struct c_can_priv *priv,
> +						void *reg, u16 val)
> +{
> +	writew(val, reg + (long)reg - (long)priv->regs);
> +}
> +
> +static int __devinit c_can_pci_probe(struct pci_dev *pdev,
> +				     const struct pci_device_id *ent)
> +{
> +	struct c_can_pci_data *c_can_pci_data = (void *)ent->driver_data;
> +	struct c_can_priv *priv;
> +	struct net_device *dev;
> +	void __iomem *addr;
> +	struct clk *clk;
> +	int ret;
> +
> +	ret = pci_enable_device(pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "pci_enable_device FAILED\n");
> +		goto out;
> +	}
> +
> +	ret = pci_request_regions(pdev, KBUILD_MODNAME);
> +	if (ret) {
> +		dev_err(&pdev->dev, "pci_request_regions FAILED\n");
> +		goto out_disable_device;
> +	}
> +
> +	pci_set_master(pdev);
> +	pci_enable_msi(pdev);
> +
> +	addr = pci_iomap(pdev, 0, pci_resource_len(pdev, 0));
> +	if (!addr) {
> +		dev_err(&pdev->dev,
> +			"device has no PCI memory resources, "
> +			"failing adapter\n");
> +		ret = -ENOMEM;
> +		goto out_release_regions;
> +	}
> +
> +	/* allocate the c_can device */
> +	dev = alloc_c_can_dev();
> +	if (!dev) {
> +		ret = -ENOMEM;
> +		goto out_iounmap;
> +	}
> +
> +	priv = netdev_priv(dev);
> +	pci_set_drvdata(pdev, dev);
> +	SET_NETDEV_DEV(dev, &pdev->dev);
> +
> +	dev->irq = pdev->irq;
> +	priv->regs = addr;
> +
> +	if (!c_can_pci_data->freq) {
> +		/* get the appropriate clk */
> +		clk = clk_get(&pdev->dev, NULL);
> +		if (IS_ERR(clk)) {
> +			dev_err(&pdev->dev, "no clock defined\n");
> +			ret = -ENODEV;
> +			goto out_free_c_can;
> +		}
> +		priv->can.clock.freq = clk_get_rate(clk);
> +		priv->priv = clk;
> +	} else {
> +		priv->can.clock.freq = c_can_pci_data->freq;
> +		priv->priv = NULL;
> +	}
> +
> +	switch (c_can_pci_data->reg_align) {
> +	case C_CAN_REG_ALIGN_32:
> +		priv->read_reg = c_can_pci_read_reg_aligned_to_32bit;
> +		priv->write_reg = c_can_pci_write_reg_aligned_to_32bit;
> +		break;
> +	case C_CAN_REG_ALIGN_16:
> +	default:
> +		priv->read_reg = c_can_pci_read_reg_aligned_to_16bit;
> +		priv->write_reg = c_can_pci_write_reg_aligned_to_16bit;
> +		break;
> +	}
> +
> +	ret = register_c_can_dev(dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
> +			KBUILD_MODNAME, ret);
> +		goto out_free_clock;
> +	}
> +
> +	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> +		 KBUILD_MODNAME, priv->regs, dev->irq);
> +
> +	return 0;
> +
> +out_free_clock:
> +	if (!priv->priv)
           ^^^

looks fishy

> +		clk_put(priv->priv);
> +out_free_c_can:
> +	pci_set_drvdata(pdev, NULL);
> +	free_c_can_dev(dev);
> +out_iounmap:
> +	pci_iounmap(pdev, addr);
> +out_release_regions:
> +	pci_disable_msi(pdev);
> +	pci_clear_master(pdev);
> +	pci_release_regions(pdev);
> +out_disable_device:
> +	/*
> +	 * do not call pci_disable_device on sta2x11 because it
> +	 * break all other Bus masters on this EP
> +	 */
> +	if(pdev->vendor == PCI_VENDOR_ID_STMICRO &&
> +	   pdev->device == PCI_DEVICE_ID_STMICRO_CAN)
> +		goto out;
> +	pci_disable_device(pdev);
> +out:
> +	return ret;
> +}
> +
> +static void __devexit c_can_pci_remove(struct pci_dev *pdev)
> +{
> +	struct net_device *dev = pci_get_drvdata(pdev);
> +	struct c_can_priv *priv = netdev_priv(dev);
> +
> +	pci_set_drvdata(pdev, NULL);
> +	free_c_can_dev(dev);
> +	if (!priv->priv)
dito
> +		clk_put(priv->priv);
> +	pci_iounmap(pdev, priv->regs);
> +	pci_disable_msi(pdev);
> +	pci_clear_master(pdev);
> +	pci_release_regions(pdev);
> +	/*
> +	 * do not call pci_disable_device on sta2x11 because it
> +	 * break all other Bus masters on this EP
> +	 */
> +	if(pdev->vendor == PCI_VENDOR_ID_STMICRO &&
> +	   pdev->device == PCI_DEVICE_ID_STMICRO_CAN)
> +		return;
> +	pci_disable_device(pdev);
> +}
> +
> +static struct c_can_pci_data c_can_sta2x11= {
> +	.reg_align = C_CAN_REG_ALIGN_32,
> +	.freq = 52000000, /* 52 Mhz */
> +};
> +
> +#define C_CAN_ID(_vend, _dev, _driverdata) {		\
> +	PCI_DEVICE(_vend, _dev),			\
> +	.driver_data = (unsigned long)&_driverdata,	\
> +}
> +DEFINE_PCI_DEVICE_TABLE(c_can_pci_tbl) = {
^^^^

static?

> +	C_CAN_ID(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_CAN,
> +		 c_can_sta2x11),
> +	{},
> +};
> +static struct pci_driver sta2x11_pci_driver = {
> +	.name = KBUILD_MODNAME,
> +	.id_table = c_can_pci_tbl,
> +	.probe = c_can_pci_probe,
> +	.remove = __devexit_p(c_can_pci_remove),
> +};
> +
> +module_pci_driver(sta2x11_pci_driver);
> +
> +MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
> +MODULE_LICENSE("GPL V2");

IIRC, the correct case is "GPL v2"

> +MODULE_DESCRIPTION("PCI CAN bus driver for Bosch C_CAN controller");
> +MODULE_DEVICE_TABLE(pci, c_can_pci_tbl);


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] net: netdev_alloc_skb() use build_skb()
From: Eric Dumazet @ 2012-06-04 14:09 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Willy Tarreau, David Miller, netdev
In-Reply-To: <1338818501.2760.1821.camel@edumazet-glaptop>

On Mon, 2012-06-04 at 16:01 +0200, Eric Dumazet wrote:

> Not sure 12 bytes of headroom would be enough (instead of the
> NET_SKB_PAD reserved in netdev_alloc_skb_ip_align(), but what could be
> done indeed is to use the first page as the skb->head, so using
> build_skb() indeed, removing one fragment, one (small) copy and one
> {put|get}_page() pair.

It would also avoid 'pulling' tcp data payload in linear part.

page_to_skb() does :

copy = len;
if (copy > skb_tailroom(skb))
	copy = skb_tailroom(skb);
memcpy(skb_put(skb, copy), p, copy);

This means GRO or TCP coalescing (or splice()) has to handle two
segments to fetch data.

^ permalink raw reply

* Re: [PATCH net-next] net: netdev_alloc_skb() use build_skb()
From: Michael S. Tsirkin @ 2012-06-04 14:17 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Willy Tarreau, David Miller, netdev
In-Reply-To: <1338818501.2760.1821.camel@edumazet-glaptop>

On Mon, Jun 04, 2012 at 04:01:41PM +0200, Eric Dumazet wrote:
> On Mon, 2012-06-04 at 16:41 +0300, Michael S. Tsirkin wrote:
> 
> > This is generally what virtio does, take a look:
> > page_to_skb fills the first fragment and receive_mergeable fills the
> > rest (other modes are for legacy hardware).
> > 
> > The way hypervisor now works is this (we call it mergeable buffers):
> > 
> > - pages are passed to hardware
> > - hypervisor puts virtio specific stuff in first 12 bytes
> >   on first page
> > - following this, the rest of the first page and all following
> >   pages have data
> > 
> > The driver gets the 1st page, allocates the skb, copies out the 12 byte
> > header and copies the first 128 bytes of data into skb.
> > The rest if any is populated by the pages.
> > 
> > So I guess I'm asking for advice, would it make sense to switch to build_skb
> > and how best to handle the data copying above? Maybe it would help
> > if we changed the hypervisor to write the 12 bytes separately?
> >   
> 
> Thanks for these details.
> 
> Not sure 12 bytes of headroom would be enough (instead of the
> NET_SKB_PAD reserved in netdev_alloc_skb_ip_align(), but what could be
> done indeed is to use the first page as the skb->head, so using
> build_skb() indeed, removing one fragment, one (small) copy and one
> {put|get}_page() pair.
> 

bnx2 and tg3 both do skb_reserve of at least NET_SKB_PAD
after build_skb. You are saying it's not a must?

Hmm so maybe we should teach the hypervisor to write data
out at an offset. Interesting.

Another question is about very small packets truesize.
build_skb sets truesize to frag_size but isn't
this too small? We keep the whole page around, no?

^ permalink raw reply

* [PATCH 0/1] net/hyperv: Use wait_event on outstanding sends during device removal
From: Haiyang Zhang @ 2012-06-04 14:35 UTC (permalink / raw)
  To: davem, netdev; +Cc: devel, haiyangz, olaf, linux-kernel

This patch is targeting net-next tree (when it's available for check in).

Haiyang Zhang (1):
  net/hyperv: Use wait_event on outstanding sends during device removal

 drivers/net/hyperv/hyperv_net.h |    1 +
 drivers/net/hyperv/netvsc.c     |   12 ++++++------
 2 files changed, 7 insertions(+), 6 deletions(-)

-- 
1.7.4.1

^ permalink raw reply

* [PATCH 1/1] net/hyperv: Use wait_event on outstanding sends during device removal
From: Haiyang Zhang @ 2012-06-04 14:35 UTC (permalink / raw)
  To: davem, netdev; +Cc: haiyangz, kys, olaf, linux-kernel, devel
In-Reply-To: <1338820532-2345-1-git-send-email-haiyangz@microsoft.com>

Change the busy-waiting/udelay to wait_event on outstanding sends.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>

---
 drivers/net/hyperv/hyperv_net.h |    1 +
 drivers/net/hyperv/netvsc.c     |   12 ++++++------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 4ffcd57..2857ab0 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -478,6 +478,7 @@ struct netvsc_device {
 	u32 nvsp_version;
 
 	atomic_t num_outstanding_sends;
+	wait_queue_head_t wait_drain;
 	bool start_remove;
 	bool destroy;
 	/*
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 8b91947..dee7b23e 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -42,6 +42,7 @@ static struct netvsc_device *alloc_net_device(struct hv_device *device)
 	if (!net_device)
 		return NULL;
 
+	init_waitqueue_head(&net_device->wait_drain);
 	net_device->start_remove = false;
 	net_device->destroy = false;
 	net_device->dev = device;
@@ -387,12 +388,8 @@ int netvsc_device_remove(struct hv_device *device)
 	spin_unlock_irqrestore(&device->channel->inbound_lock, flags);
 
 	/* Wait for all send completions */
-	while (atomic_read(&net_device->num_outstanding_sends)) {
-		dev_info(&device->device,
-			"waiting for %d requests to complete...\n",
-			atomic_read(&net_device->num_outstanding_sends));
-		udelay(100);
-	}
+	wait_event(net_device->wait_drain,
+		atomic_read(&net_device->num_outstanding_sends) == 0);
 
 	netvsc_disconnect_vsp(net_device);
 
@@ -486,6 +483,9 @@ static void netvsc_send_completion(struct hv_device *device,
 		num_outstanding_sends =
 			atomic_dec_return(&net_device->num_outstanding_sends);
 
+		if (net_device->destroy && num_outstanding_sends == 0)
+			wake_up(&net_device->wait_drain);
+
 		if (netif_queue_stopped(ndev) && !net_device->start_remove &&
 			(hv_ringbuf_avail_percent(&device->channel->outbound)
 			> RING_AVAIL_PERCENT_HIWATER ||
-- 
1.7.4.1

^ permalink raw reply related

* Re: [RFC PATCH v1 2/3] net: add VEPA, VEB bridge mode
From: Krishna Kumar2 @ 2012-06-04 14:59 UTC (permalink / raw)
  To: John Fastabend
  Cc: bhutchings, buytenh, eilong, eric.w.multanen, gregory.v.rose,
	hadi, jeffrey.t.kirsher, mst, netdev, shemminger, sri
In-Reply-To: <20120530030705.7443.22155.stgit@jf-dev1-dcblab>

John Fastabend <john.r.fastabend@intel.com> wrote on 05/30/2012 08:37:06
AM:

Some comments below:

> +static int rtnl_bridge_notify(struct net_device *dev, u16 flags)
> +{
> ...
> +		 if (!flags && master && master->netdev_ops->
ndo_bridge_getlink)
> +		 		 err = master->netdev_ops->ndo_bridge_getlink(skb,
0, 0, dev);
> +		 else if (dev->netdev_ops->ndo_bridge_getlink)
> +		 		 err = dev->netdev_ops->ndo_bridge_getlink(skb, 0,
0, dev);

I think you should do something like:

        if ((flags == BRIDGE_FLAGS_MASTER) && ...)
                ...

Also you could use BRIDGE_FLAGS_MASTER=1, SELF=2, and use
"if (flags & BRIDGE_FLAGS_MASTER)" for consistency?


+		 if (!err)
+		 		 err = rtnl_bridge_notify(dev, flags);

It is possible to return a reporting error even though
the operation succeeded. Maybe something that could be
done here to indicate that the operation succeeded, or
is that a TODO?

>  static int rtnl_bridge_setlink(struct sk_buff *skb, struct nlmsghdr
*nlh,
>                  void *arg)
>  {
..
> +   if (!flags && dev->master &&
> +       dev->master->netdev_ops->ndo_bridge_setlink)
> +      err = dev->master->netdev_ops->ndo_bridge_setlink(dev, nlh);
> +   else if ((flags & BRIDGE_FLAGS_SELF) &&
> +         dev->netdev_ops->ndo_bridge_setlink)

Same usage of MASTER here.

Thanks,
- KK

^ permalink raw reply

* Re: [PATCH net-next] net: netdev_alloc_skb() use build_skb()
From: Eric Dumazet @ 2012-06-04 15:01 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Willy Tarreau, David Miller, netdev
In-Reply-To: <20120604141731.GA30226@redhat.com>

On Mon, 2012-06-04 at 17:17 +0300, Michael S. Tsirkin wrote:

> bnx2 and tg3 both do skb_reserve of at least NET_SKB_PAD
> after build_skb. You are saying it's not a must?
> 

32 would be the minimum. NETS_SKB_PAD is using a cache line (64 bytes
on most x86 current cpus) to avoid using half a cache line.

> Hmm so maybe we should teach the hypervisor to write data
> out at an offset. Interesting.
> 
> Another question is about very small packets truesize.
> build_skb sets truesize to frag_size but isn't
> this too small? We keep the whole page around, no?

We keep one page per cpu, at most.

For example on MTU=1500 and PAGE_SIZE=4096, one page is splitted into
two fragments, of 1500 + NET_SKB_PAD + align(shared_info), so its good
enough (this is very close from 2048 'truesize')

But yes, for some uses (wifi for example), we might use a full page per
skb, yet underestimate skb->truesize. Hopefully we can track these uses
and fix them.

ath9k for example could be changed, to be able to reassemble up to 3
frags instead of 2 frags, ie extending what did commit
0d95521ea74735826cb2e28bebf6a07392c75bfa (ath9k: use split rx
buffers to get rid of order-1 skb allocations)

^ permalink raw reply

* Re: [PATCH net-next] tcp: tcp_make_synack() can use alloc_skb()
From: David Miller @ 2012-06-04 15:33 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1338789043.2760.1719.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 04 Jun 2012 07:50:43 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> There is no value using sock_wmalloc() in tcp_make_synack().
> 
> A listener socket only sends SYNACK packets, they are not queued in a
> socket queue, only in Qdisc and device layers, so the number of in
> flight packets is limited in these layers. We used sock_wmalloc() with
> the %force parameter set to 1 to ignore socket limits anyway.
> 
> This patch removes two atomic operations per SYNACK packet.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Looks good, applied.

^ permalink raw reply

* Re: [PATCH net-next] tcp: tcp_make_synack() consumes dst parameter
From: David Miller @ 2012-06-04 15:33 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1338791601.2760.1725.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 04 Jun 2012 08:33:21 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> tcp_make_synack() clones the dst, and callers release it.
> 
> We can avoid two atomic operations per SYNACK if tcp_make_synack()
> consumes dst instead of cloning it.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] net: use consume_skb() in place of kfree_skb()
From: David Miller @ 2012-06-04 15:33 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1338808639.2760.1802.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 04 Jun 2012 13:17:19 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> Remove some dropwatch/drop_monitor false positives.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] sock_diag: add SK_MEMINFO_BACKLOG
From: David Miller @ 2012-06-04 15:33 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1338817835.2760.1815.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 04 Jun 2012 15:50:35 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> Adding socket backlog len in INET_DIAG_SKMEMINFO is really useful to
> diagnose various TCP problems.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* [PATCH (net.git) 0/4] stmmac fixes for net.git.
From: Giuseppe CAVALLARO @ 2012-06-04 15:37 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro

This set of patches fixes some problem in the driver included
in net.git.
A patch fixes a problem in the driver when built as dynamic
module. Another one fixes some useless initialisations in the
rx/tx processes.

Giuseppe Cavallaro (4):
  stmmac: remove two useless initialisation
  stmmac: fix driver's doc when run kernel-doc script
  stmmac: update driver's doc
  stmmac: fix driver Kconfig when built as module

 Documentation/networking/stmmac.txt                |   44 +++++++++++--------
 drivers/net/ethernet/stmicro/stmmac/Kconfig        |    5 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h       |    3 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |   35 ++++++++++++++--
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   |   29 +------------
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |    4 +-
 6 files changed, 62 insertions(+), 58 deletions(-)

-- 
1.7.4.4

^ permalink raw reply

* [PATCH (net.git) 2/4] stmmac: fix driver's doc when run kernel-doc script
From: Giuseppe CAVALLARO @ 2012-06-04 15:37 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro
In-Reply-To: <1338824270-9222-1-git-send-email-peppe.cavallaro@st.com>

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 0caae72..18ed878 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -833,8 +833,9 @@ static u32 stmmac_get_synopsys_id(struct stmmac_priv *priv)
 
 /**
  * stmmac_selec_desc_mode
- * @dev : device pointer
- * Description: select the Enhanced/Alternate or Normal descriptors */
+ * @priv : private structure
+ * Description: select the Enhanced/Alternate or Normal descriptors
+ */
 static void stmmac_selec_desc_mode(struct stmmac_priv *priv)
 {
 	if (priv->plat->enh_desc) {
@@ -1860,6 +1861,8 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
 /**
  * stmmac_dvr_probe
  * @device: device pointer
+ * @plat_dat: platform data pointer
+ * @addr: iobase memory address
  * Description: this is the main probe function used to
  * call the alloc_etherdev, allocate the priv structure.
  */
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH (net.git) 3/4] stmmac: update driver's doc
From: Giuseppe CAVALLARO @ 2012-06-04 15:37 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro
In-Reply-To: <1338824270-9222-1-git-send-email-peppe.cavallaro@st.com>

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 Documentation/networking/stmmac.txt |   44 +++++++++++++++++++---------------
 1 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/Documentation/networking/stmmac.txt b/Documentation/networking/stmmac.txt
index ab1e8d7..5cb9a19 100644
--- a/Documentation/networking/stmmac.txt
+++ b/Documentation/networking/stmmac.txt
@@ -10,8 +10,8 @@ Currently this network device driver is for all STM embedded MAC/GMAC
 (i.e. 7xxx/5xxx SoCs), SPEAr (arm), Loongson1B (mips) and XLINX XC2V3000
 FF1152AMT0221 D1215994A VIRTEX FPGA board.
 
-DWC Ether MAC 10/100/1000 Universal version 3.60a (and older) and DWC Ether MAC 10/100
-Universal version 4.0 have been used for developing this driver.
+DWC Ether MAC 10/100/1000 Universal version 3.60a (and older) and DWC Ether
+MAC 10/100 Universal version 4.0 have been used for developing this driver.
 
 This driver supports both the platform bus and PCI.
 
@@ -54,27 +54,27 @@ net_device structure enabling the scatter/gather feature.
 When one or more packets are received, an interrupt happens. The interrupts
 are not queued so the driver has to scan all the descriptors in the ring during
 the receive process.
-This is based on NAPI so the interrupt handler signals only if there is work to be
-done, and it exits.
+This is based on NAPI so the interrupt handler signals only if there is work
+to be done, and it exits.
 Then the poll method will be scheduled at some future point.
 The incoming packets are stored, by the DMA, in a list of pre-allocated socket
 buffers in order to avoid the memcpy (Zero-copy).
 
 4.3) Timer-Driver Interrupt
-Instead of having the device that asynchronously notifies the frame receptions, the
-driver configures a timer to generate an interrupt at regular intervals.
-Based on the granularity of the timer, the frames that are received by the device
-will experience different levels of latency. Some NICs have dedicated timer
-device to perform this task. STMMAC can use either the RTC device or the TMU
-channel 2  on STLinux platforms.
+Instead of having the device that asynchronously notifies the frame receptions,
+the driver configures a timer to generate an interrupt at regular intervals.
+Based on the granularity of the timer, the frames that are received by the
+device will experience different levels of latency. Some NICs have dedicated
+timer device to perform this task. STMMAC can use either the RTC device or the
+TMU channel 2  on STLinux platforms.
 The timers frequency can be passed to the driver as parameter; when change it,
 take care of both hardware capability and network stability/performance impact.
-Several performance tests on STM platforms showed this optimisation allows to spare
-the CPU while having the maximum throughput.
+Several performance tests on STM platforms showed this optimisation allows to
+spare the CPU while having the maximum throughput.
 
 4.4) WOL
-Wake up on Lan feature through Magic and Unicast frames are supported for the GMAC
-core.
+Wake up on Lan feature through Magic and Unicast frames are supported for the
+GMAC core.
 
 4.5) DMA descriptors
 Driver handles both normal and enhanced descriptors. The latter has been only
@@ -106,7 +106,8 @@ Several driver's information can be passed through the platform
 These are included in the include/linux/stmmac.h header file
 and detailed below as well:
 
- struct plat_stmmacenet_data {
+struct plat_stmmacenet_data {
+	char *phy_bus_name;
 	int bus_id;
 	int phy_addr;
 	int interface;
@@ -124,19 +125,24 @@ and detailed below as well:
 	void (*bus_setup)(void __iomem *ioaddr);
 	int (*init)(struct platform_device *pdev);
 	void (*exit)(struct platform_device *pdev);
+	void *custom_cfg;
+	void *custom_data;
 	void *bsp_priv;
  };
 
 Where:
+ o phy_bus_name: phy bus name to attach to the stmmac.
  o bus_id: bus identifier.
  o phy_addr: the physical address can be passed from the platform.
 	    If it is set to -1 the driver will automatically
 	    detect it at run-time by probing all the 32 addresses.
  o interface: PHY device's interface.
  o mdio_bus_data: specific platform fields for the MDIO bus.
- o pbl: the Programmable Burst Length is maximum number of beats to
+ o dma_cfg: internal DMA parameters
+   o pbl: the Programmable Burst Length is maximum number of beats to
        be transferred in one DMA transaction.
        GMAC also enables the 4xPBL by default.
+   o fixed_burst/mixed_burst/burst_len
  o clk_csr: fixed CSR Clock range selection.
  o has_gmac: uses the GMAC core.
  o enh_desc: if sets the MAC will use the enhanced descriptor structure.
@@ -160,8 +166,9 @@ Where:
 	     this is sometime necessary on some platforms (e.g. ST boxes)
 	     where the HW needs to have set some PIO lines or system cfg
 	     registers.
- o custom_cfg: this is a custom configuration that can be passed while
-	      initialising the resources.
+ o custom_cfg/custom_data: this is a custom configuration that can be passed
+			   while initialising the resources.
+ o bsp_priv: another private poiter.
 
 For MDIO bus The we have:
 
@@ -180,7 +187,6 @@ Where:
  o irqs: list of IRQs, one per PHY.
  o probed_phy_irq: if irqs is NULL, use this for probed PHY.
 
-
 For DMA engine we have the following internal fields that should be
 tuned according to the HW capabilities.
 
-- 
1.7.4.4

^ 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