* Re: BUG cxgb3: Check and handle the dma mapping errors
From: Divy Le ray @ 2013-08-07 16:55 UTC (permalink / raw)
To: Jay Fenlason
Cc: Alexey Kardashevskiy, Santosh Rastapur, David S. Miller,
netdev@vger.kernel.org, Linux Kernel Mailing List
In-Reply-To: <20130805184107.GA2998@redhat.com>
On 08/05/2013 11:41 AM, Jay Fenlason wrote:
> On Mon, Aug 05, 2013 at 12:59:04PM +1000, Alexey Kardashevskiy wrote:
>> Hi!
>>
>> Recently I started getting multiple errors like this:
>>
>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>> c000001fbdaaa882 npages 1
>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>> c000001fbdaaa882 npages 1
>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>> c000001fbdaaa882 npages 1
>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>> c000001fbdaaa882 npages 1
>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>> c000001fbdaaa882 npages 1
>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>> c000001fbdaaa882 npages 1
>> cxgb3 0006:01:00.0: iommu_alloc failed, tbl c000000003067980 vaddr
>> c000001fbdaaa882 npages 1
>> ... and so on
>>
>> This is all happening on a PPC64 "powernv" platform machine. To trigger the
>> error state, it is enough to _flood_ ping CXGB3 card from another machine
>> (which has Emulex 10Gb NIC + Cisco switch). Just do "ping -f 172.20.1.2"
>> and wait 10-15 seconds.
>>
>>
>> The messages are coming from arch/powerpc/kernel/iommu.c and basically
>> mean that the driver requested more pages than the DMA window has which is
>> normally 1GB (there could be another possible source of errors -
>> ppc_md.tce_build callback - but on powernv platform it always succeeds).
>>
>>
>> The patch after which it broke is:
>> commit f83331bab149e29fa2c49cf102c0cd8c3f1ce9f9
>> Author: Santosh Rastapur <santosh@chelsio.com>
>> Date: Tue May 21 04:21:29 2013 +0000
>> cxgb3: Check and handle the dma mapping errors
>>
>> Any quick ideas? Thanks!
> That patch adds error checking to detect failed dma mapping requests.
> Before it, the code always assumed that dma mapping requests succeded,
> whether they actually do or not, so the fact that the older kernel
> does not log errors only means that the failures are being ignored,
> and any appearance of working is through pure luck. The machine could
> have just crashed at that point.
>
> What is the observed behavior of the system by the machine initiating
> the ping flood? Do the older and newer kernels differ in the
> percentage of pings that do not receive replies? O the newer kernel,
> when the mapping errors are detected, the packet that it is trying to
> transmit is dropped, but I'm not at all sure what happens on the older
> kernel after the dma mapping fails. As I mentioned earlier, I'm
> surprised it does not crash. Perhaps the folks from Chelsio have a
> better idea what happens after a dma mapping error is ignored?
Hi,
It should definitely not be ignored. It should not happen this reliably
either.
I wonder if we are not hitting a leak of iommu entries.
Cheers,
Divy
^ permalink raw reply
* Re: linux-next: Tree for Aug 7
From: Eric Dumazet @ 2013-08-07 16:40 UTC (permalink / raw)
To: Johannes Berg
Cc: Phil Sutter, Sedat Dilek, Stephen Rothwell, linux-next,
linux-kernel, netdev, David Miller, Hannes Frederic Sowa,
wireless, John Linville
In-Reply-To: <1375892522.8154.3.camel@jlt4.sipsolutions.net>
On Wed, 2013-08-07 at 18:22 +0200, Johannes Berg wrote:
> Maybe. I haven't tested it, but I'm thinking that skb->data doesn't
> point to the start of the data frame in this case, since we now call
> eth_type_trans() which pulls the ethernet header. So if the device just
> transmits skb->len starting from skb->data, it'll be wrong, no? That
> seems a basic assumption though.
Yes, it seems calling eth_type_trans() is not right here, and even could
crash.
Sorry, for being vague, I am a bit busy this morning.
^ permalink raw reply
* Re: linux-next: Tree for Aug 7
From: Johannes Berg @ 2013-08-07 16:22 UTC (permalink / raw)
To: Eric Dumazet
Cc: Phil Sutter, Sedat Dilek, Stephen Rothwell, linux-next,
linux-kernel, netdev, David Miller, Hannes Frederic Sowa,
wireless, John Linville
In-Reply-To: <1375892245.4004.31.camel@edumazet-glaptop>
On Wed, 2013-08-07 at 09:17 -0700, Eric Dumazet wrote:
> On Wed, 2013-08-07 at 18:12 +0200, Johannes Berg wrote:
> > On Wed, 2013-08-07 at 17:59 +0200, Phil Sutter wrote:
> >
> > > The idea behind this patch is that users setting the protocol to
> > > something else probably do know better and so should be left alone.
> >
> > Regardless of that, I think that still the skb pointers would be changed
> > by this patch which would confuse the receiver of the SKB (device
> > driver), no? Has anyone verified that theory? :)
>
> Maybe receivers made wrong assumptions about some headers being set or
> not set ?
Maybe. I haven't tested it, but I'm thinking that skb->data doesn't
point to the start of the data frame in this case, since we now call
eth_type_trans() which pulls the ethernet header. So if the device just
transmits skb->len starting from skb->data, it'll be wrong, no? That
seems a basic assumption though.
johannes
^ permalink raw reply
* Re: [PATCH v2.35 5/6] lib: Push MPLS tags in the OpenFlow 1.3 ordering
From: Ben Pfaff @ 2013-08-07 16:17 UTC (permalink / raw)
To: Joe Stringer
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, Ravi K,
netdev-u79uwXL29TY76Z2rM5mHXA, Isaku Yamahata
In-Reply-To: <CAOftzPjviFrD5c3ZHM6n6-Ud+REyiuaXgNzea7TFNrYZADqdXQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, Aug 06, 2013 at 11:04:31AM +0900, Joe Stringer wrote:
> The general background to this patch is that we aim to keep the datapath
> interface for MPLS actions simple. As such, when pushing an MPLS tag, this
> is done immediately after the ethernet header regardless of the presence of
> VLANs (As per OF1.3 spec). We can then implement OF1.2 behaviour by popping
> any existing VLAN tags before applying MPLS actions, then pushing the tags
> back on afterwards. This is all done on the odp actions translation side.
I think that keeping the interface simple is a good approach. Always
pushing labels just after the Ethernet header sounds reasonable to me.
(I don't speak for Jesse. He might have a different idea.)
> I'm wondering if this general approach is the preferred method, and as an
> extension, what are your thoughts on how these patches add the new
> 'vlan_tci' field into 'struct xlate_in' and apply VLAN actions twice
> (before and/or after MPLS actions). This provides a mechanism to support
> both OF1.2 and OF1.3 behaviours within the scope of current VLAN support,
> but may or may not fit in well with future ideas for QinQ (ie,
> double-stacked 0x8100 tags) or 802.1ad support.
In general, if userspace code works for our current needs, and kernel
code works for our current needs and can be extended to foreseeable
future needs, then that's fine. I often find it to be a mistake to try
to generalize too much in advance.
Thanks,
Ben.
^ permalink raw reply
* Re: linux-next: Tree for Aug 7
From: Eric Dumazet @ 2013-08-07 16:17 UTC (permalink / raw)
To: Johannes Berg
Cc: Phil Sutter, Sedat Dilek, Stephen Rothwell, linux-next,
linux-kernel, netdev, David Miller, Hannes Frederic Sowa,
wireless, John Linville
In-Reply-To: <1375891974.8154.1.camel@jlt4.sipsolutions.net>
On Wed, 2013-08-07 at 18:12 +0200, Johannes Berg wrote:
> On Wed, 2013-08-07 at 17:59 +0200, Phil Sutter wrote:
>
> > The idea behind this patch is that users setting the protocol to
> > something else probably do know better and so should be left alone.
>
> Regardless of that, I think that still the skb pointers would be changed
> by this patch which would confuse the receiver of the SKB (device
> driver), no? Has anyone verified that theory? :)
Maybe receivers made wrong assumptions about some headers being set or
not set ?
A patch can uncover prior bugs.
commit 76fe45812a3b134c3917 is an example of a fix we had to do because
of another fix ;)
^ permalink raw reply
* Re: linux-next: Tree for Aug 7
From: Johannes Berg @ 2013-08-07 16:12 UTC (permalink / raw)
To: Phil Sutter
Cc: Sedat Dilek, Stephen Rothwell, linux-next, linux-kernel, netdev,
David Miller, Hannes Frederic Sowa, wireless, John Linville
In-Reply-To: <20130807155918.GA16263@orbit.nwl.cc>
On Wed, 2013-08-07 at 17:59 +0200, Phil Sutter wrote:
> The idea behind this patch is that users setting the protocol to
> something else probably do know better and so should be left alone.
Regardless of that, I think that still the skb pointers would be changed
by this patch which would confuse the receiver of the SKB (device
driver), no? Has anyone verified that theory? :)
johannes
^ permalink raw reply
* Re: linux-next: Tree for Aug 7
From: Phil Sutter @ 2013-08-07 15:59 UTC (permalink / raw)
To: Sedat Dilek
Cc: Stephen Rothwell, linux-next, linux-kernel, netdev, David Miller,
Hannes Frederic Sowa, wireless, John Linville, Johannes Berg
In-Reply-To: <CA+icZUW+uxCsvPYcY2RRvMxRVzaBLuH9u8M4ziQNWXv=nk4b2w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]
On Wed, Aug 07, 2013 at 10:29:18AM +0200, Sedat Dilek wrote:
> On Wed, Aug 7, 2013 at 7:54 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > Hi all,
> >
> > Changes since 20130806:
> >
> > The ext4 tree lost its build failure.
> >
> > The mvebu tree gained a build failure so I used the version from
> > next-20130806.
> >
> > The akpm tree gained conflicts against the ext4 tree.
> >
> > ----------------------------------------------------------------------------
> >
>
> [ CC some netdev and wireless folks ]
>
> Yesterday, I discovered an issue with net-next.
> The patch in [1] fixed the problems in my network/wifi environment.
> Hannes confirmed that virtio_net are solved, too.
> Today's next-20130807 still needs it for affected people.
>
> - Sedat -
>
> [1] http://marc.info/?l=linux-netdev&m=137582524017840&w=2
> [2] http://marc.info/?l=linux-netdev&m=137583048219416&w=2
> [3] http://marc.info/?t=137579712800008&r=1&w=2
Could you please try the attached patch. It limits parsing the ethernet
header (by calling eth_type_trans()) to cases when the configured
protocol is ETH_P_ALL, so at least for 802.1X this should fix the
problem.
The idea behind this patch is that users setting the protocol to
something else probably do know better and so should be left alone.
Best wishes, Phil
[-- Attachment #2: af_packet_fixup.diff --]
[-- Type: text/plain, Size: 1606 bytes --]
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index bbe1ece..66bc79c 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1932,8 +1932,6 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
ph.raw = frame;
- skb->protocol = proto;
- skb->dev = dev;
skb->priority = po->sk.sk_priority;
skb->mark = po->sk.sk_mark;
sock_tx_timestamp(&po->sk, &skb_shinfo(skb)->tx_flags);
@@ -2002,13 +2000,18 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
if (unlikely(err))
return err;
- if (dev->type == ARPHRD_ETHER)
- skb->protocol = eth_type_trans(skb, dev);
-
data += dev->hard_header_len;
to_write -= dev->hard_header_len;
}
+ if (dev->type == ARPHRD_ETHER &&
+ proto = htons(ETH_P_ALL)) {
+ skb->protocol = eth_type_trans(skb, dev);
+ } else {
+ skb->protocol = proto;
+ skb->dev = dev;
+ }
+
max_frame_len = dev->mtu + dev->hard_header_len;
if (skb->protocol == htons(ETH_P_8021Q))
max_frame_len += VLAN_HLEN;
@@ -2331,15 +2334,17 @@ static int packet_snd(struct socket *sock,
sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
- if (dev->type == ARPHRD_ETHER) {
+ if (dev->type == ARPHRD_ETHER &&
+ proto == htons(ETH_P_ALL)) {
skb->protocol = eth_type_trans(skb, dev);
- if (skb->protocol == htons(ETH_P_8021Q))
- reserve += VLAN_HLEN;
} else {
skb->protocol = proto;
skb->dev = dev;
}
+ if (skb->protocol == htons(ETH_P_8021Q))
+ reserve += VLAN_HLEN;
+
if (!gso_type && (len > dev->mtu + reserve + extra_len)) {
err = -EMSGSIZE;
goto out_free;
^ permalink raw reply related
* Re: skbs delivered to 'wrong' packet_type handler
From: Erik Hugne @ 2013-08-07 15:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, tipc-discussion, jon.maloy, ying.xue
In-Reply-To: <1375883743.4004.26.camel@edumazet-glaptop>
On Wed, Aug 07, 2013 at 06:55:43AM -0700, Eric Dumazet wrote:
> Its not ignored, quite the contrary if you look at the code :
>
> vi +3595 net/core/dev.c
>
> type = skb->protocol;
> list_for_each_entry_rcu(ptype,
> &ptype_base[ntohs(type) & PTYPE_HASH_MASK], list) {
> if (ptype->type == type &&
> (ptype->dev == null_or_dev || ptype->dev == skb->dev ||
> ptype->dev == orig_dev)) {
> if (pt_prev)
> ret = deliver_skb(skb, pt_prev, orig_dev);
> pt_prev = ptype;
> }
> }
>
>
> pkt->dev being set is only meaningful for packet sockets.
>
> Protocols themselves should not care. If they want to care, they must
> add their own checks.
>
> Socket API has SO_BINDTODEVICE for this purpose.
> IP layer has RP filtering.
>
> A protocol should register a single ptype with NULL dev.
>
Very well, we'll have to fix the protocol registration and add appropriate
checks in TIPC then.
But i fail to understand why a handler registered on the native device needs
to get the packets destined to a child vlan device (the ptype->dev == orig_dev
check).
//E
^ permalink raw reply
* Re: [PATCH 2/2] net/usb: hso: allocate serial_state_notification as separate buffer
From: Dan Williams @ 2013-08-07 15:47 UTC (permalink / raw)
To: Jussi Kivilinna
Cc: netdev, Greg Kroah-Hartman, linux-usb, David S. Miller, Jan Dumon
In-Reply-To: <20130807132907.14451.2928.stgit@localhost6.localdomain6>
On Wed, 2013-08-07 at 16:29 +0300, Jussi Kivilinna wrote:
> URB transfer_buffers must not be allocated as part of larger structure because
> DMA coherence issues.
>
> Patch changes hso to allocate serial_state_notification member of
> 'struct hso_tiocmget' as separate buffer.
>
> Patch is only compile tested.
Tested-by: Dan Williams <dcbw@redhat.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> ---
> drivers/net/usb/hso.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index fc082c0..21ffa6f 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -211,7 +211,7 @@ struct hso_tiocmget {
> int intr_completed;
> struct usb_endpoint_descriptor *endp;
> struct urb *urb;
> - struct hso_serial_state_notification serial_state_notification;
> + struct hso_serial_state_notification *serial_state_notification;
> u16 prev_UART_state_bitmap;
> struct uart_icount icount;
> };
> @@ -1465,7 +1465,7 @@ static int tiocmget_submit_urb(struct hso_serial *serial,
> usb_rcvintpipe(usb,
> tiocmget->endp->
> bEndpointAddress & 0x7F),
> - &tiocmget->serial_state_notification,
> + tiocmget->serial_state_notification,
> sizeof(struct hso_serial_state_notification),
> tiocmget_intr_callback, serial,
> tiocmget->endp->bInterval);
> @@ -1499,7 +1499,7 @@ static void tiocmget_intr_callback(struct urb *urb)
> if (!tiocmget)
> return;
> usb = serial->parent->usb;
> - serial_state_notification = &tiocmget->serial_state_notification;
> + serial_state_notification = tiocmget->serial_state_notification;
> if (serial_state_notification->bmRequestType != BM_REQUEST_TYPE ||
> serial_state_notification->bNotification != B_NOTIFICATION ||
> le16_to_cpu(serial_state_notification->wValue) != W_VALUE ||
> @@ -2608,6 +2608,7 @@ static void hso_free_tiomget(struct hso_serial *serial)
> usb_free_urb(tiocmget->urb);
> tiocmget->urb = NULL;
> serial->tiocmget = NULL;
> + kfree(tiocmget->serial_state_notification);
> kfree(tiocmget);
> }
> }
> @@ -2664,6 +2665,13 @@ static struct hso_device *hso_create_bulk_serial_device(
> */
> if (serial->tiocmget) {
> tiocmget = serial->tiocmget;
> +
> + tiocmget->serial_state_notification = kzalloc(
> + sizeof(struct hso_serial_state_notification),
> + GFP_KERNEL);
> + if (!tiocmget->serial_state_notification)
> + goto tiocmget_fail;
> +
> tiocmget->urb = usb_alloc_urb(0, GFP_KERNEL);
> if (tiocmget->urb) {
> mutex_init(&tiocmget->mutex);
> @@ -2672,8 +2680,10 @@ static struct hso_device *hso_create_bulk_serial_device(
> interface,
> USB_ENDPOINT_XFER_INT,
> USB_DIR_IN);
> - } else
> + } else {
> +tiocmget_fail:
> hso_free_tiomget(serial);
> + }
> }
> }
> else
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] net/usb: hso: allocate URB setup_packets as separate buffers
From: Dan Williams @ 2013-08-07 15:46 UTC (permalink / raw)
To: Jussi Kivilinna
Cc: netdev, Greg Kroah-Hartman, linux-usb, David S. Miller, Jan Dumon
In-Reply-To: <20130807132902.14451.37517.stgit@localhost6.localdomain6>
On Wed, 2013-08-07 at 16:29 +0300, Jussi Kivilinna wrote:
> URB setup packets must not be allocated as part of larger structure because
> DMA coherence issues.
>
> Patch changes hso to allocate ctrl_req_[tr]x members of 'struct hso_serial'
> as separate buffers.
>
> Patch is only compile tested.
Tested-by: Dan Williams <dcbw@redhat.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
> ---
> drivers/net/usb/hso.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index cba1d46..fc082c0 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -237,8 +237,8 @@ struct hso_serial {
> u16 tx_data_length; /* should contain allocated length */
> u16 tx_data_count;
> u16 tx_buffer_count;
> - struct usb_ctrlrequest ctrl_req_tx;
> - struct usb_ctrlrequest ctrl_req_rx;
> + struct usb_ctrlrequest *ctrl_req_tx;
> + struct usb_ctrlrequest *ctrl_req_rx;
>
> struct usb_endpoint_descriptor *in_endp;
> struct usb_endpoint_descriptor *out_endp;
> @@ -1847,7 +1847,7 @@ static int hso_mux_serial_read(struct hso_serial *serial)
> USB_CDC_GET_ENCAPSULATED_RESPONSE,
> serial->parent->port_spec & HSO_PORT_MASK,
> serial->rx_urb[0],
> - &serial->ctrl_req_rx,
> + serial->ctrl_req_rx,
> serial->rx_data[0], serial->rx_data_length);
> }
>
> @@ -1916,7 +1916,7 @@ static int hso_mux_serial_write_data(struct hso_serial *serial)
> USB_CDC_SEND_ENCAPSULATED_COMMAND,
> serial->parent->port_spec & HSO_PORT_MASK,
> serial->tx_urb,
> - &serial->ctrl_req_tx,
> + serial->ctrl_req_tx,
> serial->tx_data, serial->tx_data_count);
> }
>
> @@ -2264,6 +2264,10 @@ static void hso_serial_common_free(struct hso_serial *serial)
> usb_free_urb(serial->tx_urb);
> kfree(serial->tx_data);
> tty_port_destroy(&serial->port);
> +
> + /* free control URB setup packet buffers */
> + kfree(serial->ctrl_req_tx);
> + kfree(serial->ctrl_req_rx);
> }
>
> static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
> @@ -2292,6 +2296,12 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
> spin_lock_init(&serial->serial_lock);
> serial->num_rx_urbs = num_urbs;
>
> + /* allocate control URB setup packet buffers */
> + serial->ctrl_req_tx = kzalloc(sizeof(*serial->ctrl_req_tx), GFP_KERNEL);
> + serial->ctrl_req_rx = kzalloc(sizeof(*serial->ctrl_req_rx), GFP_KERNEL);
> + if (!serial->ctrl_req_tx || !serial->ctrl_req_rx)
> + goto exit;
> +
> /* RX, allocate urb and initialize */
>
> /* prepare our RX buffer */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [Patch net-next] vxlan: fix a soft lockup in vxlan module removal
From: Stephen Hemminger @ 2013-08-07 15:31 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David S. Miller
In-Reply-To: <1375865002-8732-1-git-send-email-amwang@redhat.com>
On Wed, 7 Aug 2013 16:43:22 +0800
Cong Wang <amwang@redhat.com> wrote:
> From: Cong Wang <amwang@redhat.com>
>
> This is a regression introduced by:
>
> commit fe5c3561e6f0ac7c9546209f01351113c1b77ec8
> Author: stephen hemminger <stephen@networkplumber.org>
> Date: Sat Jul 13 10:18:18 2013 -0700
>
> vxlan: add necessary locking on device removal
>
> The problem is that vxlan_dellink(), which is called with RTNL lock
> held, tries to flush the workqueue synchronously, but apparently
> igmp_join and igmp_leave work need to hold RTNL lock too, therefore we
> have a soft lockup!
>
> As suggested by Stephen, probably the flush_workqueue can just be
> removed and let the normal refcounting work. The workqueue has a
> reference to device and socket, therefore the cleanups should work
> correctly.
>
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: David S. Miller <davem@davemloft.net>
> Tested-by: Cong Wang <amwang@redhat.com>
> Signed-off-by: Cong Wang <amwang@redhat.com>
>
> ---
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 8bf31d9..c51ef9b 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1837,8 +1837,6 @@ static void vxlan_dellink(struct net_device *dev, struct list_head *head)
> struct vxlan_net *vn = net_generic(dev_net(dev), vxlan_net_id);
> struct vxlan_dev *vxlan = netdev_priv(dev);
>
> - flush_workqueue(vxlan_wq);
> -
> spin_lock(&vn->sock_lock);
> hlist_del_rcu(&vxlan->hlist);
> spin_unlock(&vn->sock_lock);
Thanks for testing this.
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply
* [BUG] fs_enet not receiving multicast
From: Stephen Hemminger @ 2013-08-07 15:29 UTC (permalink / raw)
To: Pantelis Antoniou, Vitaly Bordug; +Cc: netdev
Reported on kernel bugzilla, freescale ethernet driver not
receiving multicast unless promiscuous enabled.
https://bugzilla.kernel.org/show_bug.cgi?id=60703
^ permalink raw reply
* Re: macvtap bug: using smp_processor_id() in preemptible code
From: Eric Dumazet @ 2013-08-07 15:26 UTC (permalink / raw)
To: Thomas Huth; +Cc: netdev, Vlad Yasevich, David S. Miller, Eric Dumazet
In-Reply-To: <20130807164319.20aa2333@thhw500>
On Wed, 2013-08-07 at 16:43 +0200, Thomas Huth wrote:
> Hi,
>
> I am using macvtap (via KVM/virsh) on a s390 box. With the latest
> kernel source from linux-next master branch, I suddenly get the
> following error messages in the dmesg output:
>
> BUG: using smp_processor_id() in preemptible [00000000] code: vhost-45891/45892
> caller is macvtap_do_read+0x45c/0x600 [macvtap]
> CPU: 1 PID: 45892 Comm: vhost-45891 Not tainted 3.11.0-bisecttest #13
> 000000010cab3a00 000000010cab3a10 0000000000000002 0000000000000000
> 000000010cab3aa0 000000010cab3a18 000000010cab3a18 00000000001127b4
> 0000000000000000 0000000000000001 0000000000000000 000000010000000b
> 0000000000000060 000003fe00000008 0000000000000000 000000010cab3a70
> 00000000006ea2f0 00000000001127b4 000000010cab3a00 000000010cab3a50
> Call Trace:
> ([<00000000001126ee>] show_trace+0x126/0x144)
> [<00000000001127d2>] show_stack+0xc6/0xd4
> [<000000000068bcec>] dump_stack+0x74/0xd8
> [<0000000000481066>] debug_smp_processor_id+0xf6/0x114
> [<000003ff802e9a18>] macvtap_do_read+0x45c/0x600 [macvtap]
> [<000003ff802e9c1c>] macvtap_recvmsg+0x60/0x88 [macvtap]
> [<000003ff80318c5e>] handle_rx+0x5b2/0x800 [vhost_net]
> [<000003ff8028f77c>] vhost_worker+0x15c/0x1c4 [vhost]
> [<000000000015f3ac>] kthread+0xd8/0xe4
> [<00000000006934a6>] kernel_thread_starter+0x6/0xc
> [<00000000006934a0>] kernel_thread_starter+0x0/0xc
> 2 locks held by vhost-45891/45892:
> #0: (&vq->mutex){+.+...}, at: [<000003ff80318718>] handle_rx+0x6c/0x800 [vhost_net]
> #1: (rcu_read_lock){.+.+..}, at: [<000003ff802e98fe>] macvtap_do_read+0x342/0x600 [macvtap]
>
> Apart from these "annoying" error messages (they are repeated
> continually while the KVM guest is running / macvtap is in use),
> everything still seems to work fine, though.
>
> Since everything was still working fine in v3.10, I did some bisecting
> and it seems like this commit introduced this problem:
>
> commit: ac4e4af1e59e16a018527ffa58d9d3f30bb96ca9
> Subject: macvtap: Consistently use rcu functions
>
> I am not sure how to proceed here, this is my first bug report in this
> area, so advice is welcome...
Please try following fix :
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a98fb0e..1c7aab4 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -912,8 +912,11 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
done:
rcu_read_lock();
vlan = rcu_dereference(q->vlan);
- if (vlan)
+ if (vlan) {
+ preempt_disable();
macvlan_count_rx(vlan, copied - vnet_hdr_len, ret == 0, 0);
+ preempt_enable();
+ }
rcu_read_unlock();
return ret ? ret : copied;
^ permalink raw reply related
* macvtap bug: using smp_processor_id() in preemptible code
From: Thomas Huth @ 2013-08-07 14:43 UTC (permalink / raw)
To: netdev; +Cc: Vlad Yasevich, David S. Miller, Eric Dumazet
Hi,
I am using macvtap (via KVM/virsh) on a s390 box. With the latest
kernel source from linux-next master branch, I suddenly get the
following error messages in the dmesg output:
BUG: using smp_processor_id() in preemptible [00000000] code: vhost-45891/45892
caller is macvtap_do_read+0x45c/0x600 [macvtap]
CPU: 1 PID: 45892 Comm: vhost-45891 Not tainted 3.11.0-bisecttest #13
000000010cab3a00 000000010cab3a10 0000000000000002 0000000000000000
000000010cab3aa0 000000010cab3a18 000000010cab3a18 00000000001127b4
0000000000000000 0000000000000001 0000000000000000 000000010000000b
0000000000000060 000003fe00000008 0000000000000000 000000010cab3a70
00000000006ea2f0 00000000001127b4 000000010cab3a00 000000010cab3a50
Call Trace:
([<00000000001126ee>] show_trace+0x126/0x144)
[<00000000001127d2>] show_stack+0xc6/0xd4
[<000000000068bcec>] dump_stack+0x74/0xd8
[<0000000000481066>] debug_smp_processor_id+0xf6/0x114
[<000003ff802e9a18>] macvtap_do_read+0x45c/0x600 [macvtap]
[<000003ff802e9c1c>] macvtap_recvmsg+0x60/0x88 [macvtap]
[<000003ff80318c5e>] handle_rx+0x5b2/0x800 [vhost_net]
[<000003ff8028f77c>] vhost_worker+0x15c/0x1c4 [vhost]
[<000000000015f3ac>] kthread+0xd8/0xe4
[<00000000006934a6>] kernel_thread_starter+0x6/0xc
[<00000000006934a0>] kernel_thread_starter+0x0/0xc
2 locks held by vhost-45891/45892:
#0: (&vq->mutex){+.+...}, at: [<000003ff80318718>] handle_rx+0x6c/0x800 [vhost_net]
#1: (rcu_read_lock){.+.+..}, at: [<000003ff802e98fe>] macvtap_do_read+0x342/0x600 [macvtap]
Apart from these "annoying" error messages (they are repeated
continually while the KVM guest is running / macvtap is in use),
everything still seems to work fine, though.
Since everything was still working fine in v3.10, I did some bisecting
and it seems like this commit introduced this problem:
commit: ac4e4af1e59e16a018527ffa58d9d3f30bb96ca9
Subject: macvtap: Consistently use rcu functions
I am not sure how to proceed here, this is my first bug report in this
area, so advice is welcome...
Regards,
Thomas Huth
^ permalink raw reply
* Re: skbs delivered to 'wrong' packet_type handler
From: Eric Dumazet @ 2013-08-07 13:55 UTC (permalink / raw)
To: Erik Hugne; +Cc: netdev, tipc-discussion, jon.maloy, ying.xue
In-Reply-To: <20130807130856.GI3165@eerihug-hybrid.rnd.ki.sw.ericsson.se>
On Wed, 2013-08-07 at 15:08 +0200, Erik Hugne wrote:
> We have a race condition in TIPC when using both the parent ethernet
> device, and a vlan on top of this device as TIPC bearers. For some reason,
> net/core/dev.c is delivering vlan packets to packet handlers registered to
> the native device. This only seems to occur when a packet_type handler is
> registered on both the vlan device and it's parent ethernet device. This
> causes all kinds of weird behaviour in TIPC, from cross-vlan links being
> established to oopses.
>
> At first, i thought this was due to a missing PACKET_OTHERHOST filtering
> in the TIPC ethernet code, but adding that check did not resolve the issue.
> Adding an explicit check for skb->dev vs packet_type->dev does work, but
> i dont think that's the proper way to solve it.
> What's the purpose of having a dev entry in the packet_type if it's ignored
> by the lower layers?
Its not ignored, quite the contrary if you look at the code :
vi +3595 net/core/dev.c
type = skb->protocol;
list_for_each_entry_rcu(ptype,
&ptype_base[ntohs(type) & PTYPE_HASH_MASK], list) {
if (ptype->type == type &&
(ptype->dev == null_or_dev || ptype->dev == skb->dev ||
ptype->dev == orig_dev)) {
if (pt_prev)
ret = deliver_skb(skb, pt_prev, orig_dev);
pt_prev = ptype;
}
}
pkt->dev being set is only meaningful for packet sockets.
Protocols themselves should not care. If they want to care, they must
add their own checks.
Socket API has SO_BINDTODEVICE for this purpose.
IP layer has RP filtering.
A protocol should register a single ptype with NULL dev.
^ permalink raw reply
* Re: [PATCH v3 00/11] Add namespace support for syslog
From: Serge Hallyn @ 2013-08-07 13:48 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Rui Xiang, netdev, containers, linux-kernel, libo.chen,
netfilter-devel, guz.fnst, akpm
In-Reply-To: <878v0evssv.fsf@xmission.com>
Quoting Eric W. Biederman (ebiederm@xmission.com):
>
> Since this still has not been addressed. I am going to repeat Andrews
> objection again.
>
> Isn't there a better way to get iptables information out than to use
> syslog. I did not have time to follow up on that but it did appear that
Bruno suggested NFLOG target + ulogd. That's not ideal, but doable. At
least each container should be able to do that for itself. What it
won't do is let a host admin make sure that he doesn't get corrupted
syslog entries when partial-lines get sent from several containers and
the kernel and randomly spliced together. It also would simply be
better if the information was *always* sent to userspace instead of
syslog.
> someone did have a better way to get the information out.
>
> Essentially the argument against this goes. The kernel logging facility
> is really not a particularly good tool to be using for anything other
> than kernel debugging information, and there appear to be no substantial
> uses for a separate syslog that should not be done in other ways.
>
> That design objection must be addressed before merging this code can be
> given serious consideration.
>
> Eric
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply
* Re: [PATCH v4 3/3] net: igmp: Allow user-space configuration of igmp unsolicited report interval
From: Benjamin LaHaise @ 2013-08-07 13:43 UTC (permalink / raw)
To: William Manley
Cc: netdev, luky-37, sergei.shtylyov, bhutchings, davem, hannes
In-Reply-To: <1375812195-6575-4-git-send-email-william.manley@youview.com>
On Tue, Aug 06, 2013 at 07:03:15PM +0100, William Manley wrote:
> Adds the new procfs knobs:
>
> /proc/sys/net/ipv4/conf/*/igmpv2_unsolicited_report_interval
> /proc/sys/net/ipv4/conf/*/igmpv3_unsolicited_report_interval
>
> Which will allow userspace configuration of the IGMP unsolicited report
> interval (see below) in milliseconds. The defaults are 10000ms for IGMPv2
> and 1000ms for IGMPv3 in accordance with RFC2236 and RFC3376.
>
> Background:
>
> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent. The
> delay between joins is the "IGMP Unsolicited Report Interval".
>
> Prior to this patch this value was hard coded in the kernel to 10s for
> IGMPv2 and 1s for IGMPv3. 10s is unsuitable for some use-cases, such as
> IPTV as it can cause channel change to be slow in the presence of packet
> loss.
>
> This patch allows the value to be overridden from userspace for both
> IGMPv2 and IGMPv3 such that it can be tuned accoding to the network.
>
> Tested with Wireshark and a simple program to join a (non-existent)
> multicast group. The distribution of timings for the second join differ
> based upon setting the procfs knobs.
>
> igmpvX_unsolicited_report_interval is intended to follow the pattern
> established by force_igmp_version, and while a procfs entry has been added
> a corresponding sysctl knob has not as it is my understanding that sysctl
> is deprecated[1].
>
> [1]: http://lwn.net/Articles/247243/
>
> Signed-off-by: William Manley <william.manley@youview.com>
Acked-by: Benjamin LaHaise <bcrl@kvack.org>
Also good. Thanks!
-ben
> ---
> include/linux/inetdevice.h | 2 ++
> net/ipv4/devinet.c | 8 ++++++++
> net/ipv4/igmp.c | 19 +++++++++++++++++--
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index d4c56fc..43b3c72 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -28,6 +28,8 @@ enum
> IPV4_DEVCONF_ARPFILTER,
> IPV4_DEVCONF_MEDIUM_ID,
> IPV4_DEVCONF_FORCE_IGMP_VERSION,
> + IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL,
> + IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL,
> IPV4_DEVCONF_NOXFRM,
> IPV4_DEVCONF_NOPOLICY,
> IPV4_DEVCONF_ARP_ANNOUNCE,
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index a9561c4..56ecb1b 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -73,6 +73,8 @@ static struct ipv4_devconf ipv4_devconf = {
> [IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
> [IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
> [IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
> + [IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
> + [IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] = 1000 /*ms*/,
> },
> };
>
> @@ -83,6 +85,8 @@ static struct ipv4_devconf ipv4_devconf_dflt = {
> [IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
> [IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
> [IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
> + [IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
> + [IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] = 1000 /*ms*/,
> },
> };
>
> @@ -2096,6 +2100,10 @@ static struct devinet_sysctl_table {
> DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP_PVLAN, "proxy_arp_pvlan"),
> DEVINET_SYSCTL_RW_ENTRY(FORCE_IGMP_VERSION,
> "force_igmp_version"),
> + DEVINET_SYSCTL_RW_ENTRY(IGMPV2_UNSOLICITED_REPORT_INTERVAL,
> + "igmpv2_unsolicited_report_interval"),
> + DEVINET_SYSCTL_RW_ENTRY(IGMPV3_UNSOLICITED_REPORT_INTERVAL,
> + "igmpv3_unsolicited_report_interval"),
>
> DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
> DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 9f0aaea..c5541da 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -141,10 +141,25 @@
>
> static int unsolicited_report_interval(struct in_device *in_dev)
> {
> + int interval_ms, interval_jiffies;
> +
> if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
> - return IGMP_V2_Unsolicited_Report_Interval;
> + interval_ms = IN_DEV_CONF_GET(
> + in_dev,
> + IGMPV2_UNSOLICITED_REPORT_INTERVAL);
> else /* v3 */
> - return IGMP_V3_Unsolicited_Report_Interval;
> + interval_ms = IN_DEV_CONF_GET(
> + in_dev,
> + IGMPV3_UNSOLICITED_REPORT_INTERVAL);
> +
> + interval_jiffies = msecs_to_jiffies(interval_ms);
> +
> + /* _timer functions can't handle a delay of 0 jiffies so ensure
> + * we always return a positive value.
> + */
> + if (interval_jiffies <= 0)
> + interval_jiffies = 1;
> + return interval_jiffies;
> }
>
> static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
> --
> 1.7.10.4
--
"Thought is the essence of where you are now."
^ permalink raw reply
* Re: [PATCH v4 1/3] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3
From: Benjamin LaHaise @ 2013-08-07 13:43 UTC (permalink / raw)
To: William Manley
Cc: netdev, luky-37, sergei.shtylyov, bhutchings, davem, hannes
In-Reply-To: <1375812195-6575-2-git-send-email-william.manley@youview.com>
On Tue, Aug 06, 2013 at 07:03:13PM +0100, William Manley wrote:
> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent. The
> delay between joins is the "IGMP Unsolicited Report Interval".
>
> Previously this value was hard coded to be chosen randomly between 0-10s.
> This can be too long for some use-cases, such as IPTV as it can cause
> channel change to be slow in the presence of packet loss.
>
> The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
> IGMPv3 RFC3376. This patch makes the kernel use the 1s value from the
> later RFC if we are operating in IGMPv3 mode. IGMPv2 behaviour is
> unaffected.
>
> Tested with Wireshark and a simple program to join a (non-existent)
> multicast group. The distribution of timings for the second join differ
> based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
>
> Signed-off-by: William Manley <william.manley@youview.com>
Acked-by: Benjamin LaHaise <bcrl@kvack.org>
-ben
> ---
> net/ipv4/igmp.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index d8c2327..9f0aaea 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -113,7 +113,8 @@
>
> #define IGMP_V1_Router_Present_Timeout (400*HZ)
> #define IGMP_V2_Router_Present_Timeout (400*HZ)
> -#define IGMP_Unsolicited_Report_Interval (10*HZ)
> +#define IGMP_V2_Unsolicited_Report_Interval (10*HZ)
> +#define IGMP_V3_Unsolicited_Report_Interval (1*HZ)
> #define IGMP_Query_Response_Interval (10*HZ)
> #define IGMP_Unsolicited_Report_Count 2
>
> @@ -138,6 +139,14 @@
> ((in_dev)->mr_v2_seen && \
> time_before(jiffies, (in_dev)->mr_v2_seen)))
>
> +static int unsolicited_report_interval(struct in_device *in_dev)
> +{
> + if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
> + return IGMP_V2_Unsolicited_Report_Interval;
> + else /* v3 */
> + return IGMP_V3_Unsolicited_Report_Interval;
> +}
> +
> static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
> static void igmpv3_del_delrec(struct in_device *in_dev, __be32 multiaddr);
> static void igmpv3_clear_delrec(struct in_device *in_dev);
> @@ -719,7 +728,8 @@ static void igmp_ifc_timer_expire(unsigned long data)
> igmpv3_send_cr(in_dev);
> if (in_dev->mr_ifc_count) {
> in_dev->mr_ifc_count--;
> - igmp_ifc_start_timer(in_dev, IGMP_Unsolicited_Report_Interval);
> + igmp_ifc_start_timer(in_dev,
> + unsolicited_report_interval(in_dev));
> }
> __in_dev_put(in_dev);
> }
> @@ -744,7 +754,7 @@ static void igmp_timer_expire(unsigned long data)
>
> if (im->unsolicit_count) {
> im->unsolicit_count--;
> - igmp_start_timer(im, IGMP_Unsolicited_Report_Interval);
> + igmp_start_timer(im, unsolicited_report_interval(in_dev));
> }
> im->reporter = 1;
> spin_unlock(&im->lock);
> --
> 1.7.10.4
--
"Thought is the essence of where you are now."
^ permalink raw reply
* Re: [PATCH v4 2/3] net: igmp: Don't flush routing cache when force_igmp_version is modified
From: Benjamin LaHaise @ 2013-08-07 13:43 UTC (permalink / raw)
To: William Manley
Cc: netdev, luky-37, sergei.shtylyov, bhutchings, davem, hannes
In-Reply-To: <1375812195-6575-3-git-send-email-william.manley@youview.com>
On Tue, Aug 06, 2013 at 07:03:14PM +0100, William Manley wrote:
> The procfs knob /proc/sys/net/ipv4/conf/*/force_igmp_version allows the
> IGMP protocol version to use to be explicitly set. As a side effect this
> caused the routing cache to be flushed as it was declared as a
> DEVINET_SYSCTL_FLUSHING_ENTRY. Flushing is unnecessary and this patch
> makes it so flushing does not occur.
>
> Requested by Hannes Frederic Sowa as he was reviewing other patches
> adding procfs entries.
>
> Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: William Manley <william.manley@youview.com>
Acked-by: Benjamin LaHaise <bcrl@kvack.org>
Looks good.
> ---
> include/linux/inetdevice.h | 2 +-
> net/ipv4/devinet.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index ea1e3b8..d4c56fc 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -27,9 +27,9 @@ enum
> IPV4_DEVCONF_TAG,
> IPV4_DEVCONF_ARPFILTER,
> IPV4_DEVCONF_MEDIUM_ID,
> + IPV4_DEVCONF_FORCE_IGMP_VERSION,
> IPV4_DEVCONF_NOXFRM,
> IPV4_DEVCONF_NOPOLICY,
> - IPV4_DEVCONF_FORCE_IGMP_VERSION,
> IPV4_DEVCONF_ARP_ANNOUNCE,
> IPV4_DEVCONF_ARP_IGNORE,
> IPV4_DEVCONF_PROMOTE_SECONDARIES,
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index dfc39d4..a9561c4 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -2094,11 +2094,11 @@ static struct devinet_sysctl_table {
> DEVINET_SYSCTL_RW_ENTRY(ARP_ACCEPT, "arp_accept"),
> DEVINET_SYSCTL_RW_ENTRY(ARP_NOTIFY, "arp_notify"),
> DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP_PVLAN, "proxy_arp_pvlan"),
> + DEVINET_SYSCTL_RW_ENTRY(FORCE_IGMP_VERSION,
> + "force_igmp_version"),
>
> DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
> DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
> - DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
> - "force_igmp_version"),
> DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
> "promote_secondaries"),
> DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
> --
> 1.7.10.4
--
"Thought is the essence of where you are now."
^ permalink raw reply
* [PATCH] net/usb: rtl8150: allocate URB transfer_buffer and setup_packet separately
From: Jussi Kivilinna @ 2013-08-07 13:36 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, linux-usb, Petko Manolov, David S. Miller
rtl8150 allocates URB transfer_buffer and setup_packet as part of same
structure 'struct async_req'. This can cause same cacheline to be DMA-mapped
twice with same URB. This can lead to memory corruption on some systems.
Patch make allocation of these buffers separate.
Use of 'struct async_req' was introduced by recent commit 4d12997a9b "drivers:
net: usb: rtl8150: concurrent URB bugfix".
Patch is only compile tested.
Cc: <stable@vger.kernel.org>
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
---
drivers/net/usb/rtl8150.c | 48 +++++++++++++++++++++++----------------------
1 file changed, 25 insertions(+), 23 deletions(-)
diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 6cbdac6..c353bfd 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -142,11 +142,6 @@ struct rtl8150 {
typedef struct rtl8150 rtl8150_t;
-struct async_req {
- struct usb_ctrlrequest dr;
- u16 rx_creg;
-};
-
static const char driver_name [] = "rtl8150";
/*
@@ -170,12 +165,12 @@ static int set_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
static void async_set_reg_cb(struct urb *urb)
{
- struct async_req *req = (struct async_req *)urb->context;
int status = urb->status;
if (status < 0)
dev_dbg(&urb->dev->dev, "%s failed with %d", __func__, status);
- kfree(req);
+ kfree(urb->setup_packet); /* dr */
+ kfree(urb->transfer_buffer); /* rx_creg */
usb_free_urb(urb);
}
@@ -183,25 +178,27 @@ static int async_set_registers(rtl8150_t *dev, u16 indx, u16 size, u16 reg)
{
int res = -ENOMEM;
struct urb *async_urb;
- struct async_req *req;
+ struct usb_ctrlrequest *dr;
+ u16 *rx_creg;
+
+ dr = kmalloc(sizeof(*dr), GFP_ATOMIC);
+ rx_creg = kmalloc(sizeof(*rx_creg), GFP_ATOMIC);
+ if (!dr || !rx_creg)
+ goto err;
- req = kmalloc(sizeof(struct async_req), GFP_ATOMIC);
- if (req == NULL)
- return res;
async_urb = usb_alloc_urb(0, GFP_ATOMIC);
- if (async_urb == NULL) {
- kfree(req);
- return res;
- }
- req->rx_creg = cpu_to_le16(reg);
- req->dr.bRequestType = RTL8150_REQT_WRITE;
- req->dr.bRequest = RTL8150_REQ_SET_REGS;
- req->dr.wIndex = 0;
- req->dr.wValue = cpu_to_le16(indx);
- req->dr.wLength = cpu_to_le16(size);
+ if (async_urb == NULL)
+ goto err;
+
+ *rx_creg = cpu_to_le16(reg);
+ dr->bRequestType = RTL8150_REQT_WRITE;
+ dr->bRequest = RTL8150_REQ_SET_REGS;
+ dr->wIndex = 0;
+ dr->wValue = cpu_to_le16(indx);
+ dr->wLength = cpu_to_le16(size);
usb_fill_control_urb(async_urb, dev->udev,
- usb_sndctrlpipe(dev->udev, 0), (void *)&req->dr,
- &req->rx_creg, size, async_set_reg_cb, req);
+ usb_sndctrlpipe(dev->udev, 0), (void *)dr,
+ rx_creg, size, async_set_reg_cb, NULL);
res = usb_submit_urb(async_urb, GFP_ATOMIC);
if (res) {
if (res == -ENODEV)
@@ -209,6 +206,11 @@ static int async_set_registers(rtl8150_t *dev, u16 indx, u16 size, u16 reg)
dev_err(&dev->udev->dev, "%s failed with %d\n", __func__, res);
}
return res;
+
+err:
+ kfree(dr);
+ kfree(rx_creg);
+ return res;
}
static int read_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 * reg)
^ permalink raw reply related
* [PATCH 2/2] net/usb: pegasus: allocate URB transfer_buffers as separate buffers
From: Jussi Kivilinna @ 2013-08-07 13:33 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, linux-usb, Petko Manolov, David S. Miller
In-Reply-To: <20130807133326.15270.86891.stgit@localhost6.localdomain6>
URB transfer_buffers must not be allocated as part of larger structure
because DMA coherence issues.
Patch changes pegasus to allocate intr_buff and tx_buff members of
'struct pegasus' as separate buffers.
Patch is only compile tested.
Cc: <stable@vger.kernel.org>
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
---
drivers/net/usb/pegasus.c | 11 ++++++++++-
drivers/net/usb/pegasus.h | 6 ++++--
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 3bce862..b734702 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -854,7 +854,7 @@ static int pegasus_open(struct net_device *net)
usb_fill_int_urb(pegasus->intr_urb, pegasus->usb,
usb_rcvintpipe(pegasus->usb, 3),
- pegasus->intr_buff, sizeof(pegasus->intr_buff),
+ pegasus->intr_buff, PEGASUS_INTR_BUFLEN,
intr_callback, pegasus, pegasus->intr_interval);
if ((res = usb_submit_urb(pegasus->intr_urb, GFP_KERNEL))) {
if (res == -ENODEV)
@@ -1162,6 +1162,11 @@ static int pegasus_probe(struct usb_interface *intf,
pegasus = netdev_priv(net);
pegasus->dev_index = dev_index;
+ pegasus->intr_buff = kzalloc(PEGASUS_INTR_BUFLEN, GFP_KERNEL);
+ pegasus->tx_buff = kzalloc(PEGASUS_MTU, GFP_KERNEL);
+ if (!pegasus->intr_buff || !pegasus->tx_buff)
+ goto out1;
+
res = alloc_urbs(pegasus);
if (res < 0) {
dev_err(&intf->dev, "can't allocate %s\n", "urbs");
@@ -1223,6 +1228,8 @@ out3:
out2:
free_all_urbs(pegasus);
out1:
+ kfree(pegasus->tx_buff);
+ kfree(pegasus->intr_buff);
free_netdev(net);
out:
pegasus_dec_workqueue();
@@ -1248,6 +1255,8 @@ static void pegasus_disconnect(struct usb_interface *intf)
dev_kfree_skb(pegasus->rx_skb);
pegasus->rx_skb = NULL;
}
+ kfree(pegasus->tx_buff);
+ kfree(pegasus->intr_buff);
free_netdev(pegasus->net);
pegasus_dec_workqueue();
}
diff --git a/drivers/net/usb/pegasus.h b/drivers/net/usb/pegasus.h
index d156462..6469aed 100644
--- a/drivers/net/usb/pegasus.h
+++ b/drivers/net/usb/pegasus.h
@@ -55,6 +55,8 @@
#define PEGASUS_REQ_SET_REGS 0xf1
#define PEGASUS_REQ_SET_REG PEGASUS_REQ_SET_REGS
+#define PEGASUS_INTR_BUFLEN 8
+
enum pegasus_registers {
EthCtrl0 = 0,
EthCtrl1 = 1,
@@ -96,8 +98,8 @@ typedef struct pegasus {
struct urb *rx_urb, *tx_urb, *intr_urb;
struct sk_buff *rx_skb;
int chip;
- unsigned char intr_buff[8];
- __u8 tx_buff[PEGASUS_MTU];
+ unsigned char *intr_buff;
+ __u8 *tx_buff;
__u8 eth_regs[4];
__u8 phy;
__u8 gpio_res;
^ permalink raw reply related
* [PATCH 1/2] net/usb: pegasus: do not use stack for URB buffers
From: Jussi Kivilinna @ 2013-08-07 13:33 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, linux-usb, Petko Manolov, David S. Miller
Currently pegasus passes stack memory (which must not be used for DMA
transfers) as URB buffers with get_registers(), set_registers() and
set_register(). Apply fix inside these functions instead of changing every
call site.
Stack usage was introduced by recent commit 323b34963d11 "drivers: net: usb:
pegasus: fix control urb submission".
Patch is only compile tested.
Cc: <stable@vger.kernel.org>
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
---
drivers/net/usb/pegasus.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 03e8a15..3bce862 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -127,39 +127,59 @@ static void async_ctrl_callback(struct urb *urb)
static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data)
{
int ret;
+ void *buf;
+
+ buf = kmalloc(size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
ret = usb_control_msg(pegasus->usb, usb_rcvctrlpipe(pegasus->usb, 0),
PEGASUS_REQ_GET_REGS, PEGASUS_REQT_READ, 0,
- indx, data, size, 1000);
+ indx, buf, size, 1000);
if (ret < 0)
netif_dbg(pegasus, drv, pegasus->net,
"%s returned %d\n", __func__, ret);
+ else
+ memcpy(data, buf, ret); /* ret == bytes read (max == size) */
+ kfree(buf);
return ret;
}
static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data)
{
int ret;
+ void *buf;
+
+ buf = kmemdup(data, size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
PEGASUS_REQ_SET_REGS, PEGASUS_REQT_WRITE, 0,
- indx, data, size, 100);
+ indx, buf, size, 100);
if (ret < 0)
netif_dbg(pegasus, drv, pegasus->net,
"%s returned %d\n", __func__, ret);
+ kfree(buf);
return ret;
}
static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
{
int ret;
+ void *buf;
+
+ buf = kmemdup(&data, 1, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
ret = usb_control_msg(pegasus->usb, usb_sndctrlpipe(pegasus->usb, 0),
PEGASUS_REQ_SET_REG, PEGASUS_REQT_WRITE, data,
- indx, &data, 1, 1000);
+ indx, buf, 1, 1000);
if (ret < 0)
netif_dbg(pegasus, drv, pegasus->net,
"%s returned %d\n", __func__, ret);
+ kfree(buf);
return ret;
}
^ permalink raw reply related
* Re: [Patch net-next v2 5/8] sunrpc: use generic union inet_addr
From: Jim Rees @ 2013-08-07 13:30 UTC (permalink / raw)
To: Cong Wang
Cc: Jeff Layton, netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
Trond Myklebust, J. Bruce Fields,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1375878446.11370.36.camel@cr0>
Cong Wang wrote:
On Tue, 2013-08-06 at 06:28 -0400, Jeff Layton wrote:
>
> My question is a bit more fundamental: Why are you using this new union
> in your patches instead of simply passing around "struct sockaddr"
> pointers? If you did that, then you could simply replace all of the
> rpc_* wrappers with your generic ones, since you wouldn't need to do
> the cast to this (seemingly unnecessary) union.
Because there are some places have to interpret the structure, without
this union, they need to cast to either sockaddr_in or sockaddr_in6
first, which is not as pretty as using a union.
For example, the code in netpoll:
ipv6_addr_equal(daddr, &np->local_ip.sin6.sin6_addr)
without the union, it would be:
struct sockaddr_in6 *addr = (struct sockaddr_in6 *) &np->local_ip;
ipv6_addr_equal(daddr, addr->sin6_addr);
Too bad ipv6_addr_equal() doesn't take a (void *).
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/2] net/usb: hso: allocate URB setup_packets as separate buffers
From: Jussi Kivilinna @ 2013-08-07 13:29 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, linux-usb, David S. Miller, Jan Dumon
URB setup packets must not be allocated as part of larger structure because
DMA coherence issues.
Patch changes hso to allocate ctrl_req_[tr]x members of 'struct hso_serial'
as separate buffers.
Patch is only compile tested.
Cc: <stable@vger.kernel.org>
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
---
drivers/net/usb/hso.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index cba1d46..fc082c0 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -237,8 +237,8 @@ struct hso_serial {
u16 tx_data_length; /* should contain allocated length */
u16 tx_data_count;
u16 tx_buffer_count;
- struct usb_ctrlrequest ctrl_req_tx;
- struct usb_ctrlrequest ctrl_req_rx;
+ struct usb_ctrlrequest *ctrl_req_tx;
+ struct usb_ctrlrequest *ctrl_req_rx;
struct usb_endpoint_descriptor *in_endp;
struct usb_endpoint_descriptor *out_endp;
@@ -1847,7 +1847,7 @@ static int hso_mux_serial_read(struct hso_serial *serial)
USB_CDC_GET_ENCAPSULATED_RESPONSE,
serial->parent->port_spec & HSO_PORT_MASK,
serial->rx_urb[0],
- &serial->ctrl_req_rx,
+ serial->ctrl_req_rx,
serial->rx_data[0], serial->rx_data_length);
}
@@ -1916,7 +1916,7 @@ static int hso_mux_serial_write_data(struct hso_serial *serial)
USB_CDC_SEND_ENCAPSULATED_COMMAND,
serial->parent->port_spec & HSO_PORT_MASK,
serial->tx_urb,
- &serial->ctrl_req_tx,
+ serial->ctrl_req_tx,
serial->tx_data, serial->tx_data_count);
}
@@ -2264,6 +2264,10 @@ static void hso_serial_common_free(struct hso_serial *serial)
usb_free_urb(serial->tx_urb);
kfree(serial->tx_data);
tty_port_destroy(&serial->port);
+
+ /* free control URB setup packet buffers */
+ kfree(serial->ctrl_req_tx);
+ kfree(serial->ctrl_req_rx);
}
static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
@@ -2292,6 +2296,12 @@ static int hso_serial_common_create(struct hso_serial *serial, int num_urbs,
spin_lock_init(&serial->serial_lock);
serial->num_rx_urbs = num_urbs;
+ /* allocate control URB setup packet buffers */
+ serial->ctrl_req_tx = kzalloc(sizeof(*serial->ctrl_req_tx), GFP_KERNEL);
+ serial->ctrl_req_rx = kzalloc(sizeof(*serial->ctrl_req_rx), GFP_KERNEL);
+ if (!serial->ctrl_req_tx || !serial->ctrl_req_rx)
+ goto exit;
+
/* RX, allocate urb and initialize */
/* prepare our RX buffer */
^ permalink raw reply related
* [PATCH 2/2] net/usb: hso: allocate serial_state_notification as separate buffer
From: Jussi Kivilinna @ 2013-08-07 13:29 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: Greg Kroah-Hartman, linux-usb-u79uwXL29TY76Z2rM5mHXA,
David S. Miller, Jan Dumon
In-Reply-To: <20130807132902.14451.37517.stgit-bi+AKbBUZKagILUCTcTcHdKyNwTtLsGr@public.gmane.org>
URB transfer_buffers must not be allocated as part of larger structure because
DMA coherence issues.
Patch changes hso to allocate serial_state_notification member of
'struct hso_tiocmget' as separate buffer.
Patch is only compile tested.
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Jussi Kivilinna <jussi.kivilinna-X3B1VOXEql0@public.gmane.org>
---
drivers/net/usb/hso.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index fc082c0..21ffa6f 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -211,7 +211,7 @@ struct hso_tiocmget {
int intr_completed;
struct usb_endpoint_descriptor *endp;
struct urb *urb;
- struct hso_serial_state_notification serial_state_notification;
+ struct hso_serial_state_notification *serial_state_notification;
u16 prev_UART_state_bitmap;
struct uart_icount icount;
};
@@ -1465,7 +1465,7 @@ static int tiocmget_submit_urb(struct hso_serial *serial,
usb_rcvintpipe(usb,
tiocmget->endp->
bEndpointAddress & 0x7F),
- &tiocmget->serial_state_notification,
+ tiocmget->serial_state_notification,
sizeof(struct hso_serial_state_notification),
tiocmget_intr_callback, serial,
tiocmget->endp->bInterval);
@@ -1499,7 +1499,7 @@ static void tiocmget_intr_callback(struct urb *urb)
if (!tiocmget)
return;
usb = serial->parent->usb;
- serial_state_notification = &tiocmget->serial_state_notification;
+ serial_state_notification = tiocmget->serial_state_notification;
if (serial_state_notification->bmRequestType != BM_REQUEST_TYPE ||
serial_state_notification->bNotification != B_NOTIFICATION ||
le16_to_cpu(serial_state_notification->wValue) != W_VALUE ||
@@ -2608,6 +2608,7 @@ static void hso_free_tiomget(struct hso_serial *serial)
usb_free_urb(tiocmget->urb);
tiocmget->urb = NULL;
serial->tiocmget = NULL;
+ kfree(tiocmget->serial_state_notification);
kfree(tiocmget);
}
}
@@ -2664,6 +2665,13 @@ static struct hso_device *hso_create_bulk_serial_device(
*/
if (serial->tiocmget) {
tiocmget = serial->tiocmget;
+
+ tiocmget->serial_state_notification = kzalloc(
+ sizeof(struct hso_serial_state_notification),
+ GFP_KERNEL);
+ if (!tiocmget->serial_state_notification)
+ goto tiocmget_fail;
+
tiocmget->urb = usb_alloc_urb(0, GFP_KERNEL);
if (tiocmget->urb) {
mutex_init(&tiocmget->mutex);
@@ -2672,8 +2680,10 @@ static struct hso_device *hso_create_bulk_serial_device(
interface,
USB_ENDPOINT_XFER_INT,
USB_DIR_IN);
- } else
+ } else {
+tiocmget_fail:
hso_free_tiomget(serial);
+ }
}
}
else
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox