* [PATCH net v4 3/4] r8152: support stopping/waking tx queue
From: Hayes Wang @ 2013-11-19 3:25 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1384831511-1625-1-git-send-email-hayeswang@realtek.com>
The maximum packet number which a tx aggregation buffer could contain
is the buffer size / (packet size + descriptor size).
If the tx buffer is empty and the tx queue length is more than the
maximum value which is defined above, stop the tx queue. Wake the tx
queue after any queued packet is filled in a available tx buffer.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 8a786b6..0ac2b53 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -365,6 +365,7 @@ struct r8152 {
struct mii_if_info mii;
int intr_interval;
u32 msg_enable;
+ u32 tx_qlen;
u16 ocp_base;
u8 *intr_buff;
u8 version;
@@ -1173,6 +1174,9 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
remain = rx_buf_sz - (int)(tx_agg_align(tx_data) - agg->head);
}
+ if (netif_queue_stopped(tp->netdev))
+ netif_wake_queue(tp->netdev);
+
usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2),
agg->head, (int)(tx_data - (u8 *)agg->head),
(usb_complete_t)write_bulk_callback, agg);
@@ -1393,6 +1397,10 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
skb_queue_tail(&tp->tx_queue, skb);
+ if (list_empty(&tp->tx_free) &&
+ skb_queue_len(&tp->tx_queue) > tp->tx_qlen)
+ netif_stop_queue(netdev);
+
if (!list_empty(&tp->tx_free))
tasklet_schedule(&tp->tl);
@@ -1423,6 +1431,14 @@ static void rtl8152_nic_reset(struct r8152 *tp)
}
}
+static void set_tx_qlen(struct r8152 *tp)
+{
+ struct net_device *netdev = tp->netdev;
+
+ tp->tx_qlen = rx_buf_sz / (netdev->mtu + VLAN_ETH_HLEN + VLAN_HLEN +
+ sizeof(struct tx_desc));
+}
+
static inline u8 rtl8152_get_speed(struct r8152 *tp)
{
return ocp_read_byte(tp, MCU_TYPE_PLA, PLA_PHYSTATUS);
@@ -1434,6 +1450,7 @@ static int rtl8152_enable(struct r8152 *tp)
int i, ret;
u8 speed;
+ set_tx_qlen(tp);
speed = rtl8152_get_speed(tp);
if (speed & _10bps) {
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_EEEP_CR);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net v4 1/4] r8152: fix tx/rx memory overflow
From: Hayes Wang @ 2013-11-19 3:25 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Hayes Wang
In-Reply-To: <1384831511-1625-1-git-send-email-hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
The tx/rx would access the memory which is out of the desired range.
Modify the method of checking the end of the memory to avoid it.
For r8152_tx_agg_fill(), the variable remain may become negative.
However, the declaration is unsigned, so the while loop wouldn't
break when reaching the end of the desied memory. Although to change
the declaration from unsigned to signed is enough to fix it, I also
modify the checking method for safe. Replace
remain = rx_buf_sz - sizeof(*tx_desc) -
(u32)((void *)tx_data - agg->head);
with
remain = rx_buf_sz - (int)(tx_agg_align(tx_data) - agg->head);
to make sure the variable remain is always positive. Then, the
overflow wouldn't happen.
For rx_bottom(), the rx_desc should not be used to calculate the
packet length before making sure the rx_desc is in the desired range.
Change the checking to two parts. First, check the descriptor is in
the memory. The other, using the descriptor to find out the packet
length and check if the packet is in the memory.
Signed-off-by: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
---
drivers/net/usb/r8152.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f3fce41..428600d 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -24,7 +24,7 @@
#include <linux/ipv6.h>
/* Version Information */
-#define DRIVER_VERSION "v1.01.0 (2013/08/12)"
+#define DRIVER_VERSION "v1.02.0 (2013/10/28)"
#define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>"
#define DRIVER_DESC "Realtek RTL8152 Based USB 2.0 Ethernet Adapters"
#define MODULENAME "r8152"
@@ -1136,14 +1136,14 @@ r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc, struct sk_buff *skb)
static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
{
- u32 remain;
+ int remain;
u8 *tx_data;
tx_data = agg->head;
agg->skb_num = agg->skb_len = 0;
- remain = rx_buf_sz - sizeof(struct tx_desc);
+ remain = rx_buf_sz;
- while (remain >= ETH_ZLEN) {
+ while (remain >= ETH_ZLEN + sizeof(struct tx_desc)) {
struct tx_desc *tx_desc;
struct sk_buff *skb;
unsigned int len;
@@ -1152,12 +1152,14 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
if (!skb)
break;
+ remain -= sizeof(*tx_desc);
len = skb->len;
if (remain < len) {
skb_queue_head(&tp->tx_queue, skb);
break;
}
+ tx_data = tx_agg_align(tx_data);
tx_desc = (struct tx_desc *)tx_data;
tx_data += sizeof(*tx_desc);
@@ -1167,9 +1169,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
agg->skb_len += len;
dev_kfree_skb_any(skb);
- tx_data = tx_agg_align(tx_data + len);
- remain = rx_buf_sz - sizeof(*tx_desc) -
- (u32)((void *)tx_data - agg->head);
+ tx_data += len;
+ remain = rx_buf_sz - (int)(tx_agg_align(tx_data) - agg->head);
}
usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2),
@@ -1188,7 +1189,6 @@ static void rx_bottom(struct r8152 *tp)
list_for_each_safe(cursor, next, &tp->rx_done) {
struct rx_desc *rx_desc;
struct rx_agg *agg;
- unsigned pkt_len;
int len_used = 0;
struct urb *urb;
u8 *rx_data;
@@ -1204,17 +1204,22 @@ static void rx_bottom(struct r8152 *tp)
rx_desc = agg->head;
rx_data = agg->head;
- pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
- len_used += sizeof(struct rx_desc) + pkt_len;
+ len_used += sizeof(struct rx_desc);
- while (urb->actual_length >= len_used) {
+ while (urb->actual_length > len_used) {
struct net_device *netdev = tp->netdev;
struct net_device_stats *stats;
+ unsigned int pkt_len;
struct sk_buff *skb;
+ pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
if (pkt_len < ETH_ZLEN)
break;
+ len_used += pkt_len;
+ if (urb->actual_length < len_used)
+ break;
+
stats = rtl8152_get_stats(netdev);
pkt_len -= 4; /* CRC */
@@ -1234,9 +1239,8 @@ static void rx_bottom(struct r8152 *tp)
rx_data = rx_agg_align(rx_data + pkt_len + 4);
rx_desc = (struct rx_desc *)rx_data;
- pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
len_used = (int)(rx_data - (u8 *)agg->head);
- len_used += sizeof(struct rx_desc) + pkt_len;
+ len_used += sizeof(struct rx_desc);
}
submit:
--
1.8.3.1
--
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
* [PATCH net v4 0/4] r8152 bug fixes
From: Hayes Wang @ 2013-11-19 3:25 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1384502279-9959-1-git-send-email-hayeswang@realtek.com>
For the patch #1, I modify the type of the variable "pkt_len" from "unsigned"
to "unsigned int".
Hayes Wang (4):
r8152: fix tx/rx memory overflow
r8152: modify the tx flow
r8152: support stopping/waking tx queue
r8152: fix incorrect type in assignment
drivers/net/usb/r8152.c | 109 ++++++++++++++++++++----------------------------
1 file changed, 45 insertions(+), 64 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH 2/2] ipv6: use sk_v6_copy_addrs when memcpy struct ipv6_pinfo
From: Hannes Frederic Sowa @ 2013-11-19 3:14 UTC (permalink / raw)
To: Wang Weidong
Cc: davem, gerrit, kuznet, jmorris, yoshfuji, kaber, vyasevich,
nhorman, dccp, netdev, linux-sctp, dingtianhong
In-Reply-To: <1384829247-8624-3-git-send-email-wangweidong1@huawei.com>
On Tue, Nov 19, 2013 at 10:47:27AM +0800, Wang Weidong wrote:
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 0740f93..83d011e 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1116,6 +1116,10 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
> newtp = tcp_sk(newsk);
>
> memcpy(newnp, np, sizeof(struct ipv6_pinfo));
> + /* Don't forget copy the rcv_saddr and daddr when
> + * copy ipv6_pinfo.
> + */
> + sk_v6_copy_addrs(newsk, sk);
>
> ipv6_addr_set_v4mapped(newinet->inet_daddr, &newsk->sk_v6_daddr);
>
Hmm, how did you spot this?
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH net RESEND] bonding: don't change to 802.3ad mode while ARP monitoring is running
From: Ding Tianhong @ 2013-11-19 3:02 UTC (permalink / raw)
To: Andy Gospodarek, Dan Williams, David Miller
Cc: fubar, nikolay, vfalico, netdev
In-Reply-To: <528ACD8A.1010409@greyhouse.net>
On 2013/11/19 10:31, Andy Gospodarek wrote:
> On 11/18/2013 05:50 PM, Dan Williams wrote:
>> On Mon, 2013-11-18 at 15:48 -0500, David Miller wrote:
>>> From: Dan Williams <dcbw@redhat.com>
>>> Date: Mon, 18 Nov 2013 11:44:42 -0600
>>>
>>>> On Sat, 2013-11-16 at 14:30 +0800, Ding Tianhong wrote:
>>>>> Because the ARP monitoring is not support for 802.3ad, but I still
>>>>> could change the mode to 802.3ad from ab mode while ARP monitoring
>>>>> is running, it is incorrect.
>>>>>
>>>>> So add a check for 802.3ad in bonding_store_mode to fix the problem,
>>>>> and make a new macro BOND_NO_USES_ARP() to simplify the code.
>>>> Instead of failing, couldn't the code stop ARP monitoring and allow the
>>>> mode change? This is similar to setting miimon, which disables ARP
>>>> monitoring, or setting ARP monitoring, which disables miimon.
>>>>
>>>> if (new_value && bond->params.arp_interval) {
>>>> pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
>>>> bond->dev->name);
>>>> bond->params.arp_interval = 0;
>>>> if (bond->params.arp_validate)
>>>> bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
>>>> }
>>>>
>>>> Bond mode is the most important bond option, so it seems like it should
>>>> override any of the other sub-options. I know the code doesn't do this
>>>> now, but maybe instead of the patch you propose, it would be nicer to
>>>> allow the mode change instead?
>>> I agree with Dan, if other mode changes behave this way (by dropping the
>>> incompatible feature) we should make 802.3ad do so as well at the very
>>> least for consistency.
>> Currently ALB and TLB modes will fail if arp_interval > 0, so Ding's
>> patch is technically correct.
>>
>> Instead, I'm proposing that 'mode' trumps all, and if the user changes
>> the mode, conflicting values should be cleared or reset. Otherwise
>> userspace has to duplicate a lot of kernel logic/validation. For
>> example:
>>
>> 1) set mode to ROUNDROBIN
>> 2) set arp_interval
>> 3) set mode to ALB or TLB
>> 4) FAIL - incompatible with arp_interval
>> 5) ok, set arp_interval to zero
>> 6) set mode to ALB or TLB
>> 7) SUCCESS
>>
>> Wouldn't it be nice if the kernel handled clearing arp_interval for us,
>> since it knows that arp_interval is incompatible with ALB/TLB...
>>
>> Could be done separately. I have no objection to Ding's patch other
>> than "life could be even better".
>>
>> Dan
> Nik was actually planning to work on a pretty significant rewrite of the code that handles setting and clearing of different config options, but I have not seen him chime in on this thread yet.
>
> Nik, anything you can share on this or are you still a bit away from coming up with a design and implementation?
>
ok, I will waiting for Nik's suggestion and then decide what to do next.
Ding
> --
> 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
* [PATCH net RESEND] bridge: flush br's address entry in fdb when remove the bridge dev
From: Ding Tianhong @ 2013-11-19 2:57 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller, bridge, Netdev
When the following commands are executed:
brctl addbr br0
ifconfig br0 hw ether <addr>
rmmod bridge
The calltrace will occur:
[ 563.312114] device eth1 left promiscuous mode
[ 563.312188] br0: port 1(eth1) entered disabled state
[ 563.468190] kmem_cache_destroy bridge_fdb_cache: Slab cache still has objects
[ 563.468197] CPU: 6 PID: 6982 Comm: rmmod Tainted: G O 3.12.0-0.7-default+ #9
[ 563.468199] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
[ 563.468200] 0000000000000880 ffff88010f111e98 ffffffff814d1c92 ffff88010f111eb8
[ 563.468204] ffffffff81148efd ffff88010f111eb8 0000000000000000 ffff88010f111ec8
[ 563.468206] ffffffffa062a270 ffff88010f111ed8 ffffffffa063ac76 ffff88010f111f78
[ 563.468209] Call Trace:
[ 563.468218] [<ffffffff814d1c92>] dump_stack+0x6a/0x78
[ 563.468234] [<ffffffff81148efd>] kmem_cache_destroy+0xfd/0x100
[ 563.468242] [<ffffffffa062a270>] br_fdb_fini+0x10/0x20 [bridge]
[ 563.468247] [<ffffffffa063ac76>] br_deinit+0x4e/0x50 [bridge]
[ 563.468254] [<ffffffff810c7dc9>] SyS_delete_module+0x199/0x2b0
[ 563.468259] [<ffffffff814e0922>] system_call_fastpath+0x16/0x1b
[ 570.377958] Bridge firewalling registered
------------------------------------------------
The reason is that when the bridge dev's address is changed, the
br_fdb_change_mac_address() will add new address in fdb, but when
the bridge was removed, the address entry in the fdb did not free,
the bridge_fdb_cache still has objects when destroy the cache, Fix
this by flushing the bridge address entry when removing the bridge.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
net/bridge/br_if.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 6e6194f..98084d8 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -172,6 +172,10 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
del_nbp(p);
}
+ spin_lock_bh(&br->hash_lock);
+ fdb_delete_by_addr(br, br->dev->dev_addr, 0);
+ spin_unlock_bh(&br->hash_lock);
+
br_vlan_flush(br);
del_timer_sync(&br->gc_timer);
--
1.8.0
^ permalink raw reply related
* [PATCH 2/2] ipv6: use sk_v6_copy_addrs when memcpy struct ipv6_pinfo
From: Wang Weidong @ 2013-11-19 2:47 UTC (permalink / raw)
To: davem, gerrit, kuznet, jmorris, yoshfuji, kaber, vyasevich,
nhorman
Cc: dccp, netdev, linux-sctp, dingtianhong
In-Reply-To: <1384829247-8624-1-git-send-email-wangweidong1@huawei.com>
commit efe4208f ("ipv6: make lookups simpler and faster") remove
the daddr and rcv_saddr from struct ipv6_pinfo. When use memcpy
the ipv6_pinfo, we lose the sk_v6_daddr and sk_v6_rcv_saddr, so
use the sk_v6_copy_addrs when needed.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/dccp/ipv6.c | 4 ++++
net/ipv6/tcp_ipv6.c | 4 ++++
net/sctp/ipv6.c | 4 ++++
3 files changed, 12 insertions(+)
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 4ac71ff..90182a8 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -465,6 +465,10 @@ static struct sock *dccp_v6_request_recv_sock(struct sock *sk,
newnp = inet6_sk(newsk);
memcpy(newnp, np, sizeof(struct ipv6_pinfo));
+ /* Don't forget copy the rcv_saddr and daddr when
+ * copy ipv6_pinfo.
+ */
+ sk_v6_copy_addrs(newsk, sk);
ipv6_addr_set_v4mapped(newinet->inet_daddr, &newsk->sk_v6_daddr);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 0740f93..83d011e 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1116,6 +1116,10 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
newtp = tcp_sk(newsk);
memcpy(newnp, np, sizeof(struct ipv6_pinfo));
+ /* Don't forget copy the rcv_saddr and daddr when
+ * copy ipv6_pinfo.
+ */
+ sk_v6_copy_addrs(newsk, sk);
ipv6_addr_set_v4mapped(newinet->inet_daddr, &newsk->sk_v6_daddr);
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 7567e6f..b76f143 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -656,6 +656,10 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
newnp = inet6_sk(newsk);
memcpy(newnp, np, sizeof(struct ipv6_pinfo));
+ /* Don't forget copy the rcv_saddr and daddr when
+ * copy ipv6_pinfo.
+ */
+ sk_v6_copy_addrs(newsk, sk);
/* Initialize sk's sport, dport, rcv_saddr and daddr for getsockname()
* and getpeername().
--
1.7.12
^ permalink raw reply related
* [PATCH 1/2] ipv6: add helper function for copy addrs from old sock
From: Wang Weidong @ 2013-11-19 2:47 UTC (permalink / raw)
To: davem, gerrit, kuznet, jmorris, yoshfuji, kaber, vyasevich,
nhorman
Cc: dccp, netdev, linux-sctp, dingtianhong
In-Reply-To: <1384829247-8624-1-git-send-email-wangweidong1@huawei.com>
Add sk_v6_copy_addrs for copying sk_v6_daddr and sk_v6_rcv_saddr, it
will be used in the coming patch.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
include/net/sock.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/include/net/sock.h b/include/net/sock.h
index e3a18ff..9be2b9f 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -442,6 +442,12 @@ struct sock {
#define SK_CAN_REUSE 1
#define SK_FORCE_REUSE 2
+static inline void sk_v6_copy_addrs(struct sock *newsk, struct sock *oldsk)
+{
+ newsk->sk_v6_daddr = oldsk->sk_v6_daddr;
+ newsk->sk_v6_rcv_saddr = oldsk->sk_v6_rcv_saddr;
+}
+
static inline int sk_peek_offset(struct sock *sk, int flags)
{
if ((flags & MSG_PEEK) && (sk->sk_peek_off >= 0))
--
1.7.12
^ permalink raw reply related
* [PATCH 0/2] ipv6: fix the missing copies when memcpy ipv6_pinfo
From: Wang Weidong @ 2013-11-19 2:47 UTC (permalink / raw)
To: davem, gerrit, kuznet, jmorris, yoshfuji, kaber, vyasevich,
nhorman
Cc: dccp, netdev, linux-sctp, dingtianhong
This patch series include: add the helper function, use it when
memcpy struct ipv6_pinfo.
Wang Weidong (2):
ipv6: add helper function for copy addrs from old sock
ipv6: use sk_v6_copy_addrs when memcpy struct ipv6_pinfo
include/net/sock.h | 6 ++++++
net/dccp/ipv6.c | 4 ++++
net/ipv6/tcp_ipv6.c | 4 ++++
net/sctp/ipv6.c | 4 ++++
4 files changed, 18 insertions(+)
--
1.7.12
^ permalink raw reply
* RE: [PATCH net v3 1/4] r8152: fix tx/rx memory overflow
From: hayeswang @ 2013-11-19 2:32 UTC (permalink / raw)
To: 'David Miller'
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, 'nic_swsd',
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20131115.173948.1712849813995511721.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
> Sent: Saturday, November 16, 2013 6:40 AM
> To: Hayeswang
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; nic_swsd;
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH net v3 1/4] r8152: fix tx/rx memory overflow
>
> From: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
> Date: Fri, 15 Nov 2013 15:57:56 +0800
>
> > + unsigned pkt_len;
>
> Please fully specify the type as "unsigned int". Please
> check for this
> problem in the rest of your patches too.
I would fix it.
I check the other patches, and I don't find the same problem.
Thanks.
Best Regards,
Hayes
--
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
* Re: [PATCH net RESEND] bonding: don't change to 802.3ad mode while ARP monitoring is running
From: Andy Gospodarek @ 2013-11-19 2:31 UTC (permalink / raw)
To: Dan Williams, David Miller; +Cc: dingtianhong, fubar, nikolay, vfalico, netdev
In-Reply-To: <1384815032.4774.55.camel@dcbw.foobar.com>
On 11/18/2013 05:50 PM, Dan Williams wrote:
> On Mon, 2013-11-18 at 15:48 -0500, David Miller wrote:
>> From: Dan Williams <dcbw@redhat.com>
>> Date: Mon, 18 Nov 2013 11:44:42 -0600
>>
>>> On Sat, 2013-11-16 at 14:30 +0800, Ding Tianhong wrote:
>>>> Because the ARP monitoring is not support for 802.3ad, but I still
>>>> could change the mode to 802.3ad from ab mode while ARP monitoring
>>>> is running, it is incorrect.
>>>>
>>>> So add a check for 802.3ad in bonding_store_mode to fix the problem,
>>>> and make a new macro BOND_NO_USES_ARP() to simplify the code.
>>> Instead of failing, couldn't the code stop ARP monitoring and allow the
>>> mode change? This is similar to setting miimon, which disables ARP
>>> monitoring, or setting ARP monitoring, which disables miimon.
>>>
>>> if (new_value && bond->params.arp_interval) {
>>> pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
>>> bond->dev->name);
>>> bond->params.arp_interval = 0;
>>> if (bond->params.arp_validate)
>>> bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
>>> }
>>>
>>> Bond mode is the most important bond option, so it seems like it should
>>> override any of the other sub-options. I know the code doesn't do this
>>> now, but maybe instead of the patch you propose, it would be nicer to
>>> allow the mode change instead?
>> I agree with Dan, if other mode changes behave this way (by dropping the
>> incompatible feature) we should make 802.3ad do so as well at the very
>> least for consistency.
> Currently ALB and TLB modes will fail if arp_interval > 0, so Ding's
> patch is technically correct.
>
> Instead, I'm proposing that 'mode' trumps all, and if the user changes
> the mode, conflicting values should be cleared or reset. Otherwise
> userspace has to duplicate a lot of kernel logic/validation. For
> example:
>
> 1) set mode to ROUNDROBIN
> 2) set arp_interval
> 3) set mode to ALB or TLB
> 4) FAIL - incompatible with arp_interval
> 5) ok, set arp_interval to zero
> 6) set mode to ALB or TLB
> 7) SUCCESS
>
> Wouldn't it be nice if the kernel handled clearing arp_interval for us,
> since it knows that arp_interval is incompatible with ALB/TLB...
>
> Could be done separately. I have no objection to Ding's patch other
> than "life could be even better".
>
> Dan
Nik was actually planning to work on a pretty significant rewrite of the
code that handles setting and clearing of different config options, but
I have not seen him chime in on this thread yet.
Nik, anything you can share on this or are you still a bit away from
coming up with a design and implementation?
^ permalink raw reply
* Re: [PATCH] nsproxy: Check to make sure count is truly zero before freeing
From: Steven Rostedt @ 2013-11-19 1:59 UTC (permalink / raw)
To: Gao feng
Cc: Eric W. Biederman, LKML, netdev@vger.kernel.org, David Miller,
Cedric Le Goater, Paul E. McKenney, Pavel Emelyanov,
Andrew Morton, Clark Williams, Arnaldo Carvalho de Melo
In-Reply-To: <528ABD6E.8020004@cn.fujitsu.com>
On Tue, 19 Nov 2013 09:22:54 +0800
Gao feng <gaofeng@cn.fujitsu.com> wrote:
> On 11/19/2013 08:04 AM, Steven Rostedt wrote:
> >
> > I'll start out saying that this email was a complete oops. I only kept
> > it around for reference, as this didn't fix the bug we were seeing, and
> > I used this email to just document what I initially thought.
> >
>
> Can you describe the panic situation and the way to reproduce?
> it's useful for us to find out the real problem.
>
I need to talk with more people to get more information as it happened
in our test labs. From what I remember, it was caused on 3.10-rt (even
with PREEMPT_RT disabled). We are currently testing 3.10 vanilla, to
make sure that it really does occur on mainline.
>From what I understand, it was ltp-lite that caused the panic and it
happened on a box with 16 CPUs.
I'll keep you posted when we get more data that is relevant to mainline.
This is another reason that the email went out prematurely.
-- Steve
^ permalink raw reply
* Re: [PATCH net RESEND] bonding: don't change to 802.3ad mode while ARP monitoring is running
From: Ding Tianhong @ 2013-11-19 1:48 UTC (permalink / raw)
To: Dan Williams, David Miller; +Cc: fubar, andy, nikolay, vfalico, netdev
In-Reply-To: <1384815032.4774.55.camel@dcbw.foobar.com>
On 2013/11/19 6:50, Dan Williams wrote:
> On Mon, 2013-11-18 at 15:48 -0500, David Miller wrote:
>> From: Dan Williams <dcbw@redhat.com>
>> Date: Mon, 18 Nov 2013 11:44:42 -0600
>>
>>> On Sat, 2013-11-16 at 14:30 +0800, Ding Tianhong wrote:
>>>> Because the ARP monitoring is not support for 802.3ad, but I still
>>>> could change the mode to 802.3ad from ab mode while ARP monitoring
>>>> is running, it is incorrect.
>>>>
>>>> So add a check for 802.3ad in bonding_store_mode to fix the problem,
>>>> and make a new macro BOND_NO_USES_ARP() to simplify the code.
>>>
>>> Instead of failing, couldn't the code stop ARP monitoring and allow the
>>> mode change? This is similar to setting miimon, which disables ARP
>>> monitoring, or setting ARP monitoring, which disables miimon.
>>>
>>> if (new_value && bond->params.arp_interval) {
>>> pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
>>> bond->dev->name);
>>> bond->params.arp_interval = 0;
>>> if (bond->params.arp_validate)
>>> bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
>>> }
>>>
>>> Bond mode is the most important bond option, so it seems like it should
>>> override any of the other sub-options. I know the code doesn't do this
>>> now, but maybe instead of the patch you propose, it would be nicer to
>>> allow the mode change instead?
>>
>> I agree with Dan, if other mode changes behave this way (by dropping the
>> incompatible feature) we should make 802.3ad do so as well at the very
>> least for consistency.
>
> Currently ALB and TLB modes will fail if arp_interval > 0, so Ding's
> patch is technically correct.
>
> Instead, I'm proposing that 'mode' trumps all, and if the user changes
> the mode, conflicting values should be cleared or reset. Otherwise
> userspace has to duplicate a lot of kernel logic/validation. For
> example:
>
> 1) set mode to ROUNDROBIN
> 2) set arp_interval
> 3) set mode to ALB or TLB
> 4) FAIL - incompatible with arp_interval
> 5) ok, set arp_interval to zero
> 6) set mode to ALB or TLB
> 7) SUCCESS
>
> Wouldn't it be nice if the kernel handled clearing arp_interval for us,
> since it knows that arp_interval is incompatible with ALB/TLB...
>
> Could be done separately. I have no objection to Ding's patch other
> than "life could be even better".
>
> Dan
>
>
agree, it could be better, when we set the mode which only support mii, we should
disable the arp monitoring if arp_interval > 0, and restart mii monitor, I will rebuild
the patch and resend it later.
Regards
Ding
>
> .
>
^ permalink raw reply
* Re: [PATCH 1/1] Revert "af-packet: Use existing netdev reference for bound sockets"
From: David Miller @ 2013-11-19 1:30 UTC (permalink / raw)
To: noureddine; +Cc: dborkman, willemb, phil, edumazet, netdev, greearb
In-Reply-To: <20131118.202658.118112656342762223.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 18 Nov 2013 20:26:58 -0500 (EST)
> From: Salam Noureddine <noureddine@aristanetworks.com>
> Date: Mon, 18 Nov 2013 16:17:02 -0800
>
>> Thanks for looking into this. It seems to me that just using a flag
>> won't help in avoiding the race condition since packet_notifier can
>> still run after the flag check.
>
> The bind_lock syncronizes everything.
Well, it should, but it doesn't here.
So what we need is something that synchronizes the packet_notifier with
the sendmsg path, and will make sure the po->running state is stable
throughout the call.
That's all.
^ permalink raw reply
* Re: [PATCH 1/1] Revert "af-packet: Use existing netdev reference for bound sockets"
From: David Miller @ 2013-11-19 1:26 UTC (permalink / raw)
To: noureddine; +Cc: dborkman, willemb, phil, edumazet, netdev, greearb
In-Reply-To: <CAO7SqHCBRrfq3EfLMzrOa_A4e2tyaqdzTo=V7vE06YVnk5D0iA@mail.gmail.com>
From: Salam Noureddine <noureddine@aristanetworks.com>
Date: Mon, 18 Nov 2013 16:17:02 -0800
> Thanks for looking into this. It seems to me that just using a flag
> won't help in avoiding the race condition since packet_notifier can
> still run after the flag check.
The bind_lock syncronizes everything.
^ permalink raw reply
* Re: [PATCH] nsproxy: Check to make sure count is truly zero before freeing
From: Gao feng @ 2013-11-19 1:22 UTC (permalink / raw)
To: Steven Rostedt, Eric W. Biederman
Cc: LKML, netdev@vger.kernel.org, David Miller, Cedric Le Goater,
Paul E. McKenney, Pavel Emelyanov, Andrew Morton, Clark Williams,
Arnaldo Carvalho de Melo
In-Reply-To: <20131118190457.7ad4e0a0@gandalf.local.home>
On 11/19/2013 08:04 AM, Steven Rostedt wrote:
>
> I'll start out saying that this email was a complete oops. I only kept
> it around for reference, as this didn't fix the bug we were seeing, and
> I used this email to just document what I initially thought.
>
Can you describe the panic situation and the way to reproduce?
it's useful for us to find out the real problem.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/1] Revert "af-packet: Use existing netdev reference for bound sockets"
From: Salam Noureddine @ 2013-11-19 0:17 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David Miller, Willem de Bruijn, Phil Sutter, Eric Dumazet, netdev,
greearb
In-Reply-To: <528AA531.3000305@redhat.com>
Hi Daniel,
Thanks for looking into this. It seems to me that just using a flag
won't help in avoiding the race condition
since packet_notifier can still run after the flag check. We need to
have some sort of locking to avoid it.
One possibility is to grab the po->bind_lock (which is taken in
packet_notifier) in packet_snd for bound
sockets, increment the refcnt for the net_device using dev_hold and
release the lock. Not sure if this is
too expensive though and would defeat the purpose of the original
patch. Another problem could be that it
might delay device unregistering if somebody is sending a lot of
packets on the socket.
Please let me know if I am missing something here and I would be happy
to try the patch if my analysis
is wrong.
Thanks,
Salam
On Mon, Nov 18, 2013 at 3:39 PM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 11/18/2013 07:03 PM, David Miller wrote:
>>
>> From: Daniel Borkmann <dborkman@redhat.com>
>> Date: Mon, 18 Nov 2013 10:00:53 +0100
>>
>>> On 11/18/2013 08:40 AM, Salam Noureddine wrote:
>>>>
>>>> This reverts commit 827d978037d7d0bf0860481948c6d26ead10042f
>>>> ("af-packet: Use existing netdev reference for bound sockets")
>>>>
>>>> The patch introduced a race condition between packet_snd and
>>>> packet_notifier when a net_device is being unregistered. In the case
>>>> of
>>>> a bound socket, packet_notifier can drop the last reference to the
>>>> net_device and packet_snd might end up sending a packet over a freed
>>>> net_device.
>>>
>>>
>>> So there's no other workaround possible like e.g. setting a flag in
>>> struct packet_sock so that in case our netdevice goes down, we just
>>> set the flag and if set, we return with -ENXIO in send path?
>>> Reverting this would decrease performance for everyone as we would
>>> then do the lookup every time we send a packet again.
>>
>>
>> Agreed, we should try first to find a reasonable fix instead of just
>> doing a knee-jerk revert of this change.
>
>
> Salam, could you give the below a try on your side ?
>
> I tried reproducing this with trafgen, but couldn't so far, meaning
> everything worked (before/after). Having that said, it could also be
> that I'm currently just having a really crappy nic (asix) at hand.
>
> Let me know, thanks !
>
> From 26caa771c0c7d53b28afafc76f1b91ab36a34e73 Mon Sep 17 00:00:00 2001
> From: Daniel Borkmann <dborkman@redhat.com>
> Date: Mon, 18 Nov 2013 23:31:11 +0100
> Subject: [PATCH] packet: don't send packets after we unregistered proto hook
>
> Salam reported a use after free bug in PF_PACKET that occurs when
> we're sending out frames on a socket bound device and suddenly the
> netdevice is being unregistered.
>
> Salam says:
>
> Commit 827d9780 introduced a race condition between packet_snd
>
> and packet_notifier when a net_device is being unregistered. In
> the case of a bound socket, packet_notifier can drop the last
> reference to the net_device and packet_snd might end up sending
> a packet over a freed net_device.
>
> To avoid reverting 827d9780, we could make use of po->running member
> that gets reset when we're calling __unregister_prot_hook() in
> packet_notifier() when we receive NETDEV_DOWN notification. In
> that case we receive NETDEV_DOWN before NETDEV_UNREGISTER where
> prot_hook.dev is set to NULL, so that we could make sure to
> leave send path early with -ENETDOWN, which corresponds also
> to our notification.
>
> Fixes: 827d978037d7 ("af-packet: Use existing netdev reference for bound
> sockets.")
> Reported-by: Salam Noureddine <noureddine@aristanetworks.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
> net/packet/af_packet.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 2e8286b..e3f64f2 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2094,7 +2094,7 @@ static int tpacket_snd(struct packet_sock *po, struct
> msghdr *msg)
> reserve = dev->hard_header_len;
>
> err = -ENETDOWN;
> - if (unlikely(!(dev->flags & IFF_UP)))
> + if (unlikely(!(dev->flags & IFF_UP) || !po->running))
> goto out_put;
>
> size_max = po->tx_ring.frame_size
> @@ -2250,7 +2250,7 @@ static int packet_snd(struct socket *sock,
> reserve = dev->hard_header_len;
>
> err = -ENETDOWN;
> - if (!(dev->flags & IFF_UP))
> + if (unlikely(!(dev->flags & IFF_UP) || !po->running))
> goto out_unlock;
>
> if (po->has_vnet_hdr) {
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: MLD maturity in kernel version 2.6.32.60
From: Daniel Borkmann @ 2013-11-19 0:12 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, Simon Schneider
In-Reply-To: <20131118221850.GO16541@order.stressinduktion.org>
On 11/18/2013 11:18 PM, Hannes Frederic Sowa wrote:
> On Mon, Nov 18, 2013 at 01:17:28PM +0100, Simon Schneider wrote:
>> in a development project, we started evaluating MLD functionality.
>>
>> Our development is based on kernel version 2.6.32.60.
>>
>> We noticed some pretty basic issues:
>>
>> - With force_mld_version set to 0 (we expect "MLDv2 with MLDv1 fallback"), we saw only MLDv2 messages, the fallback to MLDv1 didn't seem to work, when we sent MLDv1 messages to the unit.
>> - With force_mld_version set to 1 (we expect "MLDv1 only mode"), we also saw MLDv2 messages being generated.
>>
>> Can someone give a general statement about the MLD implementation maturity in kernel 2.6.32.60? Are there known bugs that would explain the above observations?
>>
>> We need to see whether it makes sense to put further effort in investigating/fixing any MLD related issue we see.
>>
>> If this is the wrong list to ask this kind of question, please point me to the correct one.
>
> It would be great if you could share test results with newer kernel with us.
Indeed, please do so, thus in case something is still missing so that we can fix it.
> Last time I checked we did correctly fallback to MDLv1 mode but it has been a
> while.
Yes, the fallback timeout should work now.
> If you come up with a specific patch from Daniel's series which fixes the
> issues you are seeing, we can think about including this to stable.
>
> There are still some issues in mld: we are currently a bit too noisy for my
> taste (e.g. on re-enabling interfaces). This is a big problem for large
> wireless networks e.g.
There's also still one issue which I am hesitant to implement, as i) I don't
think it's overly useful, ii) nobody complained so far. :-)
In RFC2710 (MLD v1 spec), it says:
3.7. Other fields
The length of a received MLD message is computed by taking the IPv6
Payload Length value and subtracting the length of any IPv6 extension
headers present between the IPv6 header and the MLD message. If that
length is greater than 24 octets, that indicates that there are other
fields present beyond the fields described above, perhaps belonging
to a future backwards-compatible version of MLD. An implementation
of the version of MLD specified in this document MUST NOT send an MLD
message longer than 24 octets and MUST ignore anything past the first
24 octets of a received MLD message. In all cases, the MLD checksum
MUST be computed over the entire MLD message, not just the first 24
octets.
Then, RFC3810 which "updates" RFC2710 obviously has a bigger message length
than that. In igmp6_event_query(), we only check for len == MLD_V1_QUERY_LEN
and process MLD v1. Now, in case of MLD v1-only fallback, we MUST only
operate in v1 mode.
Now, in case a v2 message comes in we could assume above statement "if
that length is greater than 24 octets, that indicates that there are other
fields present beyond the fields described above, perhaps belonging to a
future backwards-compatible version of MLD".
But then on the other hand, we get completely wrong "Maximum Response Delay"
codes in the packet header as they are differently encoded in MLD v1 and MLD
v2 thus not really backwards compatible.
^ permalink raw reply
* Re: [PATCH net v3] vti: fix spd lookup: match plaintext pkt, not ipsec pkt
From: Andrew Collins @ 2013-11-19 0:01 UTC (permalink / raw)
To: Saurabh Mohan
Cc: Christophe Gouault, Steffen Klassert, David S. Miller, Herbert Xu,
netdev@vger.kernel.org, Sergei Shtylyov, Eric Dumazet
In-Reply-To: <1902752B0C92F943AB7EA9EE13E2DEEC1273BA977C@HQ1-EXCH02.corp.brocade.com>
> Getting rid of the pre-routing mark, which had to be done outside of
> the vti tunnel code was prone to misconfiguration.
> Though it is unfortunate that it creates a kernel version dependency.
Perhaps a section in the ip-link manpage for VTI describing the
expected configuration is worthwhile?
I quite like the functionality, especially since the pre-routing mark
removal, but I ended up having to read through commit logs to figure
out how to use it which is obviously non-ideal.
^ permalink raw reply
* Re: Fw: [Bug 65131] New: kernel panic (BUG_ON raised) in SCTP function sctp_cmd_interpreter
From: Daniel Borkmann @ 2013-11-18 23:49 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: Stephen Hemminger, netdev
In-Reply-To: <528A6BFB.9050509@gmail.com>
On 11/18/2013 08:35 PM, Vlad Yasevich wrote:
> On 11/18/2013 12:46 PM, Vlad Yasevich wrote:
>> Should be fixed by:
>> commit 7926c1d5be0b7cbe5b8d5c788d7d39237e7b212c
>> Author: Daniel Borkmann <dborkman@redhat.com>
>> Date: Thu Oct 31 09:13:32 2013 +0100
>>
>> net: sctp: do not trigger BUG_ON in sctp_cmd_delete_tcb
>
> Just received confirmation that the above patch has been queued for 3.11.
Indeed, thanks !
^ permalink raw reply
* Re: [PATCH 1/1] Revert "af-packet: Use existing netdev reference for bound sockets"
From: Daniel Borkmann @ 2013-11-18 23:39 UTC (permalink / raw)
To: David Miller; +Cc: noureddine, willemb, phil, edumazet, netdev, greearb
In-Reply-To: <20131118.130310.1183708218770881544.davem@davemloft.net>
On 11/18/2013 07:03 PM, David Miller wrote:
> From: Daniel Borkmann <dborkman@redhat.com>
> Date: Mon, 18 Nov 2013 10:00:53 +0100
>
>> On 11/18/2013 08:40 AM, Salam Noureddine wrote:
>>> This reverts commit 827d978037d7d0bf0860481948c6d26ead10042f
>>> ("af-packet: Use existing netdev reference for bound sockets")
>>>
>>> The patch introduced a race condition between packet_snd and
>>> packet_notifier when a net_device is being unregistered. In the case
>>> of
>>> a bound socket, packet_notifier can drop the last reference to the
>>> net_device and packet_snd might end up sending a packet over a freed
>>> net_device.
>>
>> So there's no other workaround possible like e.g. setting a flag in
>> struct packet_sock so that in case our netdevice goes down, we just
>> set the flag and if set, we return with -ENXIO in send path?
>> Reverting this would decrease performance for everyone as we would
>> then do the lookup every time we send a packet again.
>
> Agreed, we should try first to find a reasonable fix instead of just
> doing a knee-jerk revert of this change.
Salam, could you give the below a try on your side ?
I tried reproducing this with trafgen, but couldn't so far, meaning
everything worked (before/after). Having that said, it could also be
that I'm currently just having a really crappy nic (asix) at hand.
Let me know, thanks !
From 26caa771c0c7d53b28afafc76f1b91ab36a34e73 Mon Sep 17 00:00:00 2001
From: Daniel Borkmann <dborkman@redhat.com>
Date: Mon, 18 Nov 2013 23:31:11 +0100
Subject: [PATCH] packet: don't send packets after we unregistered proto hook
Salam reported a use after free bug in PF_PACKET that occurs when
we're sending out frames on a socket bound device and suddenly the
netdevice is being unregistered.
Salam says:
Commit 827d9780 introduced a race condition between packet_snd
and packet_notifier when a net_device is being unregistered. In
the case of a bound socket, packet_notifier can drop the last
reference to the net_device and packet_snd might end up sending
a packet over a freed net_device.
To avoid reverting 827d9780, we could make use of po->running member
that gets reset when we're calling __unregister_prot_hook() in
packet_notifier() when we receive NETDEV_DOWN notification. In
that case we receive NETDEV_DOWN before NETDEV_UNREGISTER where
prot_hook.dev is set to NULL, so that we could make sure to
leave send path early with -ENETDOWN, which corresponds also
to our notification.
Fixes: 827d978037d7 ("af-packet: Use existing netdev reference for bound sockets.")
Reported-by: Salam Noureddine <noureddine@aristanetworks.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/packet/af_packet.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2e8286b..e3f64f2 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2094,7 +2094,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
reserve = dev->hard_header_len;
err = -ENETDOWN;
- if (unlikely(!(dev->flags & IFF_UP)))
+ if (unlikely(!(dev->flags & IFF_UP) || !po->running))
goto out_put;
size_max = po->tx_ring.frame_size
@@ -2250,7 +2250,7 @@ static int packet_snd(struct socket *sock,
reserve = dev->hard_header_len;
err = -ENETDOWN;
- if (!(dev->flags & IFF_UP))
+ if (unlikely(!(dev->flags & IFF_UP) || !po->running))
goto out_unlock;
if (po->has_vnet_hdr) {
--
1.8.3.1
^ permalink raw reply related
* Re: oops in pskb_expand_head - 3.11.6
From: David Miller @ 2013-11-18 23:03 UTC (permalink / raw)
To: jiri; +Cc: michele, netdev
In-Reply-To: <20131118225945.GD3743@minipsycho.orion>
From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 18 Nov 2013 23:59:45 +0100
> Mon, Nov 18, 2013 at 11:25:43PM CET, hannes@stressinduktion.org wrote:
>>On Mon, Nov 18, 2013 at 10:24:36AM +0100, Jiri Pirko wrote:
>>> It goes hand in hand with 6aafeef03b9d9ecf255f3a80ed85ee070260e1ae.
>>> Without 9037c3579a277f3a23ba476664629fda8c35f7c4 skbs that reasm len is
>>> < MTU would not get fragmented which is not desired.
>>
>>Jiri, David, do you see problems adding this to stable? We got a lot of
>>reports because of exactly those kind of crashes in the past.
>
> At this point, I do not see any reason why not to do it.
I've queued them up for -stable, thanks.
^ permalink raw reply
* Re: oops in pskb_expand_head - 3.11.6
From: Jiri Pirko @ 2013-11-18 22:59 UTC (permalink / raw)
To: Michele Baldessari, netdev
In-Reply-To: <20131118222543.GP16541@order.stressinduktion.org>
Mon, Nov 18, 2013 at 11:25:43PM CET, hannes@stressinduktion.org wrote:
>On Mon, Nov 18, 2013 at 10:24:36AM +0100, Jiri Pirko wrote:
>> Mon, Nov 18, 2013 at 10:19:22AM CET, michele@acksyn.org wrote:
>> >Hi Hannes,
>> >
>> >On Sun, Nov 17, 2013 at 08:18:10PM +0100, Hannes Frederic Sowa wrote:
>> >> On Sun, Nov 17, 2013 at 11:11:50AM +0000, Michele Baldessari wrote:
>> >> > Hi Hannes,
>> >> >
>> >> > On Sun, Nov 17, 2013 at 12:42:23AM +0100, Hannes Frederic Sowa wrote:
>> >> > > On Sat, Nov 16, 2013 at 11:16:15PM +0000, Michele Baldessari wrote:
>> >> > > > Two oops like the following were reported in Fedora 19 - kernel 3.11.6:
>> >> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1015905
>> >> > >
>> >> > > I have not followed that issue that closely, but could you try linus tree
>> >> > > or net-next?
>> >> > >
>> >> > > Maybe those two patches improve the situation:
>> >> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9037c3579a277f3a23ba476664629fda8c35f7c4
>> >> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=6aafeef03b9d9ecf255f3a80ed85ee070260e1ae
>> >> > >
>> >> > indeed a current kernel seems to fix the crash (according to the
>> >> > reporters). I'll see if I manage to bisect exactly.
>> >>
>> >> Great, if you confirm this we can ask David if he adds the commits to the
>> >> -stable queue.
>> >
>> >I can confirm that 6aafeef03b9d9ecf255f3a80ed85ee070260e1ae "netfilter:
>> >push reasm skb through instead of original frag skbs" fixes the oops
>> >mentioned in this thread. I've applied it to a 3.11.8 kernel and it
>> >fixed the crash. I had to slightly tweak it as it does not apply 100%
>> >cleanly due to 795aa6ef6a1aba99050735eadd0c2341b789b53b " netfilter:
>> >pass hook ops to hookfn".
>> >
>> >So definitely -stable material.
>> >
>> >Not sure if you also want to add 9037c3579a277f3a23ba476664629fda8c35f7c4
>> >"ip6_output: fragment outgoing reassembled skb properly". It did not
>> >have any impact in this particular scenario.
>>
>> It goes hand in hand with 6aafeef03b9d9ecf255f3a80ed85ee070260e1ae.
>> Without 9037c3579a277f3a23ba476664629fda8c35f7c4 skbs that reasm len is
>> < MTU would not get fragmented which is not desired.
>
>Jiri, David, do you see problems adding this to stable? We got a lot of
>reports because of exactly those kind of crashes in the past.
At this point, I do not see any reason why not to do it.
>
>Greetings,
>
> Hannes
^ permalink raw reply
* Re: [PATCH net-next] veth: extend features to support tunneling
From: Jesse Gross @ 2013-11-18 22:57 UTC (permalink / raw)
To: David Stevens
Cc: Alexei Starovoitov, David Miller, Eric Dumazet, Eric Dumazet,
John Fastabend, Michael S. Tsirkin, netdev@vger.kernel.org,
netdev-owner, Or Gerlitz, Stephen Hemminger
In-Reply-To: <OFF76C3619.82D931F7-ON85257C27.005F4843-85257C27.006270D6@us.ibm.com>
On Mon, Nov 18, 2013 at 9:55 AM, David Stevens <dlstevens@us.ibm.com> wrote:
> My idea for solving this ICMP issue is to forge an ICMP2BIG with the
> source address of the destination and send it back to the originating VM
> directly. It's complicated a bit because we don't want to use icmp_send()
> on the host, which will go through host routing tables when we don't
> necessarily have an IP address or routing for the bridged domain. So, to
> do this, we really should just have a stand-alone icmp sender for use by
> tunnel endpoints. But if we do this, the VM can get the correct gso_size
> accounting for the tunnel overhead too, although it's abusing PMTUD a bit
> since it doesn't ordinarily apply to hosts on the same L2 network.
OVS used to do exactly this in its tunnel implementation. It worked
fairly well although we ended up taking it out a while back since it
was a little offensive due to the need to forge addresses and have
essentially a parallel stub of an IP stack.
^ permalink raw reply
* Re: net/usb/ax88179_178a driver broken in linux-3.12
From: Mark Lord @ 2013-11-18 22:52 UTC (permalink / raw)
To: David Laight, Eric Dumazet, Ming Lei, davem, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B742F@saturn3.aculab.com>
On 13-11-18 08:32 AM, David Laight wrote:
>> From: David Laight
>>> On 13-11-17 01:56 PM, Mark Lord wrote:
>>>> On 13-11-17 01:35 PM, Mark Lord wrote:
>>>>> The USB3 network adapter locks up consistently for me here in 3.12,
>>>>> but was working without issues in 3.11.x
>>
>> The xhci driver is well broken in 3.12.
>
> To correct myself...
>
> The xhci driver has never correctly support scatter-gather requests.
> In 3.12 code was added to usbnet to generate SG transmits, and to the
> ax88179_178a driver to use them.
> TCP segmentation offload was then enabled - with does generate
> SG transfers.
>
> SG transfers for 'disks' almost certainly work because the
> fragment boundaries are 'adequately aligned'.
>
> David
>
Well, that's all very nice and whatnot,
except the ax88179_178a driver still does not work in linux-3.12,
whereas it works fine in all earlier kernels.
That's a regression.
And a simple revert (earlier in this thread) fixes it.
So.. let's revert it for now, until a proper xhci compatible patch is produced.
--
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com
^ permalink raw reply
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