* [PATCH] Fixed TPACKET V3 to signal poll when block is closed rather than every packet
From: Dan Collins @ 2014-12-19 3:49 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, Dan Collins
Make TPACKET_V3 signal poll when block is closed rather than for every
packet. Side effect is that poll will be signaled when block retire
timer expires which didn't previously happen. Issue was visible when
sending packets at a very low frequency such that all blocks are retired
before packets are received by TPACKET_V3. This caused avoidable packet
loss. The fix ensures that the signal is sent when blocks are closed
which covers the normal path where the block is filled as well as the
path where the timer expires. The case where a block is filled without
moving to the next block (ie. all blocks are full) will still cause poll
to be signaled.
Signed-off-by: Dan Collins <dan@dcollins.co.nz>
---
net/packet/af_packet.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index e52a447..6880f34 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -785,6 +785,7 @@ static void prb_close_block(struct tpacket_kbdq_core *pkc1,
struct tpacket3_hdr *last_pkt;
struct tpacket_hdr_v1 *h1 = &pbd1->hdr.bh1;
+ struct sock *sk = &po->sk;
if (po->stats.stats3.tp_drops)
status |= TP_STATUS_LOSING;
@@ -809,6 +810,8 @@ static void prb_close_block(struct tpacket_kbdq_core *pkc1,
/* Flush the block */
prb_flush_block(pkc1, pbd1, status);
+ sk->sk_data_ready(sk);
+
pkc1->kactive_blk_num = GET_NEXT_PRB_BLK_NUM(pkc1);
}
@@ -2052,12 +2055,12 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
smp_wmb();
#endif
- if (po->tp_version <= TPACKET_V2)
+ if (po->tp_version <= TPACKET_V2) {
__packet_set_status(po, h.raw, status);
- else
+ sk->sk_data_ready(sk);
+ } else {
prb_clear_blk_fill_status(&po->rx_ring);
-
- sk->sk_data_ready(sk);
+ }
drop_n_restore:
if (skb_head != skb->data && skb_shared(skb)) {
--
2.1.3
^ permalink raw reply related
* Re: [PATCH net V2] net: drop the packet when fails to do software segmentation or header check
From: David Miller @ 2014-12-19 4:02 UTC (permalink / raw)
To: jasowang; +Cc: netdev, linux-kernel, eric.dumazet
In-Reply-To: <1418958553-9962-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Fri, 19 Dec 2014 11:09:13 +0800
> Commit cecda693a969816bac5e470e1d9c9c0ef5567bca ("net: keep original skb
> which only needs header checking during software GSO") keeps the original
> skb for packets that only needs header check, but it doesn't drop the
> packet if software segmentation or header check were failed.
>
> Fixes cecda693a9 ("net: keep original skb which only needs header checking
> during software GSO")
>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable, thanks Jason.
^ permalink raw reply
* Re: [PATCH 08/10] tun: Re-uanble UFO support.
From: Jason Wang @ 2014-12-19 4:37 UTC (permalink / raw)
To: vyasevic; +Cc: mst, netdev, Vladislav Yasevich, virtualization, stefanha, ben
In-Reply-To: <5492EEDE.5050000@redhat.com>
On Thu, Dec 18, 2014 at 11:12 PM, Vlad Yasevich <vyasevic@redhat.com>
wrote:
> On 12/18/2014 12:51 AM, Jason Wang wrote:
>>
>>
>> ----- Original Message -----
>>> Now that UFO is split into v4 and v6 parts, we can bring
>>> back v4 support without any trouble.
>>>
>>> Continue to handle legacy applications by selecting the
>>> IPv6 fragment id but do not change the gso type. Thist
>>> makes sure that two legacy VMs may still communicate.
>>>
>>> Based on original work from Ben Hutchings.
>>>
>>> Fixes: 88e0e0e5aa7a ("drivers/net: Disable UFO through virtio")
>>> CC: Ben Hutchings <ben@decadent.org.uk>
>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>> ---
>>> drivers/net/tun.c | 26 ++++++++++++++------------
>>> 1 file changed, 14 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>>> index 9dd3746..8c32fca 100644
>>> --- a/drivers/net/tun.c
>>> +++ b/drivers/net/tun.c
>>> @@ -175,7 +175,7 @@ struct tun_struct {
>>> struct net_device *dev;
>>> netdev_features_t set_features;
>>> #define TUN_USER_FEATURES
>>> (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
>>> - NETIF_F_TSO6)
>>> + NETIF_F_TSO6|NETIF_F_UFO)
>>>
>>> int vnet_hdr_sz;
>>> int sndbuf;
>>> @@ -1152,20 +1152,15 @@ static ssize_t tun_get_user(struct
>>> tun_struct *tun,
>>> struct tun_file *tfile,
>>> skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
>>> break;
>>> case VIRTIO_NET_HDR_GSO_UDP:
>>> - {
>>> - static bool warned;
>>> -
>>> - if (!warned) {
>>> - warned = true;
>>> - netdev_warn(tun->dev,
>>> - "%s: using disabled UFO feature; please fix this
>>> program\n",
>>> - current->comm);
>>> - }
>>> skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
>>> - if (skb->protocol == htons(ETH_P_IPV6))
>>> + if (vlan_get_protocol(skb) == htons(ETH_P_IPV6)) {
>>
>> This probably means UDPv6 with vlan does not work well, looks like
>> another
>> independent fixe and also for stable.
>
> Ok. I can split this out.
>
>>> + /* This allows legacy application to work.
>>> + * Do not change the gso_type as it may
>>> + * not be upderstood by legacy applications.
>>> + */
>>
>> Isn't this an issue that we use SKB_GSO_UDP for a UDPv6 packet?
>> Especially
>> consider we want to fix UFOv6 in the future? We probably can use
>> SKB_GSO_UDP6 here and try to fix it in tun_put_user() if a legacy
>> userspace
>> is detected.
>
> There is a slight problem with this. This will force fragmentation
> of IPv6
> traffic between VMs since UFO6 wouldn't be enabled on a destination
> device.
> So, suddenly older VMs will see a performance regression for large
> UDPv6 traffic.
>
>
> With this code, a legacy VM will continue to receive large UDPv6
> traffic.
> New VMs will have an updated virtio-net which will reset the gso type
> on input.
I get the point and that's why you still want ipv6 offload helpers to
accept SKB_GSO_UDP. So SKB_GSO_UDP was still ambigious since it can be a
UDP6 packet in fact.
I'm not sure, but probably we could just leave the regression of legacy
guest as is and fix current kernel. The sooner we eliminate the
ambigious
the better for future development.
>
>
>>> ipv6_proxy_select_ident(skb);
>>
>> Question still for vlan, is network header correctly set here?
>> Looks like
>> ipv6_proxy_select_ident() depends on correct network header to work.
>>> +
>
> Yes, you are right... I wonder how that worked. I'll re-test.
>
> Thanks
> -vlad
> }
>>> break;
>>> - }
>>> default:
>>> tun->dev->stats.rx_frame_errors++;
>>> kfree_skb(skb);
>>> @@ -1273,6 +1268,8 @@ static ssize_t tun_put_user(struct
>>> tun_struct *tun,
>>> gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
>>> else if (sinfo->gso_type & SKB_GSO_TCPV6)
>>> gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
>>> + else if (sinfo->gso_type & SKB_GSO_UDP)
>>> + gso.gso_type = VIRTIO_NET_HDR_GSO_UDP;
>>> else {
>>> pr_err("unexpected GSO type: "
>>> "0x%x, gso_size %d, hdr_len %d\n",
>>> @@ -1780,6 +1777,11 @@ static int set_offload(struct tun_struct
>>> *tun,
>>> unsigned long arg)
>>> features |= NETIF_F_TSO6;
>>> arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
>>> }
>>> +
>>> + if (arg & TUN_F_UFO) {
>>> + features |= NETIF_F_UFO;
>>> + arg &= ~TUN_F_UFO;
>>> + }
>>> }
>>>
>>> /* This gives the user a way to test for new features in future
>>> by
>>> --
>>> 1.9.3
>>>
>>> --
>>> 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: [RFC PATCH net-next v2 1/1] net: Support for switch port configuration
From: B Viswanath @ 2014-12-19 5:14 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Samudrala, Sridhar, John Fastabend, Varlese, Marco,
netdev@vger.kernel.org, Thomas Graf, Jiri Pirko,
sfeldma@gmail.com, linux-kernel@vger.kernel.org
In-Reply-To: <549367CC.2080307@cumulusnetworks.com>
On 19 December 2014 at 05:18, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> On 12/18/14, 3:26 PM, Samudrala, Sridhar wrote:
>>
>>
>> On 12/18/2014 3:07 PM, Roopa Prabhu wrote:
>>>
>>> On 12/18/14, 11:21 AM, John Fastabend wrote:
>>>>
>>>> On 12/18/2014 10:14 AM, Roopa Prabhu wrote:
>>>>>
>>>>> On 12/18/14, 10:02 AM, Varlese, Marco wrote:
>>>>>>
>>>>>> Removed unnecessary content for ease of reading...
>>>>>>
>>>>>>>>>>>> +/* Switch Port Attributes section */
>>>>>>>>>>>> +
>>>>>>>>>>>> +enum {
>>>>>>>>>>>> + IFLA_ATTR_UNSPEC,
>>>>>>>>>>>> + IFLA_ATTR_LEARNING,
>>>>>>>>>>>
>>>>>>>>>>> Any reason you want learning here ?. This is covered as part of
>>>>>>>>>>> the bridge setlink attributes.
>>>>>>>>>>>
>>>>>>>>>> Yes, because the user may _not_ want to go through a bridge
>>>>>>>>>> interface
>>>>>>>>>
>>>>>>>>> necessarily.
>>>>>>>>> But, the bridge setlink/getlink interface was changed to
>>>>>>>>> accommodate
>>>>>>>
>>>>>>> 'self'
>>>>>>>>>
>>>>>>>>> for exactly such cases.
>>>>>>>>> I kind of understand your case for the other attributes (these are
>>>>>>>>> per port settings that switch asics provide).
>>>>>>>>>
>>>>>>>>> However, i don't understand the reason to pull in bridge attributes
>>>>>>>>> here.
>>>>>>>>>
>>>>>>>> Maybe, I am missing something so you might help. The learning
>>>>>>>> attribute -
>>>>>>>
>>>>>>> in my case - it is like all other attributes: a port attribute (as
>>>>>>> you said, port
>>>>>>> settings that the switch provides per port).
>>>>>>>>
>>>>>>>> So, what I was saying is "why the user shall go through a bridge to
>>>>>>>> configure
>>>>>>>
>>>>>>> the learning attribute"? From my perspective, it is as any other
>>>>>>> attribute and
>>>>>>> as such configurable on the port.
>>>>>>>
>>>>>>> Thinking about this some more, i don't see why any of these
>>>>>>> attributes
>>>>>>> (except loopback. I dont understand the loopback attribute) cant be
>>>>>>> part of
>>>>>>> the birdge port attributes.
>>>>>>>
>>>>>>> With this we will end up adding l2 attributes in two places: the
>>>>>>> general link
>>>>>>> attributes and bridge attributes.
>>>>>>>
>>>>>>> And since we have gone down the path of using
>>>>>>> ndo_bridge_setlink/getlink
>>>>>>> with 'self'....we should stick to that for all l2 attributes.
>>>>>>>
>>>>>>> The idea of overloading ndo_bridge_set/getlink, was to have the same
>>>>>>> set of
>>>>>>> attributes but support both cases where the user wants to go through
>>>>>>> the
>>>>>>> bridge driver or directly to the switch port driver. So, you are not
>>>>>>> really going
>>>>>>> through the bridge driver if you use 'self' and
>>>>>>> ndo_bridge_setlink/getlink.
>>>>>>>
>>>>>> Roopa, one of the comments I got from Thomas Graf on my v1 patch
>>>>>> was that your patch and mine were supplementary ("I think Roopa's
>>>>>> patches are supplementary. Not all switchdev users will be backed
>>>>>> with a Linux Bridge. I therefore welcome your patches very
>>>>>> much")... I also understood by others that the patch made sense for
>>>>>> the same reason. I simply do not understand why these attributes
>>>>>> (and maybe others in the future) could not be configured directly
>>>>>> on a standard port but have to go through a bridge.
>>>>>>
>>>>> ok, i am very confused in that case. The whole moving of bridge
>>>>> attributes from the bridge driver to rtnetlink.c was to make the
>>>>> bridge attributes accessible to any driver who wants to set l2/bridge
>>>>> attributes on their switch ports. So, its unclear to me why we are
>>>>> doing this parallel thing again. This move to rtnetlink.c was done
>>>>> during the recent rocker support. so, maybe scott/jiri can elaborate
>>>>> more.
>>>>
>>>>
>>>> Not sure if this will add to the confusion or help. But you do not
>>>> need to have the bridge.ko loaded or netdev's attached to a bridge
>>>> to use the setlink/getlink ndo ops and netlink messages.
>>>>
>>>> This was intentionally done. Its already used with NIC devices to
>>>> configure embedded bridge settings such as VEB/VEPA.
>>>
>>>
>>> that helps my case, thanks.
>>
>>
>> So the user interface to set/get the per-port attributes will be via
>> 'bridge', not 'ip'
>>
>> bridge link set dev sw0p1 port_attr bcast_flooding 1 self
>> bridge link get dev sw0p1 port_attr bcast_flooding self
>
>
> yes, l2 attributes.
>>
>>
>> We also need an interface to set per-switch attributes. Can this work?
>> bridge link set dev sw0 sw_attr bcast_flooding 1 master
>> where sw0 is a bridge representing the hardware switch.
>
>
> Not today. We discussed this @ LPC, and one way to do this would be to have
> a device
> representing the switch asic. This is in the works.
Can I assume that on platforms which house more than one asic (say
two 24 port asics, interconnected via a 10G link or equivalent, to get
a 48 port 'switch') , the 'rocker' driver (or similar) should expose
them as a single set of ports, and not as two 'switch ports' ?
>
>
> --
> 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] sunvnet: fix a memory leak in vnet_handle_offloads
From: roy.qing.li @ 2014-12-19 5:19 UTC (permalink / raw)
To: netdev
From: Li RongQing <roy.qing.li@gmail.com>
when skb_gso_segment returns error, the original skb should be freed
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
drivers/net/ethernet/sun/sunvnet.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 45c408e..d2835bf 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -1201,6 +1201,7 @@ static int vnet_handle_offloads(struct vnet_port *port, struct sk_buff *skb)
segs = skb_gso_segment(skb, dev->features & ~NETIF_F_TSO);
if (IS_ERR(segs)) {
dev->stats.tx_dropped++;
+ dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
}
--
2.1.0
^ permalink raw reply related
* [PATCH net-next] ethernet: use IS_ERR_OR_NULL as much as possible
From: roy.qing.li @ 2014-12-19 5:20 UTC (permalink / raw)
To: netdev
From: Li RongQing <roy.qing.li@gmail.com>
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
drivers/net/ethernet/broadcom/tg3.c | 2 +-
drivers/net/ethernet/realtek/r8169.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index bb48a61..2e6e504 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -7858,7 +7858,7 @@ static int tg3_tso_bug(struct tg3 *tp, struct tg3_napi *tnapi,
segs = skb_gso_segment(skb, tp->dev->features &
~(NETIF_F_TSO | NETIF_F_TSO6));
- if (IS_ERR(segs) || !segs)
+ if (IS_ERR_OR_NULL(segs))
goto tg3_tso_bug_end;
do {
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 14a1c5c..254dbe4 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6876,7 +6876,7 @@ static void r8169_csum_workaround(struct rtl8169_private *tp,
features &= ~(NETIF_F_SG | NETIF_F_IPV6_CSUM | NETIF_F_TSO6);
segs = skb_gso_segment(skb, features);
- if (IS_ERR(segs) || !segs)
+ if (IS_ERR_OR_NULL(segs))
goto drop;
do {
--
2.1.0
^ permalink raw reply related
* [PATCH net-next] netdevice: optimise netdev_intersect_features slightly
From: roy.qing.li @ 2014-12-19 5:21 UTC (permalink / raw)
To: netdev; +Cc: mkubecek
From: Li RongQing <roy.qing.li@gmail.com>
Since f1 and f2 always have NETIF_F_GEN_CSUM when doing Bitwise OR assignment,
it is unnecessory to clear NETIF_F_GEN_CSUM from the added data
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
Cc: Michal Kubecek <mkubecek@suse.cz>
---
include/linux/netdevice.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c31f74d..b8facf9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3542,9 +3542,9 @@ static inline netdev_features_t netdev_intersect_features(netdev_features_t f1,
netdev_features_t f2)
{
if (f1 & NETIF_F_GEN_CSUM)
- f1 |= (NETIF_F_ALL_CSUM & ~NETIF_F_GEN_CSUM);
+ f1 |= NETIF_F_ALL_CSUM;
if (f2 & NETIF_F_GEN_CSUM)
- f2 |= (NETIF_F_ALL_CSUM & ~NETIF_F_GEN_CSUM);
+ f2 |= NETIF_F_ALL_CSUM;
f1 &= f2;
if (f1 & NETIF_F_GEN_CSUM)
f1 &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_GEN_CSUM);
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net-next] ethernet: use IS_ERR_OR_NULL as much as possible
From: David Miller @ 2014-12-19 5:58 UTC (permalink / raw)
To: roy.qing.li; +Cc: netdev
In-Reply-To: <1418966457-23294-1-git-send-email-roy.qing.li@gmail.com>
net-next is not open for submissions, please resubmit these changes
when I open that tree back up.
^ permalink raw reply
* Re: [iwlwifi] BUG: unable to handle kernel
From: Grumbach, Emmanuel @ 2014-12-19 5:59 UTC (permalink / raw)
To: Wu, Fengguang
Cc: linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, lkp@01.org,
linux-leds@vger.kernel.org
In-Reply-To: <20141218210705.GA26694@wfg-t540p.sh.intel.com>
On Thu, 2014-12-18 at 13:07 -0800, Fengguang Wu wrote:
> On Fri, Dec 19, 2014 at 03:42:17AM +0800, Grumbach, Emmanuel wrote:
> > On Thu, 2014-12-18 at 09:13 -0800, Fengguang Wu wrote:
> > > Hi All,
> > >
> > > I don't see any relationship between the BUG and this bisected commit.
> > > Anyway, it's better to report it to the lists than to ignore.
> >
> > Right - but I have to say that I have no clue how this comment can cause
> > the bug you are seeing...
>
> s?comment?commit?
>
Yes :)
> > Do you even have an Intel Wireless device the VM could access?
>
> Nope. It's simple QEMU virtual machine boot test.
>
In that case - this just can't be right... Don't know what to say...
> Thanks,
> Fengguang
>
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-fixes.git master
> > >
> > > commit 03d6c3b0fa4f5f0379cede079ec828a6c999fe43
> > > Author: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > > AuthorDate: Wed Dec 3 10:39:07 2014 +0200
> > > Commit: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > > CommitDate: Sun Dec 14 10:20:29 2014 +0200
> > >
> > > iwlwifi: pcie: re-ACK all interrupts after device reset
> > >
> > > When we reset the device, the CSR_INT gets cleared as well
> > > as CSR_INT_MASK. Meaning that we shouldn't get any interrupt
> > > but, due to a hardware bug, recent devices will keep sending
> > > interrupts. This leads to an interrupt storm while stopping
> > > the device.
> > > The way to fix this is to ACK all the interrupts after the
> > > device is reset so that the value of CSR_INT will stay
> > > 0xffffffff.
> > >
> > > Fixes: 522713c81e4e ("iwlwifi: pcie: properly reset the device")
> > > Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > >
> > > +------------------------------------------+------------+------------+------------+
> > > | | 0a79a0c011 | 03d6c3b0fa | iwlwifi-fi |
> > > +------------------------------------------+------------+------------+------------+
> > > | boot_successes | 60 | 19 | 3 |
> > > | boot_failures | 0 | 1 | 9 |
> > > | BUG:unable_to_handle_kernel | 0 | 1 | 9 |
> > > | Oops | 0 | 1 | 9 |
> > > | RIP:strcmp | 0 | 1 | 9 |
> > > | Kernel_panic-not_syncing:Fatal_exception | 0 | 1 | 9 |
> > > | backtrace:led_trigger_register_simple | 0 | 1 | 9 |
> > > | backtrace:ledtrig_usb_init | 0 | 1 | 9 |
> > > | backtrace:kernel_init_freeable | 0 | 1 | 9 |
> > > +------------------------------------------+------------+------------+------------+
> > >
> > > [ 5.345018] g_serial gadget: Gadget Serial v2.4
> > > [ 5.345927] g_serial gadget: g_serial ready
> > > [ 5.345927] g_serial gadget: g_serial ready
> > > [ 5.346777] BUG: unable to handle kernel
> > > [ 5.346777] BUG: unable to handle kernel paging requestpaging request at ffff88000004e5f0
> > > at ffff88000004e5f0
> > > [ 5.348183] IP:
> > > [ 5.348183] IP: [<ffffffff81446a68>] strcmp+0x6/0x20
> > > [<ffffffff81446a68>] strcmp+0x6/0x20
> > > [ 5.349183] PGD 37f1067
> > > [ 5.349183] PGD 37f1067 PUD 37f2067 PUD 37f2067 PMD 37f3067 PMD 37f3067 PTE 800000000004e060PTE 800000000004e060
> > >
> > > [ 5.350498] Oops: 0000 [#1]
> > > [ 5.350498] Oops: 0000 [#1] DEBUG_PAGEALLOCDEBUG_PAGEALLOC
> > >
> > > [ 5.351360] CPU: 0 PID: 1 Comm: swapper Not tainted 3.18.0-g03d6c3b #1
> > > [ 5.351360] CPU: 0 PID: 1 Comm: swapper Not tainted 3.18.0-g03d6c3b #1
> > > [ 5.352660] task: ffff880012060000 ti: ffff88001204c000 task.ti: ffff88001204c000
> > > [ 5.352660] task: ffff880012060000 ti: ffff88001204c000 task.ti: ffff88001204c000
> > > [ 5.354143] RIP: 0010:[<ffffffff81446a68>]
> > > [ 5.354143] RIP: 0010:[<ffffffff81446a68>] [<ffffffff81446a68>] strcmp+0x6/0x20
> > > [<ffffffff81446a68>] strcmp+0x6/0x20
> > > [ 5.354451] RSP: 0000:ffff88001204fe28 EFLAGS: 00010246
> > > [ 5.354451] RSP: 0000:ffff88001204fe28 EFLAGS: 00010246
> > > [ 5.354451] RAX: 0000000000000000 RBX: ffff88000c08fe00 RCX: ffffffff81d35310
> > > [ 5.354451] RAX: 0000000000000000 RBX: ffff88000c08fe00 RCX: ffffffff81d35310
> > > [ 5.354451] RDX: ffff88000c08fe68 RSI: ffffffff826d05be RDI: ffff88000004e5f0
> > > [ 5.354451] RDX: ffff88000c08fe68 RSI: ffffffff826d05be RDI: ffff88000004e5f0
> > > [ 5.354451] RBP: ffff88001204fe28 R08: 0000000000000001 R09: 000000000000033a
> > > [ 5.354451] RBP: ffff88001204fe28 R08: 0000000000000001 R09: 000000000000033a
> > > [ 5.354451] R10: 0000000000000000 R11: ffffffff82531cd1 R12: ffff88000c19fa00
> > > [ 5.354451] R10: 0000000000000000 R11: ffffffff82531cd1 R12: ffff88000c19fa00
> > > [ 5.354451] R13: 0000000000000000 R14: ffffffff837958b8 R15: 0000000000000000
> > > [ 5.354451] R13: 0000000000000000 R14: ffffffff837958b8 R15: 0000000000000000
> > > [ 5.354451] FS: 0000000000000000(0000) GS:ffffffff82789000(0000) knlGS:0000000000000000
> > > [ 5.354451] FS: 0000000000000000(0000) GS:ffffffff82789000(0000) knlGS:0000000000000000
> > > [ 5.354451] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > > [ 5.354451] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > > [ 5.354451] CR2: ffff88000004e5f0 CR3: 0000000002776000 CR4: 00000000000006b0
> > > [ 5.354451] CR2: ffff88000004e5f0 CR3: 0000000002776000 CR4: 00000000000006b0
> > > [ 5.354451] Stack:
> > > [ 5.354451] Stack:
> > > [ 5.354451] ffff88001204fe58
> > > [ 5.354451] ffff88001204fe58 ffffffff81d35334 ffffffff81d35334 0000000000000000 0000000000000000 ffff88000c19fa00 ffff88000c19fa00
> > >
> > > [ 5.354451] ffffffff826d05be
> > > [ 5.354451] ffffffff826d05be 0000000000000000 0000000000000000 ffff88001204fe88 ffff88001204fe88 ffffffff81d35648 ffffffff81d35648
> > >
> > > [ 5.354451] ffff88000e3bbcc0
> > > [ 5.354451] ffff88000e3bbcc0 ffffffff82b3fe61 ffffffff82b3fe61 0000000000000000 0000000000000000 ffffffff82b98910 ffffffff82b98910
> > >
> > > [ 5.354451] Call Trace:
> > > [ 5.354451] Call Trace:
> > > [ 5.354451] [<ffffffff81d35334>] led_trigger_register+0x63/0x129
> > > [ 5.354451] [<ffffffff81d35334>] led_trigger_register+0x63/0x129
> > > [ 5.354451] [<ffffffff81d35648>] led_trigger_register_simple+0x35/0x79
> > > [ 5.354451] [<ffffffff81d35648>] led_trigger_register_simple+0x35/0x79
> > > [ 5.354451] [<ffffffff82b3fe61>] ? gs_bind+0xea/0xea
> > > [ 5.354451] [<ffffffff82b3fe61>] ? gs_bind+0xea/0xea
> > > [ 5.354451] [<ffffffff82b3fe78>] ledtrig_usb_init+0x17/0x2e
> > > [ 5.354451] [<ffffffff82b3fe78>] ledtrig_usb_init+0x17/0x2e
> > > [ 5.354451] [<ffffffff82b00044>] do_one_initcall+0xe6/0x171
> > > [ 5.354451] [<ffffffff82b00044>] do_one_initcall+0xe6/0x171
> > > [ 5.354451] [<ffffffff82b001c7>] kernel_init_freeable+0xf8/0x180
> > > [ 5.354451] [<ffffffff82b001c7>] kernel_init_freeable+0xf8/0x180
> > > [ 5.354451] [<ffffffff82060791>] ? rest_init+0xbd/0xbd
> > > [ 5.354451] [<ffffffff82060791>] ? rest_init+0xbd/0xbd
> > > [ 5.354451] [<ffffffff8206079a>] kernel_init+0x9/0xd0
> > > [ 5.354451] [<ffffffff8206079a>] kernel_init+0x9/0xd0
> > > [ 5.354451] [<ffffffff8207d2ba>] ret_from_fork+0x7a/0xb0
> > > [ 5.354451] [<ffffffff8207d2ba>] ret_from_fork+0x7a/0xb0
> > > [ 5.354451] [<ffffffff82060791>] ? rest_init+0xbd/0xbd
> > > [ 5.354451] [<ffffffff82060791>] ? rest_init+0xbd/0xbd
> > > [ 5.354451] Code:
> > > [ 5.354451] Code: c0 c0 eb eb f5 f5 31 31 c9 c9 40 40 8a 8a 3c 3c 0e 0e 4d 4d 8d 8d 0c 0c 08 08 40 40 84 84 ff ff 41 41 88 88 3c 3c 08 08 74 74 0d 0d 48 48 ff ff c1 c1 48 48 39 39 ca ca 75 75 e7 e7 41 41 c6 c6 41 41 01 01 00 00 5d 5d c3 c3 55 55 31 31 c0 c0 48 48 89 89 e5 e5 <8a> <8a> 14 14 07 07 3a 3a 14 14 06 06 74 74 07 07 19 19 c0 c0 83 83 c8 c8 01 01 eb eb 09 09 48 48 ff ff c0 c0 84 84 d2 d2 75 75
> > >
> > > [ 5.354451] RIP
> > > [ 5.354451] RIP [<ffffffff81446a68>] strcmp+0x6/0x20
> > > [<ffffffff81446a68>] strcmp+0x6/0x20
> > > [ 5.354451] RSP <ffff88001204fe28>
> > > [ 5.354451] RSP <ffff88001204fe28>
> > > [ 5.354451] CR2: ffff88000004e5f0
> > > [ 5.354451] CR2: ffff88000004e5f0
> > > [ 5.354451] ---[ end trace 8f9377b34c860a0c ]---
> > > [ 5.354451] ---[ end trace 8f9377b34c860a0c ]---
> > >
> > > git bisect start baa21e834941ee5fbe4bd421c871f7c0c5f9a086 70e71ca0af244f48a5dcf56dc435243792e3a495 --
> > > git bisect bad 03d6c3b0fa4f5f0379cede079ec828a6c999fe43 # 16:23 0- 1 iwlwifi: pcie: re-ACK all interrupts after device reset
> > > git bisect good 0a79a0c011cb291675e3b80760a452fcba5c59d9 # 16:28 20+ 0 iwlwifi: mvm: clear IN_HW_RESTART flag on stop()
> > > # first bad commit: [03d6c3b0fa4f5f0379cede079ec828a6c999fe43] iwlwifi: pcie: re-ACK all interrupts after device reset
> > > git bisect good 0a79a0c011cb291675e3b80760a452fcba5c59d9 # 16:30 60+ 0 iwlwifi: mvm: clear IN_HW_RESTART flag on stop()
> > > # extra tests on HEAD of iwlwifi-fixes/master
> > > git bisect bad baa21e834941ee5fbe4bd421c871f7c0c5f9a086 # 16:30 0- 9 iwlwifi: pcie: limit fw chunk sizes given to fh
> > > # extra tests on tree/branch iwlwifi-fixes/master
> > > git bisect bad baa21e834941ee5fbe4bd421c871f7c0c5f9a086 # 16:30 0- 9 iwlwifi: pcie: limit fw chunk sizes given to fh
> > > # extra tests on tree/branch linus/master
> > > git bisect good 44e8967d591686463e84a88b46b03beba3ab49fb # 16:32 60+ 0 Ceph: remove left-over reject file
> > > # extra tests on tree/branch next/master
> > > git bisect good cfaa3a95dd2b402599b1d8122dc3107478db8717 # 16:35 60+ 1 Add linux-next specific files for 20141218
> > >
> > >
> > > This script may reproduce the error.
> > >
> > > ----------------------------------------------------------------------------
> > > #!/bin/bash
> > >
> > > kernel=$1
> > > initrd=quantal-core-x86_64.cgz
> > >
> > > wget --no-clobber https://github.com/fengguang/reproduce-kernel-bug/raw/master/initrd/$initrd
> > >
> > > kvm=(
> > > qemu-system-x86_64
> > > -cpu kvm64
> > > -enable-kvm
> > > -kernel $kernel
> > > -initrd $initrd
> > > -m 320
> > > -smp 2
> > > -net nic,vlan=1,model=e1000
> > > -net user,vlan=1
> > > -boot order=nc
> > > -no-reboot
> > > -watchdog i6300esb
> > > -rtc base=localtime
> > > -serial stdio
> > > -display none
> > > -monitor null
> > > )
> > >
> > > append=(
> > > hung_task_panic=1
> > > earlyprintk=ttyS0,115200
> > > debug
> > > apic=debug
> > > sysrq_always_enabled
> > > rcupdate.rcu_cpu_stall_timeout=100
> > > panic=-1
> > > softlockup_panic=1
> > > nmi_watchdog=panic
> > > oops=panic
> > > load_ramdisk=2
> > > prompt_ramdisk=0
> > > console=ttyS0,115200
> > > console=tty0
> > > vga=normal
> > > root=/dev/ram0
> > > rw
> > > drbd.minor_count=8
> > > )
> > >
> > > "${kvm[@]}" --append "${append[*]}"
> > > ----------------------------------------------------------------------------
> > >
> > > Thanks,
> > > Fengguang
> > > _______________________________________________
> > > LKP mailing list
> > > LKP@linux.intel.com
> >
^ permalink raw reply
* [PATCH Iproute2 next] ip netns: 'ip netns id' cmd without argument should not give error
From: Mahesh Bandewar @ 2014-12-19 6:05 UTC (permalink / raw)
To: netdev, Stephen Hemminger; +Cc: Vadim Kochan, Saied Kazemi, Mahesh Bandewar
The command 'ip netns identify' without PID parameter is supposed to
use the self PID to identify its ns but a trivial error prevented it
from doing so. This patch fixes that error.
So before the patch -
# ip netns id
No pid specified
# echo $?
1
#
After the patch -
# ip netns id
test-ns
# echo $?
0
#
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: Vadim Kochan <vadim4j@gmail.com>
Cc: Saied Kazemi <saied@google.com>
---
ip/ipnetns.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 1c8aa029073e..ec9afa2177a5 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -298,7 +298,7 @@ static int netns_identify(int argc, char **argv)
DIR *dir;
struct dirent *entry;
- if (argc < 1) {
+ if (!argc) {
pidstr = "self";
} else if (argc > 1) {
fprintf(stderr, "extra arguments specified\n");
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* Re: [PATCH net-next] netdevice: optimise netdev_intersect_features slightly
From: Michal Kubecek @ 2014-12-19 6:09 UTC (permalink / raw)
To: roy.qing.li; +Cc: netdev
In-Reply-To: <1418966487-23347-1-git-send-email-roy.qing.li@gmail.com>
On Fri, Dec 19, 2014 at 01:21:27PM +0800, roy.qing.li@gmail.com wrote:
> From: Li RongQing <roy.qing.li@gmail.com>
>
> Since f1 and f2 always have NETIF_F_GEN_CSUM when doing Bitwise OR assignment,
> it is unnecessory to clear NETIF_F_GEN_CSUM from the added data
>
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> Cc: Michal Kubecek <mkubecek@suse.cz>
> ---
> include/linux/netdevice.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index c31f74d..b8facf9 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3542,9 +3542,9 @@ static inline netdev_features_t netdev_intersect_features(netdev_features_t f1,
> netdev_features_t f2)
> {
> if (f1 & NETIF_F_GEN_CSUM)
> - f1 |= (NETIF_F_ALL_CSUM & ~NETIF_F_GEN_CSUM);
> + f1 |= NETIF_F_ALL_CSUM;
> if (f2 & NETIF_F_GEN_CSUM)
> - f2 |= (NETIF_F_ALL_CSUM & ~NETIF_F_GEN_CSUM);
> + f2 |= NETIF_F_ALL_CSUM;
> f1 &= f2;
> if (f1 & NETIF_F_GEN_CSUM)
> f1 &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_GEN_CSUM);
This is not an optimization in the sense of resulting code as the
expression right of "|=" is a constant so the patch just replaces or
with one constant by an or with a different one. But the source looks
nicer so it may make sense anyway.
However, I will have to take a look at both versions of the resulting
code first as we are using the same expression (or its bitwise inverse)
later so that current version might be actually slightly more efficient.
Michal Kubecek
^ permalink raw reply
* [patches] a bunch of old bluetooth fixes
From: Al Viro @ 2014-12-19 6:18 UTC (permalink / raw)
To: David Miller
Cc: Marcel Holtmann, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA
This stuff has been sitting in my queue since March; basically,
several places in net/bluetooth assume that they are dealing with
l2cap sockets, while it is possible to get an arbitrary socket to those.
Results are not pretty.
* HIDPCONNADD gets an arbitrary user-supplied socket; the code
it calls (hidp_connection_add()) verifies that the socket is l2cap one,
but before doing so it finds l2cap_pi(ctrl_sock->sk)->chan. It's not
that big a deal (it's only 5 words past the end of struct sock), but
it's trivial to avoid and, in theory, we might end up oopsing here if
we are very unlucky and it happens to hit an unmapped page just past
the actual object ctrl_sock->sk sits in.
* CMTP counterpart of that doesn't validate the socket at all.
It proceeds to
s = __cmtp_get_session(&l2cap_pi(sock->sk)->chan->dst);
which can very easily oops - here ->chan is already garbage and we
proceed to dereference that. As with HIDP, one needs CAP_NET_ADMIN to
trigger that, but it's really a clear bug. The only sanity check we
do is verifying that nsock->sk->sk_state is equal to BT_CONNECTED,
which is not unique to bluetooth, to put it mildly. It's just 1,
so a TCP_ESTABLISHED tcp socket will pass that check just fune.
The fix is trivial...
* BNEP situation is identical to CMTP one.
I've sent these patches back then (March 10), but they seem to have fallen
through the cracks. The bugs are still there and the fixes still apply.
If you would prefer me to resend them after -rc1, just tell...
Anyway, patches follow
^ permalink raw reply
* [PATCH 1/3] bluetooth: hidp_connection_add() unsafe use of l2cap_pi()
From: Al Viro @ 2014-12-19 6:20 UTC (permalink / raw)
To: David Miller; +Cc: Marcel Holtmann, netdev, linux-bluetooth
In-Reply-To: <20141219061801.GU22149@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
it's OK after we'd verified the sockets, but not before that.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/bluetooth/hidp/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index cc25d0b..07348e1 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -1314,13 +1314,14 @@ int hidp_connection_add(struct hidp_connadd_req *req,
{
struct hidp_session *session;
struct l2cap_conn *conn;
- struct l2cap_chan *chan = l2cap_pi(ctrl_sock->sk)->chan;
+ struct l2cap_chan *chan;
int ret;
ret = hidp_verify_sockets(ctrl_sock, intr_sock);
if (ret)
return ret;
+ chan = l2cap_pi(ctrl_sock->sk)->chan;
conn = NULL;
l2cap_chan_lock(chan);
if (chan->conn)
--
2.1.3
^ permalink raw reply related
* [PATCH 2/3] cmtp: cmtp_add_connection() should verify that it's dealing with l2cap socket
From: Al Viro @ 2014-12-19 6:20 UTC (permalink / raw)
To: David Miller; +Cc: Marcel Holtmann, netdev, linux-bluetooth
In-Reply-To: <20141219061801.GU22149@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
... rather than relying on ciptool(8) never passing it anything else. Give
it e.g. an AF_UNIX connected socket (from socketpair(2)) and it'll oops,
trying to evaluate &l2cap_pi(sock->sk)->chan->dst...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/bluetooth/cmtp/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bluetooth/cmtp/core.c b/net/bluetooth/cmtp/core.c
index 67fe5e8..fd57db8 100644
--- a/net/bluetooth/cmtp/core.c
+++ b/net/bluetooth/cmtp/core.c
@@ -333,6 +333,8 @@ int cmtp_add_connection(struct cmtp_connadd_req *req, struct socket *sock)
int i, err;
BT_DBG("");
+ if (!l2cap_is_socket(sock))
+ return -EBADFD;
session = kzalloc(sizeof(struct cmtp_session), GFP_KERNEL);
if (!session)
--
2.1.3
^ permalink raw reply related
* [PATCH 3/3] bnep: bnep_add_connection() should verify that it's dealing with l2cap socket
From: Al Viro @ 2014-12-19 6:20 UTC (permalink / raw)
To: David Miller; +Cc: Marcel Holtmann, netdev, linux-bluetooth
In-Reply-To: <20141219061801.GU22149@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
same story as cmtp
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
net/bluetooth/bnep/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index 85bcc21..ce82722d0 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -533,6 +533,9 @@ int bnep_add_connection(struct bnep_connadd_req *req, struct socket *sock)
BT_DBG("");
+ if (!l2cap_is_socket(sock))
+ return -EBADFD;
+
baswap((void *) dst, &l2cap_pi(sock->sk)->chan->dst);
baswap((void *) src, &l2cap_pi(sock->sk)->chan->src);
--
2.1.3
^ permalink raw reply related
* RE: [PATCH net] r8152: drop the tx packet with invalid length
From: Hayes Wang @ 2014-12-19 6:42 UTC (permalink / raw)
To: David Miller, eric.dumazet@gmail.com
Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org
In-Reply-To: <20141126.153356.461494573591656082.davem@davemloft.net>
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, November 27, 2014 4:34 AM
[...]
> >> > Looks like a candidate for ndo_gso_check(), so that we do not drop, but
> >> > instead segment from netif_needs_gso()/validate_xmit_skb()
> >>
> >> You mean have the bridge implement the ndo_gso_check() method right?
> >
> > No, I meant this particular driver.
> >
> > Note that netif_skb_features() does only this check :
> >
> > if (gso_segs > dev->gso_max_segs || gso_segs < dev->gso_min_segs)
> > features &= ~NETIF_F_GSO_MASK;
> >
> > Ie not testing gso_max_size
> >
> > It looks like all these particular tests should be moved on
> > ndo_gso_check(), to remove code from netif_skb_features()
>
> A check against gso_max_size is generic enough that it ought to be put
> right into netif_needs_gso() rather then duplicating it into every
> driver's ndo_gso_check() method don't you think?
Excuse me. I try to implement ndo_gso_check() with kernel 3.18.
However, I still get packets with gso and their skb lengths are more
than the acceptable one. Do I miss something?
+static bool rtl8152_gso_check(struct sk_buff *skb, struct net_device *dev)
+{
+ if ((skb->len + sizeof(struct tx_desc)) > agg_buf_sz)
+ return false;
+
+ return true;
+}
@@ -5861,6 +5876,9 @@ static const struct net_device_ops rtl8152_netdev_ops = {
.ndo_set_mac_address = rtl8152_set_mac_address,
.ndo_change_mtu = rtl8152_change_mtu,
.ndo_validate_addr = eth_validate_addr,
+ .ndo_gso_check = rtl8152_gso_check,
};
Best Regards,
Hayes
^ permalink raw reply
* Re: [PATCH net-next v3] Add support of Cavium Liquidio ethernet adapters
From: Stephen Hemminger @ 2014-12-19 6:59 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: davem, netdev, Derek Chickles, Satanand Burla, Felix Manlunas,
Raghu Vatsavayi
In-Reply-To: <1418959519-31681-1-git-send-email-rvatsavayi@caviumnetworks.com>
On Thu, 18 Dec 2014 19:25:19 -0800
Raghu Vatsavayi <rvatsavayi@caviumnetworks.com> wrote:
> +static struct {
> + const char str[ETH_GSTRING_LEN];
> +} ethtool_stats_keys[] = {
> + {
> + "jiffies"
> + }, {
Jiffies does not belong in ethtool stats. Looks like some developer debug option.
^ permalink raw reply
* Re: [PATCH net-next v3] Add support of Cavium Liquidio ethernet adapters
From: Stephen Hemminger @ 2014-12-19 7:01 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: davem, netdev, Derek Chickles, Satanand Burla, Felix Manlunas,
Raghu Vatsavayi
In-Reply-To: <1418959519-31681-1-git-send-email-rvatsavayi@caviumnetworks.com>
On Thu, 18 Dec 2014 19:25:19 -0800
Raghu Vatsavayi <rvatsavayi@caviumnetworks.com> wrote:
> +
> +static u32 lio_get_link(struct net_device *dev)
> +{
> + u32 ret;
> +
> + ret = netif_carrier_ok(dev) ? 1 : 0;
> + return ret;
> +}
> +
This is unnecessary, this is what the default ethtool handler already does
if you give a NULL handler.
^ permalink raw reply
* Re: [PATCH net-next v3] Add support of Cavium Liquidio ethernet adapters
From: Stephen Hemminger @ 2014-12-19 7:03 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: davem, netdev, Derek Chickles, Satanand Burla, Felix Manlunas,
Raghu Vatsavayi
In-Reply-To: <1418959519-31681-1-git-send-email-rvatsavayi@caviumnetworks.com>
On Thu, 18 Dec 2014 19:25:19 -0800
Raghu Vatsavayi <rvatsavayi@caviumnetworks.com> wrote:
> + if (linfo->link.s.status) {
> + ecmd->speed = linfo->link.s.speed;
> + ecmd->duplex = linfo->link.s.duplex;
> + } else {
> + ecmd->speed = -1;
> + ecmd->duplex = -1;
> + }
> +
You should SPEED_UNKNOWN and DUPLEX_UNKNOWN (not -1).
and don't set ecmd->speed directly, use
ethtool_cmd_speed_sed(ecmd, speed)
^ permalink raw reply
* Re: [PATCH net-next v3] Add support of Cavium Liquidio ethernet adapters
From: Stephen Hemminger @ 2014-12-19 7:05 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: davem, netdev, Derek Chickles, Satanand Burla, Felix Manlunas,
Raghu Vatsavayi
In-Reply-To: <1418959519-31681-1-git-send-email-rvatsavayi@caviumnetworks.com>
On Thu, 18 Dec 2014 19:25:19 -0800
Raghu Vatsavayi <rvatsavayi@caviumnetworks.com> wrote:
> +static void
> +lio_get_ethtool_stats(struct net_device *netdev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + struct lio *lio = GET_LIO(netdev);
> + struct octeon_device *oct_dev = lio->oct_dev;
> + int i = 0, j;
> +
> + data[i++] = jiffies;
> + data[i++] = lio->stats.tx_packets;
> + data[i++] = lio->stats.tx_bytes;
> + data[i++] = lio->stats.rx_packets;
> + data[i++] = lio->stats.rx_bytes;
> + data[i++] = lio->stats.tx_errors;
> + data[i++] = lio->stats.tx_dropped;
> + data[i++] = lio->stats.rx_dropped;
> +
Do not mirror existing netdevice statistics in ethtool.
The purpose of ethtool stats is to provide device specific information.
^ permalink raw reply
* Re: [PATCH net-next v3] Add support of Cavium Liquidio ethernet adapters
From: Stephen Hemminger @ 2014-12-19 7:07 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: davem, netdev, Derek Chickles, Satanand Burla, Felix Manlunas,
Raghu Vatsavayi
In-Reply-To: <1418959519-31681-1-git-send-email-rvatsavayi@caviumnetworks.com>
On Thu, 18 Dec 2014 19:25:19 -0800
Raghu Vatsavayi <rvatsavayi@caviumnetworks.com> wrote:
> +/** \brief Network device get stats
> + * @param netdev pointer to network device
> + * @returns pointer stats structure
> + */
> +static struct net_device_stats *liquidio_stats(struct net_device *netdev)
> +{
> + return &(GET_LIO(netdev)->stats);
> +}
> +
Unnecessary function, this is what network core does already
if .ndo_get_stats is NULL;
^ permalink raw reply
* Re: [PATCH net-next v3] Add support of Cavium Liquidio ethernet adapters
From: Stephen Hemminger @ 2014-12-19 7:11 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: davem, netdev, Derek Chickles, Satanand Burla, Felix Manlunas,
Raghu Vatsavayi
In-Reply-To: <1418959519-31681-1-git-send-email-rvatsavayi@caviumnetworks.com>
On Thu, 18 Dec 2014 19:25:19 -0800
Raghu Vatsavayi <rvatsavayi@caviumnetworks.com> wrote:
> + if (packet_was_received) {
> + lio->stats.rx_bytes += len;
> + atomic64_inc((atomic64_t *)&lio->stats.rx_packets);
> + netdev->last_rx = jiffies;
> + } else {
> + atomic64_inc((atomic64_t *)&lio->stats.rx_dropped);
> + lio_info(lio, rx_err,
> + "netif_rxXX error rx_dropped:%ld\n",
> + lio->stats.rx_dropped);
> + }
What is point of keeping own statistics in atomic (which are slower)?
Just use dev->stats directly.
^ permalink raw reply
* Re: [PATCH net-next v3] Add support of Cavium Liquidio ethernet adapters
From: Stephen Hemminger @ 2014-12-19 7:14 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: davem, netdev, Derek Chickles, Satanand Burla, Felix Manlunas,
Raghu Vatsavayi
In-Reply-To: <1418959519-31681-1-git-send-email-rvatsavayi@caviumnetworks.com>
On Thu, 18 Dec 2014 19:25:19 -0800
Raghu Vatsavayi <rvatsavayi@caviumnetworks.com> wrote:
> +static int liquidio_xmit(struct sk_buff *skb, struct net_device *netdev)
> +{
> + struct lio *lio;
> + struct octnet_buf_free_info *finfo;
> + union octnic_cmd_setup cmdsetup;
> + struct octnic_data_pkt ndata;
> + struct octeon_device *oct;
> + int cpu = 0, status = 0;
> + int q_idx = 0;
> + int xmit_more;
> +
> + lio = GET_LIO(netdev);
> + oct = lio->oct_dev;
> + atomic64_inc((atomic64_t *)&lio->stats.tx_packets);
> + lio->stats.tx_bytes += skb->len;
> +
> + if (!ifstate_check(lio, LIO_IFSTATE_TXENABLED)) {
> + lio_info(lio, tx_err, "Transmit Busy, not enabled\n");
> + return NETDEV_TX_BUSY;
> + }
This kind of busy handling in transmit, will cause the send in higher
level to retry and spin the CPU. Better to do proper flow control
like other drivers.
^ permalink raw reply
* Re: [PATCH net-next v3] Add support of Cavium Liquidio ethernet adapters
From: Stephen Hemminger @ 2014-12-19 7:21 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: davem, netdev, Derek Chickles, Satanand Burla, Felix Manlunas,
Raghu Vatsavayi
In-Reply-To: <1418959519-31681-1-git-send-email-rvatsavayi@caviumnetworks.com>
On Thu, 18 Dec 2014 19:25:19 -0800
Raghu Vatsavayi <rvatsavayi@caviumnetworks.com> wrote:
> +static int liquidio_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
> +{
> + switch (cmd) {
> + case SIOCSHWTSTAMP:
> + return hwtstamp_ioctl(netdev, ifr, cmd);
> + default:
> + return -EINVAL
Standard error code for unsupported ioctl is -EOPNOTSUPP
^ permalink raw reply
* Re: [PATCH net-next v3] Add support of Cavium Liquidio ethernet adapters
From: Stephen Hemminger @ 2014-12-19 7:23 UTC (permalink / raw)
To: Raghu Vatsavayi
Cc: davem, netdev, Derek Chickles, Satanand Burla, Felix Manlunas,
Raghu Vatsavayi
In-Reply-To: <1418959519-31681-1-git-send-email-rvatsavayi@caviumnetworks.com>
On Thu, 18 Dec 2014 19:25:19 -0800
Raghu Vatsavayi <rvatsavayi@caviumnetworks.com> wrote:
> +static int __init liquidio_init(void)
> +{
> + int i;
> + struct handshake *hs;
> +
> + pr_info("LiquidIO: Starting Network module version %s\n",
> + LIQUIDIO_VERSION);
> +
Do you really need to log this. Systems are already too chatty.
^ 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