Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 1/3] pps, ptp: Remove dependencies on EXPERIMENTAL
From: Ben Hutchings @ 2012-11-01  1:31 UTC (permalink / raw)
  To: David Miller, Rodolfo Giometti, Richard Cochran; +Cc: netdev, linux-net-drivers
In-Reply-To: <1351733397.2706.62.camel@bwh-desktop.uk.solarflarecom.com>

These are now established subsystems, and we want drivers to be able
to select PPS and PTP_1588_CLOCK without depending on EXPERIMENTAL.
Further, the use of EXPERIMENTAL is now deprecated in general.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/ethernet/intel/Kconfig            |    4 ++--
 drivers/net/ethernet/oki-semi/pch_gbe/Kconfig |    1 -
 drivers/pps/Kconfig                           |    1 -
 drivers/ptp/Kconfig                           |    1 -
 4 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
index 0cafe4f..9e49217 100644
--- a/drivers/net/ethernet/intel/Kconfig
+++ b/drivers/net/ethernet/intel/Kconfig
@@ -123,7 +123,7 @@ config IGB_DCA
 config IGB_PTP
 	bool "PTP Hardware Clock (PHC)"
 	default n
-	depends on IGB && EXPERIMENTAL
+	depends on IGB
 	select PPS
 	select PTP_1588_CLOCK
 	---help---
@@ -225,7 +225,7 @@ config IXGBE_DCB
 config IXGBE_PTP
 	bool "PTP Clock Support"
 	default n
-	depends on IXGBE && EXPERIMENTAL
+	depends on IXGBE
 	select PPS
 	select PTP_1588_CLOCK
 	---help---
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
index 5296cc8..9867bc6 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
@@ -26,7 +26,6 @@ if PCH_GBE
 config PCH_PTP
 	bool "PCH PTP clock support"
 	default n
-	depends on EXPERIMENTAL
 	select PPS
 	select PTP_1588_CLOCK
 	select PTP_1588_CLOCK_PCH
diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 258ca59..982d16b 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -6,7 +6,6 @@ menu "PPS support"
 
 config PPS
 	tristate "PPS support"
-	depends on EXPERIMENTAL
 	---help---
 	  PPS (Pulse Per Second) is a special pulse provided by some GPS
 	  antennae. Userland can use it to get a high-precision time
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index ffdf712..a6e4b7c 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -9,7 +9,6 @@ comment "Enable Device Drivers -> PPS to see the PTP clock options."
 
 config PTP_1588_CLOCK
 	tristate "PTP clock support"
-	depends on EXPERIMENTAL
 	depends on PPS
 	help
 	  The IEEE 1588 standard defines a method to precisely
-- 
1.7.7.6



-- 
Ben Hutchings, Staff 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 related

* [PATCH net-next 0/3] Enable PPS and PTP clocks automatically
From: Ben Hutchings @ 2012-11-01  1:29 UTC (permalink / raw)
  To: David Miller, Rodolfo Giometti, Richard Cochran; +Cc: netdev, linux-net-drivers

There are a number of problems with PPS and PTP kconfig options:

1. PPS and PTP_1588_CLOCK depend on EXPERIMENTAL, therefore so must the
   clock drivers.  (And EXPERIMENTAL is deprecated.)
2. PTP_1588_CLOCK depends on PPS, so users or clock driver options must
   explicitly select both.
3. In general, users have to enable too many options to get hardware
   timestamps and PTP working.
4. Boolean options for clock drivers combined with net drivers can force
   the PTP and PPS subsystems to be built-in unnecessarily.

This series changes the options as follows:

1. Nothing depends on EXPERIMENTAL.
2. Only PTP_1588_CLOCK selects PPS.
3, 4. The boolean options are removed and the net driver options select
   PTP_1588_CLOCK.  Where a net driver and associated clock driver are
   separate modules, the clock driver remains optional but is default y.

Ben.

Ben Hutchings (3):
  pps, ptp: Remove dependencies on EXPERIMENTAL
  ptp: Make PTP_1588_CLOCK select rather than depend on PPS
  ptp: Enable clock drivers along with associated net/PHY drivers

 drivers/net/ethernet/intel/Kconfig                 |   28 +------------------
 drivers/net/ethernet/intel/igb/Makefile            |    4 +--
 drivers/net/ethernet/intel/igb/igb.h               |    6 ----
 drivers/net/ethernet/intel/igb/igb_ethtool.c       |    2 -
 drivers/net/ethernet/intel/igb/igb_main.c          |   28 --------------------
 drivers/net/ethernet/intel/ixgbe/Makefile          |    3 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe.h           |    6 ----
 drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c   |    2 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c      |   28 --------------------
 drivers/net/ethernet/oki-semi/pch_gbe/Kconfig      |   16 -----------
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    |    2 -
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   |   14 ----------
 drivers/net/ethernet/sfc/Kconfig                   |    7 -----
 drivers/net/ethernet/sfc/Makefile                  |    3 +-
 drivers/net/ethernet/sfc/net_driver.h              |    2 -
 drivers/net/ethernet/sfc/nic.h                     |   26 ------------------
 drivers/pps/Kconfig                                |    1 -
 drivers/ptp/Kconfig                                |   19 ++++++-------
 18 files changed, 14 insertions(+), 183 deletions(-)

-- 
1.7.7.6


-- 
Ben Hutchings, Staff 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: [tcpdump-workers] vlan tagged packets and libpcap breakage
From: Ani Sinha @ 2012-11-01  1:22 UTC (permalink / raw)
  To: Guy Harris; +Cc: netdev, Francesco Ruggeri, tcpdump-workers
In-Reply-To: <B23C28F0-4005-4970-984D-3BAD063EA513@alum.mit.edu>

On Wed, Oct 31, 2012 at 5:50 PM, Guy Harris <guy@alum.mit.edu> wrote:
>
> On Oct 31, 2012, at 3:35 PM, Ani Sinha <ani@aristanetworks.com> wrote:
>
>> yes but if the packet is passed to the filter within libpcap (when we
>> are not using the kernel filter) before the reinsertion,
>
> ...that would be a bug.
>
> Currently, that bug doesn't exist in the recvfrom() code path, but *does* appear to exist in the tpacket code path - and that code path also runs the filter before the SLL header is constructed.  That should be fixed.


yes, agreed.

^ permalink raw reply

* Re: [tcpdump-workers] vlan tagged packets and libpcap breakage
From: Guy Harris @ 2012-11-01  0:50 UTC (permalink / raw)
  To: Ani Sinha; +Cc: netdev, Francesco Ruggeri, tcpdump-workers
In-Reply-To: <CAOxq_8O8Vkm5Napp_h6HVj6C=EH2gd_PQJJ1N5d_VQZrRZFYNQ@mail.gmail.com>


On Oct 31, 2012, at 3:35 PM, Ani Sinha <ani@aristanetworks.com> wrote:

> yes but if the packet is passed to the filter within libpcap (when we
> are not using the kernel filter) before the reinsertion,

...that would be a bug.

Currently, that bug doesn't exist in the recvfrom() code path, but *does* appear to exist in the tpacket code path - and that code path also runs the filter before the SLL header is constructed.  That should be fixed.

Yes, it means constructing the SLL header and inserting the tags blah blah blah on packets that might not pass the filter, but in most cases the filtering should be done in the kernel, not userland, when doing a live capture, and in those cases where the filtering can't done in the kernel, you're already somewhat hosed by the fact that the packets are being copied to the buffer and wakeups are being delivered before the packets are being filtered.

^ permalink raw reply

* [PATCH v2] r8169: Fix WoL on RTL8168d/8111d.
From: Cyril Brulebois @ 2012-11-01  0:00 UTC (permalink / raw)
  To: netdev, davem
  Cc: romieu, nic_swsd, linux-kernel, florent.fourcot, Cyril Brulebois
In-Reply-To: <20121028230523.GA26778@electric-eye.fr.zoreil.com>

This regression was spotted between Debian squeeze and Debian wheezy
kernels (respectively based on 2.6.32 and 3.2). More info about
Wake-on-LAN issues with Realtek's 816x chipsets can be found in the
following thread: http://marc.info/?t=132079219400004

Probable regression from d4ed95d796e5126bba51466dc07e287cebc8bd19;
more chipsets are likely affected.

Tested on top of a 3.2.23 kernel.

Reported-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
Tested-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr>
Hinted-by: Francois Romieu <romieu@fr.zoreil.com>
Signed-off-by: Cyril Brulebois <kibi@debian.org>
---
 drivers/net/ethernet/realtek/r8169.c |    2 ++
 1 file changed, 2 insertions(+)

v2: Update link in commit message, and re-send to netdev/Dave according
    to Francois Romieu's comment, thanks.

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index e7ff886..eb6a5e4 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -3827,6 +3827,8 @@ static void rtl_wol_suspend_quirk(struct rtl8169_private *tp)
 	void __iomem *ioaddr = tp->mmio_addr;
 
 	switch (tp->mac_version) {
+	case RTL_GIGA_MAC_VER_25:
+	case RTL_GIGA_MAC_VER_26:
 	case RTL_GIGA_MAC_VER_29:
 	case RTL_GIGA_MAC_VER_30:
 	case RTL_GIGA_MAC_VER_32:
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH v3 2/6] PM / Runtime: introduce pm_runtime_set[get]_memalloc_noio()
From: Ming Lei @ 2012-10-31 23:18 UTC (permalink / raw)
  To: Alan Stern
  Cc: Oliver Neukum, linux-kernel, Minchan Kim, Greg Kroah-Hartman,
	Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
	netdev, linux-usb, linux-pm, linux-mm
In-Reply-To: <Pine.LNX.4.44L0.1210311139300.1954-100000@iolanthe.rowland.org>

On Wed, Oct 31, 2012 at 11:41 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>
> Sorry, I misread your message.  You are setting the device's flag, not
> the thread's flag.

Never mind.

>
> This still doesn't help in this case where CONFIG_PM_RUNTIME is
> disabled.  I think it will be simpler to set the noio flag during every
> device reset.

Yes, it's better to set the flag during every device reset now.

Also pppoe or network interface over serial port is a bit difficult to
deal with, as Oliver pointed out.


Thanks,
--
Ming Lei

^ permalink raw reply

* Re: [tcpdump-workers] vlan tagged packets and libpcap breakage
From: Guy Harris @ 2012-10-31 22:20 UTC (permalink / raw)
  To: Ani Sinha; +Cc: Michael Richardson, netdev, Francesco Ruggeri, tcpdump-workers
In-Reply-To: <CAOxq_8PsXb-oUy85Eji7GSAbUZD_Bds8skmGNP4BWSewQWx8PA@mail.gmail.com>


On Oct 31, 2012, at 2:50 PM, Ani Sinha <ani@aristanetworks.com> wrote:

> pcap files that already have the tags reinsrted should work with
> current filter code. However for live traffic, one has to get the tags
> from CMSG() and then reinsert it back to the packet for the current
> filter to work.

*Somebody* has to do that, at least to packets that pass the filter, before they're handed to a libpcap-based application, for programs that expect to see packets as they arrived from/were transmitted to the wire to work.

I.e., the tags *should* be reinserted by libpcap, and, as I understand it, that's what the

	#if defined(HAVE_PACKET_AUXDATA) && defined(HAVE_LINUX_TPACKET_AUXDATA_TP_VLAN_TCI)
		...
	#endif

blocks of code in pcap-linux.c in libpcap are doing.

Now, if filtering is being done in the *kernel*, and the tags aren't being reinserted by the kernel, then filter code stuffed into the kernel would need to differ from filter code run in userland.  There's already precedent for that on Linux, with the "cooked mode" headers; those are synthesized by libpcap from the metadata returned for PF_PACKET sockets, and the code that attempts to hand the kernel a filter goes through the filter code, which was generated under the assumption that the packet begins with a "cooked mode" header, and modifies (a copy of) the code to, instead, use the special Linux-BPF-interpreter offsets to access the metadata.

The right thing to do here would be to, if possible, do the same, so that the kernel doesn't have to reinsert VLAN tags for packets that aren't going to be handed to userland.

And, yes, if that should be done for some interfaces with some kernel versions but not all interfaces for all kernel versions, there would need to be a way for libpcap to ask whether it's necessary.  Is it necessary on any interfaces *before* the kernel change in question?

^ permalink raw reply

* Re: [BUG]  TIPC handling of -ERESTARTSYS in connect()
From: Chris Friesen @ 2012-10-31 23:01 UTC (permalink / raw)
  To: Ying Xue; +Cc: netdev, Allan Stephens, Jon Maloy
In-Reply-To: <5046161F.8070703@genband.com>

On 09/04/2012 08:54 AM, Chris Friesen wrote:
> On 09/02/2012 09:16 PM, Ying Xue wrote:
>>
>>>>
>>>> It looks like the case where we come in with "sock->state ==
>>>> SS_LISTENING" has changed. Previously we would return -EOPNOTSUPP but
>>>> now it'll be -EINVAL. Is that intentional?
>>>
>>> Just noticed something else. In the SS_CONNECTING case I don't think
>>> there's any point in setting "res = -EALREADY" since a bit further
>>> down it gets set unconditionally anyway.
>>>
>> Yes, you are right. Please check v2 version.
>
> Looks good to me. Still haven't had a chance to actually test it though,
> we're pushing out a release and time is scarce.

I finally got a chance to test this out, and it does indeed fix the 
problem.  Please push it upstream.

Tested-by: Chris Friesen <chris.friesen@genband.com>

Chris

^ permalink raw reply

* Re: [tcpdump-workers] vlan tagged packets and libpcap breakage
From: Michael Richardson @ 2012-10-31 22:42 UTC (permalink / raw)
  To: netdev, tcpdump-workers; +Cc: Ani Sinha, Francesco Ruggeri
In-Reply-To: <CAOxq_8PsXb-oUy85Eji7GSAbUZD_Bds8skmGNP4BWSewQWx8PA@mail.gmail.com>

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


>>>>> "Ani" == Ani Sinha <ani@aristanetworks.com> writes:
    Ani> cc'ing netdev.

    >> Do you think that the existance of this behaviour could be exposed in
    >> sysctl, /proc/net or /sys equivalent (we still don't have /sys/net...)?
    >> As a read only file that had a 0/1 in it?

    Ani> yes, we definitely need a run time check. Whether this could be in the
    Ani> form of a socket option or a /proc entry I don't know.


unsigned int netdev_8021q_inskb = 1;

...
	{
		.ctl_name	= NET_CORE_8021q_INSKB,
		.procname	= "netdev_8021q_inskb",
		.data		= &netdev_8021q_inskb,
		.maxlen		= sizeof(int),
		.mode		= 0444,
		.proc_handler	= proc_dointvec
	},

would seem to do it to me.
Then pcap can fopen("/proc/sys/net/core/netdev_8021q_inskb") and if it
finds it, and it is >0, then do the cmsg thing.

    >> It sounds like it's impossible to capture all traffic now, and do vlan
    >> filtering later on?

    Ani> pcap files that already have the tags reinsrted should work with
    Ani> current filter code. However for live traffic, one has to get the tags
    Ani> from CMSG() and then reinsert it back to the packet for the current
    Ani> filter to work. This is slow.

I think that this better vlan handling will be much faster, and make
things much more consistent between systems with and without hardware
offload, so it's great news.

However, a number of people use Linux machines to do traffic captures of various
kinds using interfaces dedicated to that purpose.

It would be nice if there was a way to get the packet "whole", such as
by having a second PACKET_CAPTURE point, which was before the adjustment
of the vlan.  (obviously, if you had hardware, you'd want to turn that
off)
Many would like this capture point to actually eat all packets for the
interfaces it was defined for, as people doing captures never want those
packets going up the stack. 

-- 
]       He who is tired of Weird Al is tired of life!           |  firewalls  [
]   Michael Richardson, Sandelman Software Works, Ottawa, ON    |net architect[
] mcr@sandelman.ottawa.on.ca http://www.sandelman.ottawa.on.ca/ |device driver[
   Kyoto Plus: watch the video <http://www.youtube.com/watch?v=kzx1ycLXQSE>
	               then sign the petition. 

[-- Attachment #2: Type: application/pgp-signature, Size: 307 bytes --]

^ permalink raw reply

* [RFC virtio-next 4/4] caif_virtio: Add CAIF over virtio
From: Sjur Brændeland @ 2012-10-31 22:46 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Vikram ARV, Michael S. Tsirkin, netdev, Linus Walleij,
	dmitry.tarnyagin, linux-kernel, virtualization, sjur,
	Sjur Brændeland
In-Reply-To: <1351723614-4145-1-git-send-email-sjur@brendeland.net>

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Add the CAIF Virtio Link layer, used for communicating with a
modem over shared memory. Virtio is used as the transport mechanism.

In the TX direction the virtio rings are used in the normal fashion,
sending data in the available ring. But in the rx direction the
the we have flipped the direction of the virtio ring, and
implemented the virtio access-function similar to what is found
in vhost.c.

CAIF also uses the virtio configuration space for getting
configuration parameters such as headroom, tailroom etc.

Signed-off-by: Vikram ARV <vikram.arv@stericsson.com>
Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 drivers/net/caif/Kconfig        |    9 +
 drivers/net/caif/Makefile       |    3 +
 drivers/net/caif/caif_virtio.c  |  627 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/virtio_ids.h |    1 +
 4 files changed, 640 insertions(+)
 create mode 100644 drivers/net/caif/caif_virtio.c

diff --git a/drivers/net/caif/Kconfig b/drivers/net/caif/Kconfig
index abf4d7a..a01f617 100644
--- a/drivers/net/caif/Kconfig
+++ b/drivers/net/caif/Kconfig
@@ -47,3 +47,12 @@ config CAIF_HSI
        The caif low level driver for CAIF over HSI.
        Be aware that if you enable this then you also need to
        enable a low-level HSI driver.
+
+config CAIF_VIRTIO
+       tristate "CAIF virtio transport driver"
+       default n
+       depends on CAIF
+       depends on REMOTEPROC
+       select VIRTIO
+       ---help---
+       The caif driver for CAIF over Virtio.
diff --git a/drivers/net/caif/Makefile b/drivers/net/caif/Makefile
index 91dff86..d9ee26a 100644
--- a/drivers/net/caif/Makefile
+++ b/drivers/net/caif/Makefile
@@ -13,3 +13,6 @@ obj-$(CONFIG_CAIF_SHM) += caif_shm.o
 
 # HSI interface
 obj-$(CONFIG_CAIF_HSI) += caif_hsi.o
+
+# Virtio interface
+obj-$(CONFIG_CAIF_VIRTIO) += caif_virtio.o
diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c
new file mode 100644
index 0000000..e50940f
--- /dev/null
+++ b/drivers/net/caif/caif_virtio.c
@@ -0,0 +1,627 @@
+/*
+ * Copyright (C) ST-Ericsson AB 2012
+ * Contact: Sjur Brendeland / sjur.brandeland@stericsson.com
+ * Authors: Vicram Arv / vikram.arv@stericsson.com,
+ *	    Dmitry Tarnyagin / dmitry.tarnyagin@stericsson.com
+ *	    Sjur Brendeland / sjur.brandeland@stericsson.com
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ":" fmt
+#include <linux/module.h>
+#include <linux/virtio.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_config.h>
+#include <linux/dma-mapping.h>
+#include <linux/netdevice.h>
+#include <linux/if_arp.h>
+#include <linux/spinlock.h>
+#include <net/caif/caif_dev.h>
+#include <linux/virtio_caif.h>
+#include "../drivers/virtio/vring.h"
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Vicram Arv <vikram.arv@stericsson.com>");
+MODULE_DESCRIPTION("Virtio CAIF Driver");
+
+/*
+ * struct cfv_info - Caif Virtio control structure
+ * @cfdev:	caif common header
+ * @vdev:	Associated virtio device
+ * @vq_rx:	rx/downlink virtqueue
+ * @vq_tx:	tx/uplink virtqueue
+ * @ndev:	associated netdevice
+ * @queued_tx:	number of buffers queued in the tx virtqueue
+ * @watermark_tx: indicates number of buffers the tx queue
+ *		should shrink to to unblock datapath
+ * @tx_lock:	protects vq_tx to allow concurrent senders
+ * @tx_hr:	transmit headroom
+ * @rx_hr:	receive headroom
+ * @tx_tr:	transmit tailroom
+ * @rx_tr:	receive tailroom
+ * @mtu:	transmit max size
+ * @mru:	receive max size
+ */
+struct cfv_info {
+	struct caif_dev_common cfdev;
+	struct virtio_device *vdev;
+	struct virtqueue *vq_rx;
+	struct virtqueue *vq_tx;
+	struct net_device *ndev;
+	unsigned int queued_tx;
+	unsigned int watermark_tx;
+	/* Protect access to vq_tx */
+	spinlock_t tx_lock;
+	/* Copied from Virtio config space */
+	u16 tx_hr;
+	u16 rx_hr;
+	u16 tx_tr;
+	u16 rx_tr;
+	u32 mtu;
+	u32 mru;
+};
+
+/*
+ * struct token_info - maintains Transmit buffer data handle
+ * @size:	size of transmit buffer
+ * @dma_handle: handle to allocated dma device memory area
+ * @vaddr:	virtual address mapping to allocated memory area
+ */
+struct token_info {
+	size_t size;
+	u8 *vaddr;
+	dma_addr_t dma_handle;
+};
+
+/* Default if virtio config space is unavailable */
+#define CFV_DEF_MTU_SIZE 4096
+#define CFV_DEF_HEADROOM 16
+#define CFV_DEF_TAILROOM 16
+
+/* Require IP header to be 4-byte aligned. */
+#define IP_HDR_ALIGN 4
+
+/*
+ * virtqueue_next_avail_desc - get the next available descriptor
+ * @_vq: the struct virtqueue we're talking about
+ * @head: index of the descriptor in the ring
+ *
+ * Look for the next available descriptor in the available ring.
+ * Return NULL if nothing new in the available.
+ */
+static struct vring_desc *virtqueue_next_avail_desc(struct virtqueue *_vq,
+						    int *head)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	u16 avail_idx, hd, last_avail_idx = vq->last_avail_idx;
+
+	START_USE(vq);
+
+	if (unlikely(vq->broken))
+		goto err;
+
+	/* The barier secures observability of avail->idx after interrupt */
+	virtio_rmb(vq);
+
+	if (vq->last_avail_idx == vq->vring.avail->idx)
+		goto err;
+
+	avail_idx = vq->vring.avail->idx;
+	if (unlikely((u16)(avail_idx - last_avail_idx) > vq->vring.num)) {
+		BAD_RING(vq, "Avail index moved from %u to %u",
+			 last_avail_idx, avail_idx);
+		goto err;
+	}
+
+	/*
+	 * The barier secures observability of the ring content
+	 * after avail->idx update
+	 */
+	virtio_rmb(vq);
+
+	hd =  vq->vring.avail->ring[last_avail_idx & (vq->vring.num - 1)];
+	/* If their number is silly, that's an error. */
+	if (unlikely(hd >= vq->vring.num)) {
+		BAD_RING(vq, "Remote says index %d > %u is available",
+			 *head, vq->vring.num);
+		goto err;
+	}
+
+	END_USE(vq);
+	*head = hd;
+	return &vq->vring.desc[hd];
+err:
+	END_USE(vq);
+	*head = -1;
+	return NULL;
+}
+
+/*
+ * virtqueue_next_linked_desc - get next linked descriptor from the ring
+ * @_vq: the struct virtqueue we're talking about
+ * @desc: "current" descriptor
+ *
+ * Each buffer in the virtqueues is a chain of descriptors. This
+ * function returns the next descriptor in the chain,* or NULL if we're at
+ * the end.
+ *
+ * Side effect: the function increments vq->last_avail_idx if a non-linked
+ * descriptor is passed as &desc argument.
+ */
+static struct vring_desc *virtqueue_next_linked_desc(struct virtqueue *_vq,
+						     struct vring_desc *desc)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	unsigned int next;
+
+	START_USE(vq);
+
+	/* If this descriptor says it doesn't chain, we're done. */
+	if (!(desc->flags & VRING_DESC_F_NEXT))
+		goto no_next;
+
+	next = desc->next;
+	/* Make sure compiler knows to grab that: we don't want it changing! */
+	/* We will use the result as an index in an array, so most
+	 * architectures only need a compiler barrier here.
+	 */
+	read_barrier_depends();
+
+	if (unlikely(next >= vq->vring.num)) {
+		BAD_RING(vq, "Desc index is %u > %u\n", next, vq->vring.num);
+		goto err;
+	}
+
+	desc = &vq->vring.desc[next];
+
+	if (desc->flags & VRING_DESC_F_INDIRECT) {
+		pr_err("Indirect descriptor not supported\n");
+		goto err;
+	}
+
+	END_USE(vq);
+	return desc;
+no_next:
+	vq->last_avail_idx++;
+err:
+	END_USE(vq);
+	return NULL;
+}
+
+/*
+ * virtqueue_add_buf_to_used - release a used descriptor
+ * @_vq: the struct virtqueue we're talking about
+ * @head: index of the descriptor to be released
+ * @len: number of linked descriptors in a chain
+ *
+ * The function releases a used descriptor in a reversed ring
+ */
+static int virtqueue_add_buf_to_used(struct virtqueue *_vq,
+				     unsigned int head, int len)
+{
+	struct vring_virtqueue *vr_vq = to_vvq(_vq);
+	struct vring_used_elem	*used;
+	int used_idx, err = -EINVAL;
+
+	START_USE(vr_vq);
+
+	if (unlikely(vr_vq->broken))
+		goto err;
+
+	if (unlikely(head >= vr_vq->vring.num)) {
+		BAD_RING(vr_vq, "Invalid head index (%u) > max desc idx (%u) ",
+			 head, vr_vq->vring.num - 1);
+		goto err;
+	}
+
+	/*
+	 * The virtqueue contains a ring of used buffers.  Get a pointer to the
+	 * next entry in that used ring.
+	 */
+	used_idx = (vr_vq->vring.used->idx & (vr_vq->vring.num - 1));
+	used = &vr_vq->vring.used->ring[used_idx];
+	used->id = head;
+	used->len = len;
+
+	/* Make sure buffer is written before we update index. */
+	virtio_wmb(vr_vq);
+	++vr_vq->vring.used->idx;
+	err = 0;
+err:
+	END_USE(vr_vq);
+	return err;
+
+}
+
+/*
+ * virtqueue_next_desc - get next available or linked descriptor
+ * @_vq: the struct virtqueue we're talking about
+ * @desc: "current" descriptor.
+ * @head: on return it is filled by the descriptor index in case of
+ *	available descriptor was returned, or -1 in case of linked
+ *	descriptor.
+ *
+ * The function is to be used as an iterator through received descriptors.
+ */
+static struct vring_desc *virtqueue_next_desc(struct virtqueue *_vq,
+					      struct vring_desc *desc,
+					      int *head)
+{
+	struct vring_desc *next = virtqueue_next_linked_desc(_vq, desc);
+
+	if (next == NULL) {
+		virtqueue_add_buf_to_used(_vq, *head, 0);
+		/* tell the remote processor to recycle buffer */
+		virtqueue_kick(_vq);
+		next = virtqueue_next_avail_desc(_vq, head);
+	} 
+	return next;
+}
+
+/*
+ * This is invoked whenever the remote processor completed processing
+ * a TX msg we just sent it, and the buffer is put back to the used ring.
+ */
+static void cfv_release_used_buf(struct virtqueue *vq_tx)
+{
+	struct cfv_info *cfv = vq_tx->vdev->priv;
+
+	BUG_ON(vq_tx != cfv->vq_tx);
+
+	for (;;) {
+		unsigned int len;
+		struct token_info *buf_info;
+
+		/* Get used buffer from used ring to recycle used descriptors */
+		spin_lock_bh(&cfv->tx_lock);
+		buf_info = virtqueue_get_buf(vq_tx, &len);
+
+		if (!buf_info)
+			goto out;
+
+		BUG_ON(!cfv->queued_tx);
+		if (--cfv->queued_tx < cfv->watermark_tx) {
+			cfv->watermark_tx = 0;
+			netif_tx_wake_all_queues(cfv->ndev);
+		}
+		spin_unlock_bh(&cfv->tx_lock);
+
+		dma_free_coherent(vq_tx->vdev->dev.parent->parent,
+				  buf_info->size, buf_info->vaddr,
+				  buf_info->dma_handle);
+		kfree(buf_info);
+	}
+	return;
+out:
+	spin_unlock_bh(&cfv->tx_lock);
+}
+
+static int cfv_read_desc(struct vring_desc *d,
+			 void **buf, size_t *size)
+{
+	if (d->flags & VRING_DESC_F_INDIRECT) {
+		pr_warn("Indirect descriptor not supported by CAIF\n");
+		return -EINVAL;
+	}
+
+	if (!(d->flags & VRING_DESC_F_WRITE)) {
+		pr_warn("Write descriptor not supported by CAIF\n");
+		/* CAIF expects a input descriptor here */
+		return -EINVAL;
+	}
+	*buf = phys_to_virt(d->addr);
+	*size = d->len;
+	return 0;
+}
+
+static struct sk_buff *cfv_alloc_and_copy_skb(struct cfv_info *cfv,
+					      u8 *frm, u32 frm_len)
+{
+	struct sk_buff *skb;
+	u32 cfpkt_len, pad_len;
+
+	/* Verify that packet size with down-link header and mtu size */
+	if (frm_len > cfv->mru || frm_len <= cfv->rx_hr + cfv->rx_tr) {
+		netdev_err(cfv->ndev,
+			   "Invalid frmlen:%u  mtu:%u hr:%d tr:%d\n",
+			   frm_len, cfv->mru,  cfv->rx_hr,
+			   cfv->rx_tr);
+		return NULL;
+	}
+
+	cfpkt_len = frm_len - (cfv->rx_hr + cfv->rx_tr);
+
+	pad_len = (unsigned long)(frm + cfv->rx_hr) & (IP_HDR_ALIGN - 1);
+
+	skb = netdev_alloc_skb(cfv->ndev, frm_len + pad_len);
+	if (!skb)
+		return NULL;
+
+	/* Reserve space for headers. */
+	skb_reserve(skb, cfv->rx_hr + pad_len);
+
+	memcpy(skb_put(skb, cfpkt_len), frm + cfv->rx_hr, cfpkt_len);
+	return skb;
+}
+
+/*
+ * This is invoked whenever the remote processor has sent down-link data
+ * on the Rx VQ avail ring and it's time to digest a message.
+ *
+ * CAIF virtio passes a complete CAIF frame including head/tail room
+ * in each linked descriptor. So iterate over all available buffers
+ * in available-ring and the associated linked descriptors.
+ */
+static void cfv_recv(struct virtqueue *vq_rx)
+{
+	struct cfv_info *cfv = vq_rx->vdev->priv;
+	struct vring_desc *desc;
+	struct sk_buff *skb;
+	int head = -1;
+	void *buf;
+	size_t len;
+
+	for (desc = virtqueue_next_avail_desc(vq_rx, &head);
+	     desc != NULL && !cfv_read_desc(desc, &buf, &len);
+	     desc = virtqueue_next_desc(vq_rx, desc, &head)) {
+
+		skb = cfv_alloc_and_copy_skb(cfv, buf, len);
+
+		if (!skb)
+			goto err;
+
+		skb->protocol = htons(ETH_P_CAIF);
+		skb_reset_mac_header(skb);
+		skb->dev = cfv->ndev;
+
+		/* Push received packet up the stack. */
+		if (netif_receive_skb(skb))
+			goto err;
+
+		++cfv->ndev->stats.rx_packets;
+		cfv->ndev->stats.rx_bytes += skb->len;
+	}
+	return;
+err:
+	++cfv->ndev->stats.rx_dropped;
+	return;
+}
+
+static int cfv_netdev_open(struct net_device *netdev)
+{
+	netif_carrier_on(netdev);
+	return 0;
+}
+
+static int cfv_netdev_close(struct net_device *netdev)
+{
+	netif_carrier_off(netdev);
+	return 0;
+}
+
+static struct token_info *cfv_alloc_and_copy_to_dmabuf(struct cfv_info *cfv,
+						       struct sk_buff *skb,
+						       struct scatterlist *sg)
+{
+	struct caif_payload_info *info = (void *)&skb->cb;
+	struct token_info *buf_info = NULL;
+	u8 pad_len, hdr_ofs;
+
+	if (unlikely(cfv->tx_hr + skb->len + cfv->tx_tr > cfv->mtu)) {
+		netdev_warn(cfv->ndev, "Invalid packet len (%d > %d)\n",
+			    cfv->tx_hr + skb->len + cfv->tx_tr, cfv->mtu);
+		goto err;
+	}
+
+	buf_info = kmalloc(sizeof(struct token_info), GFP_ATOMIC);
+	if (unlikely(!buf_info))
+		goto err;
+
+	/* Make the IP header aligned in tbe buffer */
+	hdr_ofs = cfv->tx_hr + info->hdr_len;
+	pad_len = hdr_ofs & (IP_HDR_ALIGN - 1);
+	buf_info->size = cfv->tx_hr + skb->len + cfv->tx_tr + pad_len;
+
+	if (WARN_ON_ONCE(cfv->vdev->dev.parent))
+		goto err;
+
+	/* allocate coherent memory for the buffers */
+	buf_info->vaddr =
+		dma_alloc_coherent(cfv->vdev->dev.parent->parent,
+				   buf_info->size, &buf_info->dma_handle,
+				   GFP_ATOMIC);
+	if (unlikely(!buf_info->vaddr)) {
+		netdev_warn(cfv->ndev,
+			    "Out of DMA memory (alloc %zu bytes)\n",
+			    buf_info->size);
+		goto err;
+	}
+
+	/* copy skbuf contents to send buffer */
+	skb_copy_bits(skb, 0, buf_info->vaddr + cfv->tx_hr + pad_len, skb->len);
+	sg_init_one(sg, buf_info->vaddr + pad_len,
+		    skb->len + cfv->tx_hr + cfv->rx_hr);
+	return buf_info;
+err:
+	kfree(buf_info);
+	return NULL;
+}
+
+/*
+ * This is invoked whenever the host processor application has sent up-link data.
+ * Send it in the TX VQ avail ring.
+ *
+ * CAIF Virtio sends does not use linked descriptors in the tx direction.
+ */
+static int cfv_netdev_tx(struct sk_buff *skb, struct net_device *netdev)
+{
+	struct cfv_info *cfv = netdev_priv(netdev);
+	struct token_info *buf_info;
+	struct scatterlist sg;
+	bool flow_off = false;
+
+	buf_info = cfv_alloc_and_copy_to_dmabuf(cfv, skb, &sg);
+	spin_lock_bh(&cfv->tx_lock);
+
+	/*
+	 * Add buffer to avail ring.
+	 * Note: in spite of a space check at beginning of the function,
+	 * the add_buff call might fail in case of concurrent access on smp
+	 * systems.
+	 */
+	if (WARN_ON(virtqueue_add_buf(cfv->vq_tx, &sg, 0, 1,
+				      buf_info, GFP_ATOMIC) < 0)) {
+		/* It should not happen */
+		++cfv->ndev->stats.tx_dropped;
+		flow_off = true;
+	} else {
+		/* update netdev statistics */
+		cfv->queued_tx++;
+		cfv->ndev->stats.tx_packets++;
+		cfv->ndev->stats.tx_bytes += skb->len;
+	}
+
+	/*
+	 * Flow-off check takes into account number of cpus to make sure
+	 * virtqueue will not be overfilled in any possible smp conditions.
+	 */
+	flow_off = cfv->queued_tx + num_present_cpus() >=
+		virtqueue_get_vring_size(cfv->vq_tx);
+
+	/* tell the remote processor it has a pending message to read */
+	virtqueue_kick(cfv->vq_tx);
+
+	if (flow_off) {
+		cfv->watermark_tx = cfv->queued_tx >> 1;
+		netif_tx_stop_all_queues(netdev);
+	}
+
+	spin_unlock_bh(&cfv->tx_lock);
+
+	dev_kfree_skb(skb);
+
+	/* Try to speculatively free used buffers */
+	if (flow_off)
+		cfv_release_used_buf(cfv->vq_tx);
+
+	return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops cfv_netdev_ops = {
+	.ndo_open = cfv_netdev_open,
+	.ndo_stop = cfv_netdev_close,
+	.ndo_start_xmit = cfv_netdev_tx,
+};
+
+static void cfv_netdev_setup(struct net_device *netdev)
+{
+	netdev->netdev_ops = &cfv_netdev_ops;
+	netdev->type = ARPHRD_CAIF;
+	netdev->tx_queue_len = 100;
+	netdev->flags = IFF_POINTOPOINT | IFF_NOARP;
+	netdev->mtu = CFV_DEF_MTU_SIZE;
+	netdev->destructor = free_netdev;
+}
+
+#define GET_VIRTIO_CONFIG_OPS(_v, _var, _f) \
+	((_v)->config->get(_v, offsetof(struct virtio_caif_transf_config, _f), \
+			   &_var, \
+			   FIELD_SIZEOF(struct virtio_caif_transf_config, _f)))
+
+static int __devinit cfv_probe(struct virtio_device *vdev)
+{
+	vq_callback_t *vq_cbs[] = { cfv_recv, cfv_release_used_buf };
+	const char *names[] = { "input", "output" };
+	const char *cfv_netdev_name = "cfvrt";
+	struct net_device *netdev;
+	struct virtqueue *vqs[2];
+	struct cfv_info *cfv;
+	int err = 0;
+
+	netdev = alloc_netdev(sizeof(struct cfv_info), cfv_netdev_name,
+			      cfv_netdev_setup);
+	if (!netdev)
+		return -ENOMEM;
+
+	cfv = netdev_priv(netdev);
+	cfv->vdev = vdev;
+	cfv->ndev = netdev;
+
+	spin_lock_init(&cfv->tx_lock);
+
+	/* Get two virtqueues, for tx/ul and rx/dl */
+	err = vdev->config->find_vqs(vdev, 2, vqs, vq_cbs, names);
+	if (err)
+		goto free_cfv;
+
+	cfv->vq_rx = vqs[0];
+	cfv->vq_tx = vqs[1];
+
+	if (vdev->config->get) {
+		GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_hr, headroom);
+		GET_VIRTIO_CONFIG_OPS(vdev, cfv->rx_hr, headroom);
+		GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_tr, tailroom);
+		GET_VIRTIO_CONFIG_OPS(vdev, cfv->rx_tr, tailroom);
+		GET_VIRTIO_CONFIG_OPS(vdev, cfv->mtu, mtu);
+		GET_VIRTIO_CONFIG_OPS(vdev, cfv->mru, mtu);
+	} else {
+		cfv->tx_hr = CFV_DEF_HEADROOM;
+		cfv->rx_hr = CFV_DEF_HEADROOM;
+		cfv->tx_tr = CFV_DEF_TAILROOM;
+		cfv->rx_tr = CFV_DEF_TAILROOM;
+		cfv->mtu = CFV_DEF_MTU_SIZE;
+		cfv->mru = CFV_DEF_MTU_SIZE;
+
+	}
+
+	vdev->priv = cfv;
+
+	netif_carrier_off(netdev);
+
+	/* register Netdev */
+	err = register_netdev(netdev);
+	if (err) {
+		dev_err(&vdev->dev, "Unable to register netdev (%d)\n", err);
+		goto vqs_del;
+	}
+
+	/* tell the remote processor it can start sending messages */
+	virtqueue_kick(cfv->vq_rx);
+	return 0;
+
+vqs_del:
+	vdev->config->del_vqs(cfv->vdev);
+free_cfv:
+	free_netdev(netdev);
+	return err;
+}
+
+static void __devexit cfv_remove(struct virtio_device *vdev)
+{
+	struct cfv_info *cfv = vdev->priv;
+	vdev->config->reset(vdev);
+	vdev->config->del_vqs(cfv->vdev);
+	unregister_netdev(cfv->ndev);
+}
+
+static struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_CAIF, VIRTIO_DEV_ANY_ID },
+	{ 0 },
+};
+
+static unsigned int features[] = {
+};
+
+static struct virtio_driver caif_virtio_driver = {
+	.feature_table		= features,
+	.feature_table_size	= ARRAY_SIZE(features),
+	.driver.name		= KBUILD_MODNAME,
+	.driver.owner		= THIS_MODULE,
+	.id_table		= id_table,
+	.probe			= cfv_probe,
+	.remove			= cfv_remove,
+};
+
+module_driver(caif_virtio_driver, register_virtio_driver,
+	      unregister_virtio_driver);
+MODULE_DEVICE_TABLE(virtio, id_table);
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 270fb22..8ddad5a 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -37,5 +37,6 @@
 #define VIRTIO_ID_RPMSG		7 /* virtio remote processor messaging */
 #define VIRTIO_ID_SCSI		8 /* virtio scsi */
 #define VIRTIO_ID_9P		9 /* 9p virtio console */
+#define VIRTIO_ID_CAIF		12 /* virtio caif */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
-- 
1.7.9.5

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related

* [RFC virtio-next 3/4] virtio_ring: Call callback function even when used ring is empty
From: Sjur Brændeland @ 2012-10-31 22:46 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Michael S. Tsirkin, netdev, Linus Walleij, dmitry.tarnyagin,
	linux-kernel, virtualization, sjur, Sjur Brændeland
In-Reply-To: <1351723614-4145-1-git-send-email-sjur@brendeland.net>

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Enable option to force call of callback function even if
used ring is empty. This is needed for reversed vring.
Add a helper function  __vring_interrupt and add extra
boolean argument for forcing callback when interrupt is called.
The original vring_interrupt semantic and signature is
perserved.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 drivers/remoteproc/remoteproc_virtio.c |    2 +-
 drivers/virtio/virtio_ring.c           |    6 +++---
 include/linux/virtio_ring.h            |    8 +++++++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index e7a4780..ddde863 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -63,7 +63,7 @@ irqreturn_t rproc_vq_interrupt(struct rproc *rproc, int notifyid)
 	if (!rvring || !rvring->vq)
 		return IRQ_NONE;
 
-	return vring_interrupt(0, rvring->vq);
+	return __vring_interrupt(0, rvring->vq, true);
 }
 EXPORT_SYMBOL(rproc_vq_interrupt);
 
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 9027af6..af85034 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -504,11 +504,11 @@ void *virtqueue_detach_unused_buf(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_detach_unused_buf);
 
-irqreturn_t vring_interrupt(int irq, void *_vq)
+irqreturn_t __vring_interrupt(int irq, void *_vq, bool force)
 {
 	struct vring_virtqueue *vq = to_vvq(_vq);
 
-	if (!more_used(vq)) {
+	if (!force && !more_used(vq)) {
 		pr_debug("virtqueue interrupt with no work for %p\n", vq);
 		return IRQ_NONE;
 	}
@@ -522,7 +522,7 @@ irqreturn_t vring_interrupt(int irq, void *_vq)
 
 	return IRQ_HANDLED;
 }
-EXPORT_SYMBOL_GPL(vring_interrupt);
+EXPORT_SYMBOL_GPL(__vring_interrupt);
 
 struct virtqueue *vring_new_virtqueue(unsigned int index,
 				      unsigned int num,
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 63c6ea1..ccb7915 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -20,5 +20,11 @@ void vring_del_virtqueue(struct virtqueue *vq);
 /* Filter out transport-specific feature bits. */
 void vring_transport_features(struct virtio_device *vdev);
 
-irqreturn_t vring_interrupt(int irq, void *_vq);
+irqreturn_t __vring_interrupt(int irq, void *_vq, bool force);
+
+static inline irqreturn_t vring_interrupt(int irq, void *_vq)
+{
+	return __vring_interrupt(irq, _vq, false);
+}
+
 #endif /* _LINUX_VIRTIO_RING_H */
-- 
1.7.9.5

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related

* [RFC virtio-next 2/4] include/vring.h: Add support for reversed vritio rings.
From: Sjur Brændeland @ 2012-10-31 22:46 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Michael S. Tsirkin, netdev, Linus Walleij, dmitry.tarnyagin,
	linux-kernel, virtualization, sjur, Sjur Brændeland
In-Reply-To: <1351723614-4145-1-git-send-email-sjur@brendeland.net>

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Add last avilable index to the vring_virtqueue structure,
this is done to prepare for implementation of the reversed vring.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 drivers/virtio/vring.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/virtio/vring.h b/drivers/virtio/vring.h
index b997fc3..3b53961 100644
--- a/drivers/virtio/vring.h
+++ b/drivers/virtio/vring.h
@@ -51,6 +51,9 @@ struct vring_virtqueue
 	/* Last used index we've seen. */
 	u16 last_used_idx;
 
+	/* Last avail index seen. NOTE: Only used for reversed rings.*/
+	u16 last_avail_idx;
+
 	/* How to notify other side. FIXME: commonalize hcalls! */
 	void (*notify)(struct virtqueue *vq);
 
-- 
1.7.9.5

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related

* [RFC virtio-next 1/4] virtio: Move definitions to header file vring.h
From: Sjur Brændeland @ 2012-10-31 22:46 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Michael S. Tsirkin, netdev, Linus Walleij, dmitry.tarnyagin,
	linux-kernel, virtualization, sjur, Sjur Brændeland
In-Reply-To: <1351723614-4145-1-git-send-email-sjur@brendeland.net>

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Move the vring_virtqueue structure, memory barrier and debug
macros out from virtio_ring.c to the new header file vring.h.
This is done in order to allow other kernel modules to access the
virtio internal data-structures.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
Tis patch triggers a couple of checkpatch warnings, but I've
chosen to do a clean copy and not do any corrections.

 drivers/virtio/virtio_ring.c |   96 +--------------------------------
 drivers/virtio/vring.h       |  121 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+), 95 deletions(-)
 create mode 100644 drivers/virtio/vring.h

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index ffd7e7d..9027af6 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -23,101 +23,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/hrtimer.h>
-
-/* virtio guest is communicating with a virtual "device" that actually runs on
- * a host processor.  Memory barriers are used to control SMP effects. */
-#ifdef CONFIG_SMP
-/* Where possible, use SMP barriers which are more lightweight than mandatory
- * barriers, because mandatory barriers control MMIO effects on accesses
- * through relaxed memory I/O windows (which virtio-pci does not use). */
-#define virtio_mb(vq) \
-	do { if ((vq)->weak_barriers) smp_mb(); else mb(); } while(0)
-#define virtio_rmb(vq) \
-	do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)
-#define virtio_wmb(vq) \
-	do { if ((vq)->weak_barriers) smp_wmb(); else wmb(); } while(0)
-#else
-/* We must force memory ordering even if guest is UP since host could be
- * running on another CPU, but SMP barriers are defined to barrier() in that
- * configuration. So fall back to mandatory barriers instead. */
-#define virtio_mb(vq) mb()
-#define virtio_rmb(vq) rmb()
-#define virtio_wmb(vq) wmb()
-#endif
-
-#ifdef DEBUG
-/* For development, we want to crash whenever the ring is screwed. */
-#define BAD_RING(_vq, fmt, args...)				\
-	do {							\
-		dev_err(&(_vq)->vq.vdev->dev,			\
-			"%s:"fmt, (_vq)->vq.name, ##args);	\
-		BUG();						\
-	} while (0)
-/* Caller is supposed to guarantee no reentry. */
-#define START_USE(_vq)						\
-	do {							\
-		if ((_vq)->in_use)				\
-			panic("%s:in_use = %i\n",		\
-			      (_vq)->vq.name, (_vq)->in_use);	\
-		(_vq)->in_use = __LINE__;			\
-	} while (0)
-#define END_USE(_vq) \
-	do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; } while(0)
-#else
-#define BAD_RING(_vq, fmt, args...)				\
-	do {							\
-		dev_err(&_vq->vq.vdev->dev,			\
-			"%s:"fmt, (_vq)->vq.name, ##args);	\
-		(_vq)->broken = true;				\
-	} while (0)
-#define START_USE(vq)
-#define END_USE(vq)
-#endif
-
-struct vring_virtqueue
-{
-	struct virtqueue vq;
-
-	/* Actual memory layout for this queue */
-	struct vring vring;
-
-	/* Can we use weak barriers? */
-	bool weak_barriers;
-
-	/* Other side has made a mess, don't try any more. */
-	bool broken;
-
-	/* Host supports indirect buffers */
-	bool indirect;
-
-	/* Host publishes avail event idx */
-	bool event;
-
-	/* Head of free buffer list. */
-	unsigned int free_head;
-	/* Number we've added since last sync. */
-	unsigned int num_added;
-
-	/* Last used index we've seen. */
-	u16 last_used_idx;
-
-	/* How to notify other side. FIXME: commonalize hcalls! */
-	void (*notify)(struct virtqueue *vq);
-
-#ifdef DEBUG
-	/* They're supposed to lock for us. */
-	unsigned int in_use;
-
-	/* Figure out if their kicks are too delayed. */
-	bool last_add_time_valid;
-	ktime_t last_add_time;
-#endif
-
-	/* Tokens for callbacks. */
-	void *data[];
-};
-
-#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
+#include "vring.h"
 
 /* Set up an indirect table of descriptors and add it to the queue. */
 static int vring_add_indirect(struct vring_virtqueue *vq,
diff --git a/drivers/virtio/vring.h b/drivers/virtio/vring.h
new file mode 100644
index 0000000..b997fc3
--- /dev/null
+++ b/drivers/virtio/vring.h
@@ -0,0 +1,121 @@
+/* Virtio ring implementation.
+ *
+ *  Copyright 2007 Rusty Russell IBM Corporation
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+#ifndef _LINUX_VIRTIO_RING_H_
+#define _LINUX_VIRTIO_RING_H_
+
+#include <linux/virtio_ring.h>
+#include <linux/virtio.h>
+
+struct vring_virtqueue
+{
+	struct virtqueue vq;
+
+	/* Actual memory layout for this queue */
+	struct vring vring;
+
+	/* Can we use weak barriers? */
+	bool weak_barriers;
+
+	/* Other side has made a mess, don't try any more. */
+	bool broken;
+
+	/* Host supports indirect buffers */
+	bool indirect;
+
+	/* Host publishes avail event idx */
+	bool event;
+
+	/* Number of free buffers */
+	unsigned int num_free;
+	/* Head of free buffer list. */
+	unsigned int free_head;
+	/* Number we've added since last sync. */
+	unsigned int num_added;
+
+	/* Last used index we've seen. */
+	u16 last_used_idx;
+
+	/* How to notify other side. FIXME: commonalize hcalls! */
+	void (*notify)(struct virtqueue *vq);
+
+#ifdef DEBUG
+	/* They're supposed to lock for us. */
+	unsigned int in_use;
+
+	/* Figure out if their kicks are too delayed. */
+	bool last_add_time_valid;
+	ktime_t last_add_time;
+#endif
+
+	/* Tokens for callbacks. */
+	void *data[];
+};
+#define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
+
+/* virtio guest is communicating with a virtual "device" that actually runs on
+ * a host processor.  Memory barriers are used to control SMP effects. */
+#ifdef CONFIG_SMP
+/* Where possible, use SMP barriers which are more lightweight than mandatory
+ * barriers, because mandatory barriers control MMIO effects on accesses
+ * through relaxed memory I/O windows (which virtio-pci does not use). */
+#define virtio_mb(vq) \
+	do { if ((vq)->weak_barriers) smp_mb(); else mb(); } while(0)
+#define virtio_rmb(vq) \
+	do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)
+#define virtio_wmb(vq) \
+	do { if ((vq)->weak_barriers) smp_wmb(); else wmb(); } while(0)
+#else
+/* We must force memory ordering even if guest is UP since host could be
+ * running on another CPU, but SMP barriers are defined to barrier() in that
+ * configuration. So fall back to mandatory barriers instead. */
+#define virtio_mb(vq) mb()
+#define virtio_rmb(vq) rmb()
+#define virtio_wmb(vq) wmb()
+#endif
+
+#ifdef DEBUG
+/* For development, we want to crash whenever the ring is screwed. */
+#define BAD_RING(_vq, fmt, args...)				\
+	do {							\
+		dev_err(&(_vq)->vq.vdev->dev,			\
+			"%s:"fmt, (_vq)->vq.name, ##args);	\
+		BUG();						\
+	} while (0)
+/* Caller is supposed to guarantee no reentry. */
+#define START_USE(_vq)						\
+	do {							\
+		if ((_vq)->in_use)				\
+			panic("%s:in_use = %i\n",		\
+			      (_vq)->vq.name, (_vq)->in_use);	\
+		(_vq)->in_use = __LINE__;			\
+	} while (0)
+#define END_USE(_vq) \
+	do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; } while(0)
+#else
+#define BAD_RING(_vq, fmt, args...)				\
+	do {							\
+		dev_err(&_vq->vq.vdev->dev,			\
+			"%s:"fmt, (_vq)->vq.name, ##args);	\
+		(_vq)->broken = true;				\
+	} while (0)
+#define START_USE(vq)
+#define END_USE(vq)
+#endif
+
+#endif
-- 
1.7.9.5

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related

* [RFC virtio-next 0/4] Introduce CAIF Virtio and reversed Vrings
From: Sjur Brændeland @ 2012-10-31 22:46 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Michael S. Tsirkin, netdev, Linus Walleij, dmitry.tarnyagin,
	linux-kernel, virtualization, sjur

This patch-set introduces the CAIF Virtio Link layer. The purpose is to
communicate with a remote processor (a modem) over shared memory. Virtio
is used as the transport mechanism, and the Remoteproc framework provides
configuration and management of the Virtio rings and devices. The modem
and Linux host may be on the same SoC, or connected over a shared memory
interface such as LLI.

Zero-Copy data transport on the modem is primary goal for CAIF Virtio.
In order to achieve Zero-Copy the direction of the Virtio rings are
flipped in the RX direction. So we have implemented the Virtio
access-function similar to what is found in vhost.c.

The connected LTE-modem is a multi-processor system with an advanced
memory allocator on board. In order to provide zero-copy from the modem
to the connected Linux host, the direction of the Virtio rings are
reversed. This allows the modem to allocate data-buffers in RX
direction and pass them to the Linux host, and recycled buffers to be
sent back to the modem.

The option of providing pre-allocated buffers in RX direction has been
considered, but rejected. The allocation of data buffers happens deep
down in the network signaling stack on the LTE modem before it is known
what type of data is received. It may be data that is handled within the
modem and never sent to the Linux host, or IP traffic going to the host.
Pre-allocated Virtio buffers does not fit for this usage. Another issue
is that the network traffic pattern may vary, resulting in variation of
number and size of buffers allocated from the memory allocator. Dynamic
allocation is needed in order to utilize memory properly. Due to this,
we decided we had to implement "reversed" vrings. Reversed vrings allows
us to minimize the impact on the current memory allocator and buffer
handling on the modem. 

In order to implement reversed rings we have added functions for reading
descriptors from the available-ring and adding descriptors to the used-ring.
The internal data-structures in virtio_ring.c are moved into a new header
file so the data-structures can be accessed by caif_virtio.

The data buffers in TX direction are allocated using dma_alloc_coherent().
This allows memory to be allocated from the memory region shared between
the Host and modem.

In TX direction single linearized TX buffers are added to the vring. In
RX direction linearized frames are also used, but multiple descriptors may
be linked. This is done to allow maximum efficiency for the LTE modem.

This patch set is not yet fully tested and does not handle all negative
scenarios correctly. So at this stage we're primarily looking for review
comments related to the structure of the Virtio code. There are several
options on how to structure this, and feedback is welcomed.

Thanks,
Sjur

Sjur Brændeland (4):
  virtio: Move definitions to header file vring.h
  include/vring.h: Add support for reversed vritio rings.
  virtio_ring: Call callback function even when used ring is empty
  caif_virtio: Add CAIF over virtio

 drivers/net/caif/Kconfig               |    9 +
 drivers/net/caif/Makefile              |    3 +
 drivers/net/caif/caif_virtio.c         |  627 ++++++++++++++++++++++++++++++++
 drivers/remoteproc/remoteproc_virtio.c |    2 +-
 drivers/virtio/virtio_ring.c           |  102 +-----
 drivers/virtio/vring.h                 |  124 +++++++
 include/linux/virtio_ring.h            |    8 +-
 include/uapi/linux/virtio_ids.h        |    1 +
 8 files changed, 776 insertions(+), 100 deletions(-)
 create mode 100644 drivers/net/caif/caif_virtio.c
 create mode 100644 drivers/virtio/vring.h

-- 
1.7.9.5

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: vlan tagged packets and libpcap breakage
From: Ani Sinha @ 2012-10-31 22:35 UTC (permalink / raw)
  To: Guy Harris; +Cc: netdev, tcpdump-workers, Michael Richardson, Francesco Ruggeri
In-Reply-To: <5422DBB2-EABF-4C9F-B0CD-8C77E91F9FF8@alum.mit.edu>

On Wed, Oct 31, 2012 at 3:20 PM, Guy Harris <guy@alum.mit.edu> wrote:
>
> On Oct 31, 2012, at 2:50 PM, Ani Sinha <ani@aristanetworks.com> wrote:
>
>> pcap files that already have the tags reinsrted should work with
>> current filter code. However for live traffic, one has to get the tags
>> from CMSG() and then reinsert it back to the packet for the current
>> filter to work.
>
> *Somebody* has to do that, at least to packets that pass the filter,


yes but if the packet is passed to the filter within libpcap (when we
are not using the kernel filter) before the reinsertion, then the
filter has to be taught to look into the metadata for tag information
and not in the packet itself.

before they're handed to a libpcap-based application, for programs
that expect to see packets as they arrived from/were transmitted to
the wire to work.
>
> I.e., the tags *should* be reinserted by libpcap, and, as I understand it, that's what the
>
>         #if defined(HAVE_PACKET_AUXDATA) && defined(HAVE_LINUX_TPACKET_AUXDATA_TP_VLAN_TCI)
>                 ...
>         #endif
>
> blocks of code in pcap-linux.c in libpcap are doing.

yes, I agree.
_______________________________________________
tcpdump-workers mailing list
tcpdump-workers@lists.tcpdump.org
https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers

^ permalink raw reply

* Re: [net PATCH V2] net: fix divide by zero in tcp algorithm illinois
From: Jesper Dangaard Brouer @ 2012-10-31 22:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, netdev, Petr Matousek, Eric Dumazet
In-Reply-To: <20121031134836.16b31625@s6510.linuxnetplumber.net>

On Wed, 2012-10-31 at 13:48 -0700, Stephen Hemminger wrote:
> On Wed, 31 Oct 2012 13:45:32 +0100
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> 
> > Reading TCP stats when using TCP Illinois congestion control algorithm
> > can cause a divide by zero kernel oops.
[...]
> 
> Acked-by: Stephen Hemminger <shemminger@vyatta.com>

When DaveM is without power, people even have/get time to review and ack
my patches ;-). Thanks Stephen and Eric.

--Jesper

^ permalink raw reply

* Re: [PATCH v2 2/3] pppoatm: fix race condition with destroying of vcc
From: Krzysztof Mazur @ 2012-10-31 22:04 UTC (permalink / raw)
  To: chas williams - CONTRACTOR; +Cc: davem, dwmw2, netdev, linux-kernel
In-Reply-To: <20121031160352.68353ecc@thirdoffive.cmf.nrl.navy.mil>

On Wed, Oct 31, 2012 at 04:03:52PM -0400, chas williams - CONTRACTOR wrote:
> 
> reversing the order of the push and close certainly seems like the right
> thing to do. i would like to see if it would fix your problem.  making
> the minimal change to get something working would be preferred before
> adding additional complexity.  i am just surprised we havent seen this
> bug before.

Yes, it fixes the problem and it's probably the best fix for original
issue.

There are also some minor potential issues in pppoatm driver:

	- locking issues, but now only between pppoatm_send() and
	  vcc_sendmsg() and maybe some other functions,

	- missing check for SS_CONNECTED in pppoatm_ioctl,

	- problem described in comment in pppoatm_may_send() when
	  sk->sk_sndbuf < MTU, sk_wmem_alloc_get() should be added
	  there

but I think that for now the patch that changes the order of push
and close is sufficient.

I probably saw that bug a log time ago (around 2.6.30), but it was
too rare to see what caused panic, but after 
9d02daf754238adac48fa075ee79e7edd3d79ed3 (pppoatm: Fix excessive queue bloat)
this bug occurs much more frequently.

Thanks.

Krzysiek

-- >8 --
Subject: [PATCH] atm: detach protocol before closing vcc

The vcc_destroy_socket() closes vcc before the protocol is detached
from vcc by calling vcc->push() with NULL skb. This leaves some time
window, where the protocol may call vcc->send() on closed vcc.

It happens at least with pppoatm protocol and usbatm driver, and causes
an Oops:

Oops: 0000 [#1] PREEMPT
Pid: 0, comm: swapper Not tainted 3.6.0-krzysiek-00001-gb7cd93b-dirty #60    /AK32
EIP: 0060:[<c01413c6>] EFLAGS: 00010082 CPU: 0
EIP is at __wake_up_common+0x16/0x70
EAX: 30707070 EBX: 00000292 ECX: 00000001 EDX: dca75fc0
ESI: 00000000 EDI: de7f500f EBP: df409f24 ESP: df409f08
 DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
CR0: 8005003b CR2: 30707070 CR3: 1c920000 CR4: 000007d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process swapper (pid: 0, ti=df408000 task=c07bd4e0 task.ti=c07b0000)
Stack:
 00000000 00000001 00000001 dca75fc0 00000292 00000000 de7f500f df409f3c
 c0143299 00000000 00000000 dc84f000 dc84f000 df409f4c c0602bf0 00000000
 dc84f000 df409f58 c0604301 dc840cc0 df409fb4 c04672e5 c076a240 00000000
Call Trace:
 [<c0143299>] __wake_up+0x29/0x50
 [<c0602bf0>] vcc_write_space+0x40/0x80
 [<c0604301>] atm_pop_raw+0x21/0x30
 [<c04672e5>] usbatm_tx_process+0x2a5/0x380
 [<c0126cf9>] tasklet_action+0x39/0x70
 [<c0126f1f>] __do_softirq+0x7f/0x120
 [<c0126ea0>] ? local_bh_enable_ip+0xa0/0xa0
 <IRQ>

Now the protocol is detached before vcc is closed.

Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
 net/atm/common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/atm/common.c b/net/atm/common.c
index 0c0ad93..a0e4411 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -171,10 +171,10 @@ static void vcc_destroy_socket(struct sock *sk)
 	set_bit(ATM_VF_CLOSE, &vcc->flags);
 	clear_bit(ATM_VF_READY, &vcc->flags);
 	if (vcc->dev) {
-		if (vcc->dev->ops->close)
-			vcc->dev->ops->close(vcc);
 		if (vcc->push)
 			vcc->push(vcc, NULL); /* atmarpd has no push */
+		if (vcc->dev->ops->close)
+			vcc->dev->ops->close(vcc);
 
 		while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
 			atm_return(vcc, skb->truesize);
-- 
1.8.0.172.g62af90c

^ permalink raw reply related

* Re: [tcpdump-workers] vlan tagged packets and libpcap breakage
From: Ani Sinha @ 2012-10-31 21:50 UTC (permalink / raw)
  To: Michael Richardson, netdev; +Cc: tcpdump-workers, Francesco Ruggeri
In-Reply-To: <3246.1351717319@obiwan.sandelman.ca>

cc'ing netdev.


On Wed, Oct 31, 2012 at 2:01 PM, Michael Richardson <mcr@sandelman.ca> wrote:
>
> Thanks for this email.
>
>>>>>> "Ani" == Ani Sinha <ani@aristanetworks.com> writes:
>     Ani> remove "inline" from vlan_core.c functions
>     Ani> Signed-off-by: David S. Miller <davem@davemloft.net>
>
>     Ani> As a result of this patch, with or without HW acceleration support in
>     Ani> the kernel driver, the vlan tag information is pulled out from the
>     Ani> packet and is inserted in the skb metadata. Thereafter, the vlan tag
>     Ani> information for a 802.1q packet can be obtained my applications
>     Ani> running in the user space by using the auxdata and cmsg
>     Ani> structure.
>
> Do you think that the existance of this behaviour could be exposed in
> sysctl, /proc/net or /sys equivalent (we still don't have /sys/net...)?
> As a read only file that had a 0/1 in it?

yes, we definitely need a run time check. Whether this could be in the
form of a socket option or a /proc entry I don't know.

>
> Should be one stanza to net/dev/sysctl_net_core, I think.
> If we are fast, maybe no kernel will ship without it.
>
> (An aside, is this auxdata/cmsg info available on UDP sockets too?)
>
>     Ani> More recently, two more patches were committed to net-next that enable
>     Ani> the kernel BPF filter to filter packets using their vlan tags :
>
>     Ani> http://www.spinics.net/lists/netdev/msg214758.html
>     Ani> http://www.spinics.net/lists/netdev/msg214759.html
>
> okay...
>
>     Ani> Now to the real problem. The filter that is currently generated by
>     Ani> libpcap (gencode.c) uses offsets into the packet to obtain the vlan
>     Ani> tag for a packet and then compare it to the one provided by the user
>     Ani> in the filter expression (gen_vlan()). This will no longer work if the
>     Ani> tag has been pulled off from the packet itself. One has to use the
>     Ani> negative offsets (SKF_AD_VLAN_TAG) as used by the kernel BPF filter to
>     Ani> pass down the attribute that we need to compare against.  Now here's a
>     Ani> bigger problem. How do we avoid breakage in case we run libpcap on top
>     Ani> of an older kernel that does not extract the tags from the packet? How
>     Ani> do we handle cases of parsing and filtering against captured pcap
>     Ani> files that already have the vlan tags reinserted into the packet data?
>     Ani> There might be other corner cases that I am missing and not
>     Ani> understanding here.
>
> AFAIK, We will have to modify pcap-linux to produce different filters.
>
>     Ani> Are you guys aware of the problem? Is there any thoughts as to how we
>     Ani> may be able to address the problem or not at all?
>
> Nobody gave us a heads up, no.

Glad that I brought this up. Actually Bill Fenner who also works for
us helped me a lot in understanding the problem and potentially how it
can be addressed.


>
> It sounds like it's impossible to capture all traffic now, and do vlan
> filtering later on?

pcap files that already have the tags reinsrted should work with
current filter code. However for live traffic, one has to get the tags
from CMSG() and then reinsert it back to the packet for the current
filter to work. This is slow.

ani

^ permalink raw reply

* Re: [PATCH net-next 1/8] bridge: bridge port parameters over netlink
From: Stephen Hemminger @ 2012-10-31 21:30 UTC (permalink / raw)
  To: John Fastabend; +Cc: davem, netdev
In-Reply-To: <50912F4D.2090602@intel.com>

On Wed, 31 Oct 2012 07:01:49 -0700
John Fastabend <john.r.fastabend@intel.com> wrote:

> On 10/29/2012 5:57 PM, Stephen Hemminger wrote:
> > Expose bridge port parameters over netlink. By switching
> > from a single byet to a nested message, this can be used for
> > other bridge parameters.
> >
> > Although, this changes IFLA_PROTINFO attribute from one byte to a full nested
> > set of attributes; it is safe for applications because the
> > old message used IFLA_PROTINFO and new one uses
> >   IFLA_PROTINFO | NLA_F_NESTED.
> >
> > The code still accepts to old format requests, and therefore stays
> > compatiable with user mode RSTP daemon. Since the type field
> > for nested and unnested attributes are different, and the old
> > code in libnetlink doesn't do the mask, it is also safe to use
> > with old versions of bridge monitor command.
> >
> > Note: although mode is only a boolean, treating it as a
> > full byte since in the future someone will probably want to add more
> > values (like macvlan has).
> >
> > Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
> >
> > ---
> 
> Stephen,
> 
> Did you see these two patches
> 
> http://patchwork.ozlabs.org/patch/193900/
> http://patchwork.ozlabs.org/patch/193901/
> 
> here I added nested bridge attributes to IFLA_AF_SPEC and pass them down
> to the drivers as needed. Should we merge these two sets so that we have
> only a single nested set of bridge attributes? Either in IFLA_AF_SPEC or
> IFLA_PROTINFO.
> 
> The attributes in this patch are port specifics and the one in the
> patches listed above are bridge specific so in this sense perhaps
> its OK to keep them separate. I'm not sure it matters much either
> way but thought I would mention it.
> 
> Also I suspect these two series will have conflicts but I haven't tried
> yet.
> 
> Thanks,
> John

This is an area where there is no clear choice.
I would like to keep AF_UNSPEC for non-protocol stuff,
that is why I targeted PF_BRIDGE:IFLA_PROTINFO.

Other alternative would be to add sysctl which is less message
based, but is more general. (ie. /default and /all are available).

^ permalink raw reply

* Re: [PATCH net-next v2] packet: tx_ring: allow the user to choose tx data offset
From: Paul Chavent @ 2012-10-31 21:22 UTC (permalink / raw)
  To: David Miller; +Cc: Paul.Chavent, netdev
In-Reply-To: <20121031.131434.910561089094397853.davem@davemloft.net>

On 10/31/2012 06:14 PM, David Miller wrote:
> From: Paul Chavent <Paul.Chavent@onera.fr>
> Date: Mon, 22 Oct 2012 08:56:54 +0200
>
>> +		if (unlikely((off < off_min) || (off_max < off))) {
>> +			pr_err("payload offset (%d) out of range [%d;%d]\n",
>> +				off, off_min, off_max);
>> +			return -EINVAL;
>> +		}
>
> Users should not be able to spam the kernel log with error messages
> by simply make setsockopt() calls.
>
> This error log is inappropriate.

This error log is thrown when the user call send, not setsockopt !

It seems that the user can already fire an error msg by simply pass a too long tp_len.

Though, should i remove it from the tpacket_fill_skb function and silently return -EINVAL ?


>> @@ -110,6 +110,7 @@ struct packet_sock {
>>   	unsigned int		tp_reserve;
>>   	unsigned int		tp_loss:1;
>>   	unsigned int		tp_tstamp;
>> +	unsigned int		tp_tx_has_off:1;
>>   	struct packet_type	prot_hook ____cacheline_aligned_in_smp;
>>   };
>
> Please put the new bit field next to other existing bit fields so that
> there is less wasted space in the struct.

OK.

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

^ permalink raw reply

* Re: skb_linearize
From: Ben Hutchings @ 2012-10-31 21:03 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, Herbert Xu
In-Reply-To: <20121031191530.GA3674@redhat.com>

On Wed, 2012-10-31 at 21:15 +0200, Michael S. Tsirkin wrote:
> On Sun, Sep 16, 2012 at 04:07:12PM +0100, Ben Hutchings wrote:
> > On Sun, 2012-09-16 at 12:17 +0300, Michael S. Tsirkin wrote:
> > > I notice that dev_hard_start_xmit might invoke
> > > __skb_linearize e.g. if device does not support NETIF_F_SG.
> > > 
> > > This in turn onvokes __pskb_pull_tail, and
> > > documentation of __pskb_pull_tail says:
> > >   &sk_buff MUST have reference count of 1.
> > > 
> > > I am guessing 'reference count' means users in this context, right?
> > > IIUC this is because it modifies skb in a way that
> > > isn't safe if anyone else is looking at the skb.
> > > 
> > > 
> > > However, I don't see what guarantees that reference
> > > count is 1 when dev_hard_start_xmit invokes
> > > linearize. In particular it calls dev_queue_xmit_nit
> > > which could queue packets on a network tap.
> > > 
> > > Could someone help me understand please?
> > 
> > Reference count here means references to struct sk_buff itself.  The
> > header area and data fragments are allowed to be shared.
> > 
> > dev_queue_xmit_nit() clones the skb for each tap, so the reference count
> > on the original skb remains 1.
> > 
> > Ben.
> 
> Interesting. But don't skb clones share the fragment list?

Yes.

> Maybe I misunderstand? If they do it looks like the following race
> would be possible:
> 
> - skb is cloned and queued e.g. at socket receive queue.
>   dataref becomes 2.
> - On CPU 1, skb_copy_datagram_iovec is called on clone 1, is reads nr_frags and sees
>   value > 1.
> - On CPU 2, __skb_linearize is now called on clone 2, it modified the
>   skb so nr_frags is now 0, and does put_page for all frags > 1.
> - On CPU 1, skb_copy_datagram_iovec will now use the previously read
>   nr_frags > 1 and access a fragment page that was already freed.
> 
> What did I miss?

__skb_linearize() calls __pskb_pull_tail(), which starts with:

	if (eat > 0 || skb_cloned(skb)) {
		if (pskb_expand_head(skb, 0, eat > 0 ? eat + 128 : 0,
				     GFP_ATOMIC))
			return NULL;
	}

pskb_expand_head() will then create a new unshared head area for the skb
being linearised, and will add a reference to each fragment page
(skb_frag_ref()).  __pskb_pull_tail() unreferences the pages later, as
you say, but this all cancels out.

(pskb_expand_head() will also cancel zero-copy (skb_orphan_frags() ->
skb_copy_ubufs()), which seems like it should be done only after the
head area has been unshared.  But skb_clone() will also do that, so I
don't know how the zero-copy flag would still be set on a cloned skb.)

Ben.

-- 
Ben Hutchings, Staff 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 net-next 3/8] bridge: implement BPDU blocking
From: Stephen Hemminger @ 2012-10-31 20:57 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev
In-Reply-To: <k6q2vm$1ug$1@ger.gmane.org>

On Wed, 31 Oct 2012 02:38:46 +0000 (UTC)
Cong Wang <xiyou.wangcong@gmail.com> wrote:

> On Tue, 30 Oct 2012 at 00:57 GMT, Stephen Hemminger <shemminger@vyatta.com> wrote:
> > --- a/net/bridge/br_stp_bpdu.c	2012-10-29 17:07:08.005982838 -0700
> > +++ b/net/bridge/br_stp_bpdu.c	2012-10-29 17:31:14.327457434 -0700
> > @@ -170,6 +170,13 @@ void br_stp_rcv(const struct stp_proto *
> >  	if (!ether_addr_equal(dest, br->group_addr))
> >  		goto out;
> >  
> > +	if (p->flags & BR_BPDU_GUARD) {
> > +		br_notice(br, "BPDU received on blocked port %u(%s)\n",
> > +			  (unsigned int) p->port_no, p->dev->name);
> 
> net_ratelimit() ?
> 

Not necessary since this causes port of bridge to become disabled;
which blocks all further packets (including STP) until manually reset.

^ permalink raw reply

* Re: [PATCH net-next] sockopt: Change getsockopt() of SO_BINDTODEVICE to return an interface name
From: Andi Kleen @ 2012-10-31 20:47 UTC (permalink / raw)
  To: Brian Haley
  Cc: David Miller, Pavel Emelyanov, Eric Dumazet,
	netdev@vger.kernel.org
In-Reply-To: <509184D9.8030103@hp.com>

Brian Haley <brian.haley@hp.com> writes:

> Instead of having the getsockopt() of SO_BINDTODEVICE return an index, which
> will then require another call like if_indextoname() to get the actual interface
> name, have it return the name directly.
>
> This also matches the existing man page description on socket(7) which mentions
> the argument being an interface name.
>
> If the value has not been set, zero is returned and optlen will be set to zero
> to indicate there is no interface name present.

That will break all existing programs using the return value, right?
Better to fix the manpage

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

^ permalink raw reply

* Fw: [Bug 42764] BUG at net/core/skbuff.c:147
From: Stephen Hemminger @ 2012-10-31 20:51 UTC (permalink / raw)
  To: Michael Chan; +Cc: netdev



Begin forwarded message:

Date: Wed, 31 Oct 2012 08:55:31 +0000 (UTC)
From: bugzilla-daemon@bugzilla.kernel.org
To: shemminger@linux-foundation.org
Subject: [Bug 42764] BUG at net/core/skbuff.c:147


https://bugzilla.kernel.org/show_bug.cgi?id=42764


Guido Aulisi <guido.aulisi@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |guido.aulisi@gmail.com




--- Comment #3 from Guido Aulisi <guido.aulisi@gmail.com>  2012-10-31 08:55:31 ---
I got it one more time. Ethernet card is:
0e:00.0 Ethernet controller: Broadcom Corporation NetXtreme II BCM5709S Gigabit
Ethernet (rev 20)

ethtool -i eth0
driver: bnx2
version: 2.1.6
firmware-version: bc 4.4.14 IPMI 0.1.16
bus-info: 0000:04:00.0

ethtool -i eth2
driver: bnx2
version: 2.1.6
firmware-version: bc 4.4.10
bus-info: 0000:0e:00.0

Could this be related to an old firmware?

This machine does a lot of network traffic, because it load data from a
database every night.

Kernel version is 3.0.34

Sorry for not reporting hardware, I thought it was a protocol bug not related
to a specific card.

------------[ cut here ]------------
kernel BUG at net/core/skbuff.c:147!
invalid opcode: 0000 [#1] SMP 
CPU 2 
Modules linked in: af_packet ipmi_devintf ipmi_si ipmi_msghandler ipv6 fuse
loop sr_mod cdrom mptsas mptscsih mptbase scsi_transport_sas tpm_tis tpm bnx2
tpm_bios sg i2c_piix4 i2c_core shpchp pci_hotplug button serio_raw linear
scsi_dh_alua scsi_dh_rdac scsi_dh_hp_sw scsi_dh_emc dm_round_robin sd_mod
crc_t10dif qla2xxx scsi_transport_fc scsi_tgt dm_snapshot dm_multipath scsi_dh
scsi_mod edd dm_mod ext3 mbcache jbd fan thermal processor thermal_sys hwmon
[last unloaded: usbcore]

Pid: 21566, comm: nscd Not tainted 3.0.34-inps #5 IBM BladeCenter LS42
-[7902CQG]-/Server Blade
RIP: 0010:[<ffffffff8123fd12>]  [<ffffffff8123fd12>] skb_push+0x75/0x7e
RSP: 0018:ffff880b956c39d8  EFLAGS: 00010292
RAX: 0000000000000083 RBX: 0000000000000800 RCX: 0000000000023382
RDX: 0000000000007878 RSI: 0000000000000046 RDI: ffffffff8152ee9c
RBP: ffff880b956c39f8 R08: 0000000000000000 R09: 0720072007200720
R10: ffff880b956c37c8 R11: 0720072007200720 R12: 0000000000000000
R13: ffff8810231fb718 R14: 0000000000000055 R15: ffff880c25d24000
FS:  00007fd9a9222950(0000) GS:ffff880c3fc00000(0000) knlGS:00000000e8dafb90
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 00007fd9b403a000 CR3: 0000000c265e8000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process nscd (pid: 21566, threadinfo ffff880b956c2000, task ffff8803e50e4790)
Stack:
 0000000000000057 0000000000000080 ffff880c25d24000 ffff880c71806440
 ffff880b956c3a38 ffffffff8125e0c5 ffff880b956c3a28 0000000000000036
 ffff8810231fb680 ffff8810231fb718 ffff880b2cc49380 ffff8810231fb710
Call Trace:
 [<ffffffff8125e0c5>] eth_header+0x29/0xa8
 [<ffffffff81251a59>] neigh_resolve_output+0x284/0x2ed
 [<ffffffff8127575e>] ip_finish_output+0x24c/0x293
 [<ffffffff81097cac>] ? zone_watermark_ok+0x1a/0x1c
 [<ffffffff81275846>] ip_output+0xa1/0xa5
 [<ffffffff81274b92>] ip_local_out+0x24/0x29
 [<ffffffff81274ba0>] ip_send_skb+0x9/0x4c
 [<ffffffff8129178b>] udp_send_skb+0x28b/0x2e8
 [<ffffffff812935cb>] udp_sendmsg+0x501/0x708
 [<ffffffff810e42dd>] ? poll_freewait+0x8d/0x8d
 [<ffffffff812740ec>] ? ip_append_page+0x4f4/0x4f4
 [<ffffffff8109b8f2>] ? __alloc_pages_nodemask+0x731/0x77c
 [<ffffffff810541ad>] ? sched_clock_local+0x1c/0x80
 [<ffffffff81299607>] inet_sendmsg+0x83/0x90
 [<ffffffff81238e93>] sock_sendmsg+0xe3/0x106
 [<ffffffff810c7c8a>] ? ____cache_alloc_node+0x4c/0x132
 [<ffffffff810c7b4f>] ? fallback_alloc+0xe1/0x1d0
 [<ffffffff8126e390>] ? __ip_route_output_key+0x139/0x816
 [<ffffffff812d24e3>] ? _raw_spin_unlock_bh+0xf/0x11
 [<ffffffff8123ba9e>] ? release_sock+0xfd/0x106
 [<ffffffff81239463>] sys_sendto+0xfa/0x123
 [<ffffffff81239c3d>] ? sys_connect+0x88/0x9c
 [<ffffffff812d78bb>] system_call_fastpath+0x16/0x1b
Code: 8b 57 68 48 89 44 24 10 8b 87 d0 00 00 00 48 89 44 24 08 8b bf cc 00 00
00 31 c0 48 89 3c 24 48 c7 c7 5a 79 3a 81 e8 df 03 09 00 <0f> 0b eb fe 48 89 c8
c9 c3 55 89 f1 48 89 e5 48 83 ec 20 83 7f 
RIP  [<ffffffff8123fd12>] skb_push+0x75/0x7e
 RSP <ffff880b956c39d8>
---[ end trace 7e8c514a1c33a3f0 ]---

Output of lspci -vvv:

0e:00.0 Ethernet controller: Broadcom Corporation NetXtreme II BCM5709S Gigabit
Ethernet (rev 20)
    Subsystem: IBM Device 039f
    Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+
Stepping- SERR+ FastB2B- DisINTx+
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-
    Latency: 0, Cache Line Size: 64 bytes
    Interrupt: pin A routed to IRQ 26
    Region 0: Memory at ce000000 (64-bit, non-prefetchable) [size=32M]
    Capabilities: [48] Power Management version 3
        Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
PME(D0+,D1-,D2-,D3hot+,D3cold+)
        Status: D0 PME-Enable- DSel=0 DScale=1 PME-
    Capabilities: [50] Vital Product Data <?>
    Capabilities: [58] Message Signalled Interrupts: Mask- 64bit+ Count=1/8
Enable-
        Address: 0000000000000000  Data: 0000
    Capabilities: [a0] MSI-X: Enable+ Mask- TabSize=9
        Vector table: BAR=0 offset=0000c000
        PBA: BAR=0 offset=0000e000
    Capabilities: [ac] Express (v2) Endpoint, MSI 00
        DevCap:    MaxPayload 512 bytes, PhantFunc 0, Latency L0s <4us, L1
<64us
            ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
        DevCtl:    Report errors: Correctable- Non-Fatal+ Fatal+ Unsupported-
            RlxdOrd+ ExtTag- PhantFunc- AuxPwr+ NoSnoop+
            MaxPayload 256 bytes, MaxReadReq 4096 bytes
        DevSta:    CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr+ TransPend-
        LnkCap:    Port #0, Speed 2.5GT/s, Width x4, ASPM L0s L1, Latency L0
<4us, L1 <4us
            ClockPM- Suprise- LLActRep- BwNot-
        LnkCtl:    ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
            ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
        LnkSta:    Speed 2.5GT/s, Width x4, TrErr- Train- SlotClk+ DLActive-
BWMgmt- ABWMgmt-
        DevCap2: Completion Timeout: Range ABCD, TimeoutDis+
        DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis+
        LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-,
Selectable De-emphasis: -6dB
             Transmit Margin: Normal Operating Range, EnterModifiedCompliance-
ComplianceSOS-
             Compliance De-emphasis: -6dB
        LnkSta2: Current De-emphasis Level: -6dB
    Kernel driver in use: bnx2
    Kernel modules: bnx2

0e:00.1 Ethernet controller: Broadcom Corporation NetXtreme II BCM5709S Gigabit
Ethernet (rev 20)
    Subsystem: IBM Device 039f
    Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+
Stepping- SERR+ FastB2B- DisINTx-
    Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort-
<MAbort- >SERR- <PERR- INTx-
    Latency: 0, Cache Line Size: 64 bytes
    Interrupt: pin B routed to IRQ 26
    Region 0: Memory at d0000000 (64-bit, non-prefetchable) [size=32M]
    Capabilities: [48] Power Management version 3
        Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA
PME(D0+,D1-,D2-,D3hot+,D3cold+)
        Status: D0 PME-Enable- DSel=0 DScale=1 PME-
    Capabilities: [50] Vital Product Data <?>
    Capabilities: [58] Message Signalled Interrupts: Mask- 64bit+ Count=1/8
Enable-
        Address: 0000000000000000  Data: 0000
    Capabilities: [a0] MSI-X: Enable- Mask- TabSize=8
        Vector table: BAR=0 offset=0000c000
        PBA: BAR=0 offset=0000e000
    Capabilities: [ac] Express (v2) Endpoint, MSI 00
        DevCap:    MaxPayload 512 bytes, PhantFunc 0, Latency L0s <4us, L1
<64us
            ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
        DevCtl:    Report errors: Correctable- Non-Fatal+ Fatal+ Unsupported-
            RlxdOrd+ ExtTag- PhantFunc- AuxPwr+ NoSnoop+
            MaxPayload 256 bytes, MaxReadReq 4096 bytes
        DevSta:    CorrErr+ UncorrErr- FatalErr- UnsuppReq+ AuxPwr+ TransPend-
        LnkCap:    Port #0, Speed 2.5GT/s, Width x4, ASPM L0s L1, Latency L0
<4us, L1 <4us
            ClockPM- Suprise- LLActRep- BwNot-
        LnkCtl:    ASPM Disabled; RCB 128 bytes Disabled- Retrain- CommClk-
            ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
        LnkSta:    Speed 2.5GT/s, Width x4, TrErr- Train- SlotClk+ DLActive-
BWMgmt- ABWMgmt-
        DevCap2: Completion Timeout: Range ABCD, TimeoutDis+
        DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis+
        LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-,
Selectable De-emphasis: -6dB
             Transmit Margin: Normal Operating Range, EnterModifiedCompliance-
ComplianceSOS-
             Compliance De-emphasis: -6dB
        LnkSta2: Current De-emphasis Level: -6dB
    Kernel driver in use: bnx2
    Kernel modules: bnx2

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

^ permalink raw reply

* Re: [net PATCH V2] net: fix divide by zero in tcp algorithm illinois
From: Stephen Hemminger @ 2012-10-31 20:48 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: David S. Miller, netdev, Petr Matousek, Eric Dumazet
In-Reply-To: <20121031124318.30915.32293.stgit@dragon>

On Wed, 31 Oct 2012 13:45:32 +0100
Jesper Dangaard Brouer <brouer@redhat.com> wrote:

> Reading TCP stats when using TCP Illinois congestion control algorithm
> can cause a divide by zero kernel oops.
> 
> The division by zero occur in tcp_illinois_info() at:
>  do_div(t, ca->cnt_rtt);
> where ca->cnt_rtt can become zero (when rtt_reset is called)
> 
> Steps to Reproduce:
>  1. Register tcp_illinois:
>      # sysctl -w net.ipv4.tcp_congestion_control=illinois
>  2. Monitor internal TCP information via command "ss -i"
>      # watch -d ss -i
>  3. Establish new TCP conn to machine
> 
> Either it fails at the initial conn, or else it needs to wait
> for a loss or a reset.
> 
> This is only related to reading stats.  The function avg_delay() also
> performs the same divide, but is guarded with a (ca->cnt_rtt > 0) at its
> calling point in update_params().  Thus, simply fix tcp_illinois_info().
> 
> Function tcp_illinois_info() / get_info() is called without
> socket lock.  Thus, eliminate any race condition on ca->cnt_rtt
> by using a local stack variable.  Simply reuse info.tcpv_rttcnt,
> as its already set to ca->cnt_rtt.
> Function avg_delay() is not affected by this race condition, as
> its called with the socket lock.
> 
> Cc: Petr Matousek <pmatouse@redhat.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> 
> ---
> V2:
>  Address Eric Dumazets input:
>   - Save 2 bytes of stack, by using info.tcpv_rttcnt.
>   - Help compiler, and define "u64 t" inside if() lexical scope.
> 
> 
>  net/ipv4/tcp_illinois.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/tcp_illinois.c b/net/ipv4/tcp_illinois.c
> index 813b43a..834857f 100644
> --- a/net/ipv4/tcp_illinois.c
> +++ b/net/ipv4/tcp_illinois.c
> @@ -313,11 +313,13 @@ static void tcp_illinois_info(struct sock *sk, u32 ext,
>  			.tcpv_rttcnt = ca->cnt_rtt,
>  			.tcpv_minrtt = ca->base_rtt,
>  		};
> -		u64 t = ca->sum_rtt;
>  
> -		do_div(t, ca->cnt_rtt);
> -		info.tcpv_rtt = t;
> +		if (info.tcpv_rttcnt > 0) {
> +			u64 t = ca->sum_rtt;
>  
> +			do_div(t, info.tcpv_rttcnt);
> +			info.tcpv_rtt = t;
> +		}
>  		nla_put(skb, INET_DIAG_VEGASINFO, sizeof(info), &info);
>  	}
>  }
> 

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

^ permalink raw reply


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