* Re: [PATCH 0/8] csiostor: Chelsio FCoE offload driver submission
From: Naresh Kumar Inna @ 2012-08-24 19:04 UTC (permalink / raw)
To: David Miller
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <20120824.130406.1059160453285824849.davem@davemloft.net>
On 8/24/2012 10:34 PM, David Miller wrote:
> From: Naresh Kumar Inna <naresh@chelsio.com>
> Date: Fri, 24 Aug 2012 03:57:45 +0530
>
>> This is the initial submission of the Chelsio FCoE offload driver (csiostor)
>> to the upstream kernel. This driver currently supports FCoE offload
>> functionality over Chelsio T4-based 10Gb Converged Network Adapters.
>>
>> The following patches contain the driver sources for csiostor driver and
>> updates to firmware/hardware header files shared between csiostor and
>> cxgb4 (Chelsio T4-based NIC driver). The csiostor driver is dependent on these
>> header updates. These patches have been generated against scsi 'misc' branch.
>>
>> csiostor is a low level SCSI driver that interfaces with PCI, SCSI midlayer and
>> FC transport subsystems. This driver claims the FCoE PCIe function on the
>> Chelsio Converged Network Adapter. It relies on firmware events for slow path
>> operations like discovery, thereby offloading session management. The driver
>> programs firmware via Work Request interfaces for fast path I/O offload
>> features.
>
> You are going to have to get rid of these module parameters.
>
> That have to do with things that are in no way specific to your device,
> and therefore should be configured using generic kernel facilities.
>
> Using driver specific module parameters results in a poor user
> experience, because in order to make a configuration change the user
> has to know exactly what kind of device and driver is underneath,
> and then learn what the unique method is to make that configuration
> change.
>
> If you use a generic facility, the user only needs to learn one way to
> make a configuration change, regardless of device type and driver.
>
Hi Dave,
Thanks for reviewing. Is your comment with respect to any particular
module parameter[s] in this driver or all of them?
Regards,
Naresh.
^ permalink raw reply
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Stephen Warren @ 2012-08-24 18:56 UTC (permalink / raw)
To: Timur Tabi
Cc: Fleming Andy-AFLEMING, David Miller, ddaney.cavm@gmail.com,
netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
Scott Wood
In-Reply-To: <5037C9AF.3060903@freescale.com>
On 08/24/2012 12:36 PM, Timur Tabi wrote:
> Stephen Warren wrote:
>> When translating the child node's reg property into the parent's address
>> space, the parent's reg property shouldn't even be used at all; all the
>> mapping is done through the ranges property.
>>
>> I thought the code error-checked for a missing ranges property, but I
>> guess not...
>
> I don't think 'ranges' is always necessary, because sometimes the child
> nodes have a different address space that's not mapped to the parent. For
> instance, I2C devices have addresses that are not mapped to the I2C
> controller itself.
In the I2C case, the address spaces are disjoint, so there's never any
mapping between them, so there's no need for ranges.
Any time the child address space is intended to be part of the parent's
address space, I believe ranges is supposed to be specified, perhaps
even mandatory, even if the translation is 1:1.
> Anyway, thanks to Scott for helping me figure this out. I was missing a
> ranges property:
>
> fpga: board-control@3,0 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
> reg = <3 0 0x30>;
> ranges = <0 3 0 0x30>;
>
> This maps the child address of 0 to the parent address of 3 0.
Yes, that looks reasonable.
^ permalink raw reply
* Re: [patch net-next 0/3] team: couple of fixes
From: Jiri Pirko @ 2012-08-24 18:53 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, brouer@redhat.com, ogerlitz@mellanox.com
In-Reply-To: <20120824.134705.178022240623528140.davem@davemloft.net>
24. 8. 2012 v 19:47, David Miller <davem@davemloft.net>:
> From: Jiri Pirko <jiri@resnulli.us>
> Date: Thu, 23 Aug 2012 15:26:50 +0200
>
>> + vlan helper
>>
>> Jiri Pirko (3):
>> teamd: don't print warn message on -ESRCH during event send
>> vlan: add helper which can be called to se it device is used by vlan
>> team: do not allow to add VLAN challenged port when vlan is used
>
> Applied with typos corrected :-)
I owe you a beer :)
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 6/8] csiostor: Chelsio FCoE offload driver submission (headers part 1).
From: Naresh Kumar Inna @ 2012-08-24 18:45 UTC (permalink / raw)
To: Love, Robert W
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <50367309.3080306@intel.com>
On 8/23/2012 11:45 PM, Love, Robert W wrote:
> On 8/23/2012 3:27 PM, Naresh Kumar Inna wrote:
>> This patch contains the first set of the header files for csiostor driver.
>>
>> Signed-off-by: Naresh Kumar Inna <naresh@chelsio.com>
>> ---
>> drivers/scsi/csiostor/csio_defs.h | 143 ++++++
>> drivers/scsi/csiostor/csio_fcoe_proto.h | 843 +++++++++++++++++++++++++++++++
>
> I think most of these protocol definitions can be found in
> include/scsi/fc/ and possibly incluse/scsi/scsi_transport_fc.h. I think
> any protocol stuff missing should be added to the common header files.
>
> Thanks, //Rob--
Hi Rob,
Thanks for reviewing this patch. I will look into how best I can re-use
the headers in include/scsi/fc and get back if I have questions.
Regards,
Naresh.
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Scott Wood @ 2012-08-24 18:43 UTC (permalink / raw)
To: Timur Tabi
Cc: Stephen Warren, Fleming Andy-AFLEMING, David Miller,
ddaney.cavm@gmail.com, netdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org
In-Reply-To: <5037C9AF.3060903@freescale.com>
On 08/24/2012 01:36 PM, Timur Tabi wrote:
> Stephen Warren wrote:
>> When translating the child node's reg property into the parent's address
>> space, the parent's reg property shouldn't even be used at all; all the
>> mapping is done through the ranges property.
>>
>> I thought the code error-checked for a missing ranges property, but I
>> guess not...
>
> I don't think 'ranges' is always necessary, because sometimes the child
> nodes have a different address space that's not mapped to the parent. For
> instance, I2C devices have addresses that are not mapped to the I2C
> controller itself.
>
> Anyway, thanks to Scott for helping me figure this out. I was missing a
> ranges property:
>
> fpga: board-control@3,0 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
> reg = <3 0 0x30>;
> ranges = <0 3 0 0x30>;
>
> This maps the child address of 0 to the parent address of 3 0. It seems
> obvious now, but it was driving me crazy. We've never put child devices
> under our FPGA nodes, so there was no prior use case of a 'ranges'
> property in any of the localbus devices that I could learn from. Plus,
> this is the first time we're probing directly on a child of a localbus device.
There's ep8248e.dts, not that I'd have expected you to look there. :-)
-Scott
^ permalink raw reply
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Timur Tabi @ 2012-08-24 18:36 UTC (permalink / raw)
To: Stephen Warren
Cc: Fleming Andy-AFLEMING, David Miller, ddaney.cavm@gmail.com,
netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
Scott Wood
In-Reply-To: <5037C815.4020906@wwwdotorg.org>
Stephen Warren wrote:
> When translating the child node's reg property into the parent's address
> space, the parent's reg property shouldn't even be used at all; all the
> mapping is done through the ranges property.
>
> I thought the code error-checked for a missing ranges property, but I
> guess not...
I don't think 'ranges' is always necessary, because sometimes the child
nodes have a different address space that's not mapped to the parent. For
instance, I2C devices have addresses that are not mapped to the I2C
controller itself.
Anyway, thanks to Scott for helping me figure this out. I was missing a
ranges property:
fpga: board-control@3,0 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
reg = <3 0 0x30>;
ranges = <0 3 0 0x30>;
This maps the child address of 0 to the parent address of 3 0. It seems
obvious now, but it was driving me crazy. We've never put child devices
under our FPGA nodes, so there was no prior use case of a 'ranges'
property in any of the localbus devices that I could learn from. Plus,
this is the first time we're probing directly on a child of a localbus device.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH 6/8] csiostor: Chelsio FCoE offload driver submission (headers part 1).
From: Naresh Kumar Inna @ 2012-08-24 18:36 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <1345751929.10190.83.camel@haakon2.linux-iscsi.org>
On 8/24/2012 1:28 AM, Nicholas A. Bellinger wrote:
> On Fri, 2012-08-24 at 03:57 +0530, Naresh Kumar Inna wrote:
>> This patch contains the first set of the header files for csiostor driver.
>>
>> Signed-off-by: Naresh Kumar Inna <naresh@chelsio.com>
>> ---
>> drivers/scsi/csiostor/csio_defs.h | 143 ++++++
>> drivers/scsi/csiostor/csio_fcoe_proto.h | 843 +++++++++++++++++++++++++++++++
>> drivers/scsi/csiostor/csio_hw.h | 668 ++++++++++++++++++++++++
>> drivers/scsi/csiostor/csio_init.h | 158 ++++++
>> 4 files changed, 1812 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/scsi/csiostor/csio_defs.h
>> create mode 100644 drivers/scsi/csiostor/csio_fcoe_proto.h
>> create mode 100644 drivers/scsi/csiostor/csio_hw.h
>> create mode 100644 drivers/scsi/csiostor/csio_init.h
>>
>
> Hi Naresh,
>
> Just commenting on csio_defs.h bits here... As Robert mentioned, you'll
> need to convert the driver to use (or add to) upstream protocol
> definitions and drop the csio_fcoe_proto.h bits..
>
Hi Nicholas,
I would like take up the discussion of the protocol header file in that
email thread. Please find the rest of my replies inline.
Thanks for reviewing,
Naresh.
>> diff --git a/drivers/scsi/csiostor/csio_defs.h b/drivers/scsi/csiostor/csio_defs.h
>> new file mode 100644
>> index 0000000..4f1c713
>> --- /dev/null
>> +++ b/drivers/scsi/csiostor/csio_defs.h
>> @@ -0,0 +1,143 @@
>> +/*
>> + * This file is part of the Chelsio FCoE driver for Linux.
>> + *
>> + * Copyright (c) 2008-2012 Chelsio Communications, Inc. All rights reserved.
>> + *
>> + * This software is available to you under a choice of one of two
>> + * licenses. You may choose to be licensed under the terms of the GNU
>> + * General Public License (GPL) Version 2, available from the file
>> + * COPYING in the main directory of this source tree, or the
>> + * OpenIB.org BSD license below:
>> + *
>> + * Redistribution and use in source and binary forms, with or
>> + * without modification, are permitted provided that the following
>> + * conditions are met:
>> + *
>> + * - Redistributions of source code must retain the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer.
>> + *
>> + * - Redistributions in binary form must reproduce the above
>> + * copyright notice, this list of conditions and the following
>> + * disclaimer in the documentation and/or other materials
>> + * provided with the distribution.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
>> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
>> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
>> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>> + * SOFTWARE.
>> + */
>> +
>> +#ifndef __CSIO_DEFS_H__
>> +#define __CSIO_DEFS_H__
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/timer.h>
>> +#include <linux/list.h>
>> +#include <linux/bug.h>
>> +#include <linux/pci.h>
>> +#include <linux/jiffies.h>
>> +
>> +/* Function returns */
>> +enum csio_retval {
>> + CSIO_SUCCESS = 0,
>> + CSIO_INVAL = 1,
>> + CSIO_BUSY = 2,
>> + CSIO_NOSUPP = 3,
>> + CSIO_TIMEOUT = 4,
>> + CSIO_NOMEM = 5,
>> + CSIO_NOPERM = 6,
>> + CSIO_RETRY = 7,
>> + CSIO_EPROTO = 8,
>> + CSIO_EIO = 9,
>> + CSIO_CANCELLED = 10,
>> +};
>> +
>
> Please don't assign macros for errno's, and give them positive values.
>
Although some of these return values appear to be mapped to errno
values, there are others that do not have an errno equivalent (example
CSIO_CANCELLED). We may have future needs to have more driver/protocol
specific return values as well. What do you suggest?
>> +#define csio_retval_t enum csio_retval
>
> Please get rid of this csio_retval_t nonsense.
I can get rid of the typedef and use enum csio_retval instead.
>
>> +
>> +enum {
>> + CSIO_FALSE = 0,
>> + CSIO_TRUE = 1,
>> +};
>> +
>
> Same here, please use normal Boolean macros
>
>> +#define CSIO_ROUNDUP(__v, __r) (((__v) + (__r) - 1) / (__r))
>> +#define CSIO_INVALID_IDX 0xFFFFFFFF
>> +#define csio_inc_stats(elem, val) ((elem)->stats.val++)
>> +#define csio_dec_stats(elem, val) ((elem)->stats.val--)
>
> No reason for either of this stats inc+dec macros. Please drop them.
I will get rid of them.
>
>> +#define csio_valid_wwn(__n) ((*__n >> 4) == 0x5 ? CSIO_TRUE : \
>> + CSIO_FALSE)
>> +#define CSIO_WORD_TO_BYTE 4
>> +
>> +static inline int
>> +csio_list_deleted(struct list_head *list)
>> +{
>> + return ((list->next == list) && (list->prev == list));
>> +}
>> +
>> +#define csio_list_next(elem) (((struct list_head *)(elem))->next)
>> +#define csio_list_prev(elem) (((struct list_head *)(elem))->prev)
>> +
>> +#define csio_deq_from_head(head, elem) \
>> +do { \
>> + if (!list_empty(head)) { \
>> + *((struct list_head **)(elem)) = csio_list_next((head)); \
>> + csio_list_next((head)) = \
>> + csio_list_next(csio_list_next((head))); \
>> + csio_list_prev(csio_list_next((head))) = (head); \
>> + INIT_LIST_HEAD(*((struct list_head **)(elem))); \
>> + } else \
>> + *((struct list_head **)(elem)) = (struct list_head *)NULL;\
>> +} while (0)
>> +
>
> This code is confusing as hell.. Why can't you just use normal list.h
> macros for this..?
I have not found an equivalent function in list.h that does the above
and the following macro. Could you please point me to it? I have seen a
couple of other drivers define their own macros to achieve what this
macro does, hence I assumed there isnt a list.h macro that does this.
>> +#define csio_deq_from_tail(head, elem) \
>> +do { \
>> + if (!list_empty(head)) { \
>> + *((struct list_head **)(elem)) = csio_list_prev((head)); \
>> + csio_list_prev((head)) = \
>> + csio_list_prev(csio_list_prev((head))); \
>> + csio_list_next(csio_list_prev((head))) = (head); \
>> + INIT_LIST_HEAD(*((struct list_head **)(elem))); \
>> + } else \
>> + *((struct list_head **)(elem)) = (struct list_head *)NULL;\
>> +} while (0)
>> +
>
> Same here.. Please don't use macros like this.
>
>> +/* State machine */
>> +typedef void (*csio_sm_state_t)(void *, uint32_t);
>> +
>> +struct csio_sm {
>> + struct list_head sm_list;
>> + csio_sm_state_t sm_state;
>> +};
>> +
>> +#define csio_init_state(__smp, __state) \
>> + (((struct csio_sm *)(__smp))->sm_state = (csio_sm_state_t)(__state))
>> +
>> +#define csio_set_state(__smp, __state) \
>> + (((struct csio_sm *)(__smp))->sm_state = (csio_sm_state_t)(__state))
>> +
>> +
>> +#define csio_post_event(__smp, __evt) \
>> + (((struct csio_sm *)(__smp))->sm_state((__smp), (uint32_t)(__evt)))
>> +
>> +#define csio_get_state(__smp) (((struct csio_sm *)(__smp))->sm_state)
>> +
>> +#define csio_match_state(__smp, __state) \
>> + (csio_get_state((__smp)) == (csio_sm_state_t)(__state))
>> +
>
> Why does any of the sm_state stuff need to be in macros..? Please
> inline all of this code.
I will change them to static inline.
>
>> +#define CSIO_ASSERT(cond) \
>> +do { \
>> + if (unlikely(!((cond)))) \
>> + BUG(); \
>> +} while (0)
>> +
>> +#ifdef __CSIO_DEBUG__
>> +#define CSIO_DB_ASSERT(__c) CSIO_ASSERT((__c))
>> +#else
>> +#define CSIO_DB_ASSERT(__c)
>> +#endif
>> +
>> +#endif /* ifndef __CSIO_DEFS_H__ */
>
>
^ permalink raw reply
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Stephen Warren @ 2012-08-24 18:29 UTC (permalink / raw)
To: Timur Tabi
Cc: Fleming Andy-AFLEMING, David Miller, ddaney.cavm@gmail.com,
netdev@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
Scott Wood
In-Reply-To: <5037AB67.6030106@freescale.com>
On 08/24/2012 10:27 AM, Timur Tabi wrote:
> Stephen Warren wrote:
>>>>>> Then, that'd have to be <9 1>;
>>>>
>>>> Actually, I had #size-cells = <0>.
>
>> I think that if you have #size-cells=<0>, then you'll see the following
>> error message when attempting to translate the address into the parent's
>> address space:
>>
>> prom_parse: Bad cell count for /board-control@3,0/mdio-mux-emi2
>
> It doesn't appear to be working. Here's my tree:
>
> / {
> model = "fsl,P5020DS";
> compatible = "fsl,P5020DS";
> #address-cells = <2>;
> #size-cells = <2>;
> interrupt-parent = <&mpic>;
>
> lbc: localbus@ffe124000 {
> reg = <0xf 0xfe124000 0 0x1000>;
> ranges = <0 0 0xf 0xe8000000 0x08000000
> 2 0 0xf 0xffa00000 0x00040000
> 3 0 0xf 0xffdf0000 0x00008000>;
>
> fpga: board-control@3,0 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
> reg = <3 0 0x30>;
>
> mdio-mux-emi1 {
> compatible = "mdio-mux-mmioreg";
> mdio-parent-bus = <&mdio0>;
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <9 1>; // BRDCFG1
> mux-mask = <0x78>; // EMI1
>
> That means that the physical address that I need is fffdf0009. However,
> when I call of_address_to_resource(), the returned address I get is fe8000009.
>
> So it's not picking up the "3" in the 'reg' property of the
> board-control@3,0 node. What am I missing? Do I need a 'ranges' property
> in the board-control@3,0 node?
Yes.
When translating the child node's reg property into the parent's address
space, the parent's reg property shouldn't even be used at all; all the
mapping is done through the ranges property.
I thought the code error-checked for a missing ranges property, but I
guess not...
^ permalink raw reply
* Re: [PATCH 5/8] csiostor: Chelsio FCoE offload driver submission (sources part 5).
From: Joe Perches @ 2012-08-24 18:27 UTC (permalink / raw)
To: Naresh Kumar Inna
Cc: Nicholas A. Bellinger, JBottomley@parallels.com,
linux-scsi@vger.kernel.org, Dimitrios Michailidis,
netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <5037BC94.1050201@chelsio.com>
On Fri, 2012-08-24 at 23:10 +0530, Naresh Kumar Inna wrote:
> Is there a TRUE/FALSE define in a standard header file?
include/linux/stddef.h:
enum {
false = 0,
true = 1
};
> I see a lot of
> kernel code defining their own TRUE/FALSE.
Those should be changed eventually.
^ permalink raw reply
* Re: [RFC PATCH bridge 4/5] bridge: Add private ioctls to configure vlans on bridge ports
From: Vlad Yasevich @ 2012-08-24 18:19 UTC (permalink / raw)
To: paulmck; +Cc: netdev
In-Reply-To: <20120824175616.GL2472@linux.vnet.ibm.com>
On 08/24/2012 01:56 PM, Paul E. McKenney wrote:
> On Thu, Aug 23, 2012 at 03:29:54PM -0400, Vlad Yasevich wrote:
>> Add a private ioctl to add and remove vlan configuration on bridge port.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>
> One question below...
>
> index 7222fe1..3a5b1f9 100644
>> --- a/net/bridge/br_ioctl.c
>> +++ b/net/bridge/br_ioctl.c
>> @@ -289,6 +289,37 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>> case BRCTL_GET_FDB_ENTRIES:
>> return get_fdb_entries(br, (void __user *)args[1],
>> args[2], args[3]);
>> + case BRCTL_ADD_VLAN:
>> + {
>> + struct net_bridge_port *p;
>> +
>> + if (!capable(CAP_NET_ADMIN))
>> + return -EPERM;
>> +
>> + rcu_read_lock();
>> + if ((p = br_get_port(br, args[1])) == NULL) {
>> + rcu_read_unlock();
>> + return -EINVAL;
>> + }
>> + rcu_read_unlock();
>
> Why is it safe to pass "p" out of the RCU read-side critical section?
> I don't see that br_get_port() does anything to make this safe, at least
> not in v3.5.
>
You right. It not really safe. As Stephen pointed out, it is
accidentally protected by rtnl, so it will not go away. However, that's
not a good excuse and I've already fixed it in my tree.
thanks
-vlad
>> + return br_set_port_vlan(p, args[2]);
>> + }
>> +
>> + case BRCTL_DEL_VLAN:
>> + {
>> + struct net_bridge_port *p;
>> +
>> + if (!capable(CAP_NET_ADMIN))
>> + return -EPERM;
>> +
>> + rcu_read_lock();
>> + if ((p = br_get_port(br, args[1])) == NULL) {
>> + rcu_read_unlock();
>> + return -EINVAL;
>> + }
>> + rcu_read_unlock();
>> + br_set_port_vlan(p, args[2]);
>> + }
>> }
>>
>> return -EOPNOTSUPP;
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index b6c56ab..5639c1c 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -402,6 +402,8 @@ extern int br_del_if(struct net_bridge *br,
>> extern int br_min_mtu(const struct net_bridge *br);
>> extern netdev_features_t br_features_recompute(struct net_bridge *br,
>> netdev_features_t features);
>> +extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
>> +extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
>>
>> /* br_input.c */
>> extern int br_handle_frame_finish(struct sk_buff *skb);
>> --
>> 1.7.7.6
>>
>> --
>> 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
>>
>
> --
> 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: Gianfar ethernet drop package
From: Eric Dumazet @ 2012-08-24 18:17 UTC (permalink / raw)
To: Aníbal Almeida Pinto; +Cc: netdev
In-Reply-To: <5037A86E.2000004@efacec.com>
On Fri, 2012-08-24 at 17:14 +0100, Aníbal Almeida Pinto wrote:
> Hi,
>
> I have an board with a ppc with a 8379, kernel 3.0.39 and with a Gianfar
> Ethernet Controller Version 1.2.
>
> When connect the board to office network the number of dropped packets
> reported by ifconfig continues to increase.
>
> Following the test made at [1] I use tcpdump and when running it the
> number of drop packages don't have increased.
>
> When executing ethtool eth0 -S it return a rx-dropped-by-kernel != 0.
> And the rest of drop packages
>
> Is possible to get info why the packages are dropped?
You receive frames for unknown protocols. So they are dropped.
When running tcpdump, your tcpdump get them, they are not dropped.
So just parse your tcpdump output ?
^ permalink raw reply
* Re: [RFC PATCH bridge 4/5] bridge: Add private ioctls to configure vlans on bridge ports
From: Stephen Hemminger @ 2012-08-24 18:11 UTC (permalink / raw)
To: paulmck; +Cc: Vlad Yasevich, netdev
In-Reply-To: <20120824175616.GL2472@linux.vnet.ibm.com>
On Fri, 24 Aug 2012 10:56:16 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> > + return -EPERM;
> > +
> > + rcu_read_lock();
> > + if ((p = br_get_port(br, args[1])) == NULL) {
> > + rcu_read_unlock();
> > + return -EINVAL;
> > + }
> > + rcu_read_unlock();
>
> Why is it safe to pass "p" out of the RCU read-side critical section?
> I don't see that br_get_port() does anything to make this safe, at least
> not in v3.5.
It would be accidentally protected by rtnl?
^ permalink raw reply
* Re: [net-next 04/13] e1000e: cleanup checkpatch PREFER_PR_LEVEL warning
From: Joe Perches @ 2012-08-24 18:02 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Bruce Allan, netdev, gospo, sassmann
In-Reply-To: <1345730504.16471.3.camel@joe2Laptop>
On Thu, 2012-08-23 at 07:01 -0700, Joe Perches wrote:
> On Thu, 2012-08-23 at 02:56 -0700, Jeff Kirsher wrote:
> > From: Bruce Allan <bruce.w.allan@intel.com>
> >
> > checkpatch warning: Prefer pr_info(... to printk(KERN_INFO, ...
> []
> > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> []
> > @@ -4330,9 +4330,8 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
> > u32 ctrl = er32(CTRL);
> >
> > /* Link status message must follow this format for user tools */
> > - printk(KERN_INFO "e1000e: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
> > - adapter->netdev->name,
> > - adapter->link_speed,
> > + pr_info("e1000e: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
> > + adapter->netdev->name, adapter->link_speed,
> > adapter->link_duplex == FULL_DUPLEX ? "Full" : "Half",
> > (ctrl & E1000_CTRL_TFCE) && (ctrl & E1000_CTRL_RFCE) ? "Rx/Tx" :
> > (ctrl & E1000_CTRL_RFCE) ? "Rx" :
>
> I think these conversions are not a good idea.
>
> When you have a specific message format that must be
> followed, use printk.
>
> pr_<level> may at some point in the near future use
> #define pr_fmt(fmt) KBUiLD_MODNAME ": " fmt
> as a global default equivalent.
Hey Jeff.
The comment above this change (and the other) reads
/* Link status message must follow this format for user tools */
This file already uses #define pr_fmt(fmt) KBUILD_MODNAME...
With this patch, the output form changes to use 2 prefixes.
Is that really desired? Probably not.
If the comments are old and don't apply any more, they
should be removed.
^ permalink raw reply
* Re: [RFC PATCH bridge 4/5] bridge: Add private ioctls to configure vlans on bridge ports
From: Paul E. McKenney @ 2012-08-24 17:56 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: netdev
In-Reply-To: <1345750195-31598-5-git-send-email-vyasevic@redhat.com>
On Thu, Aug 23, 2012 at 03:29:54PM -0400, Vlad Yasevich wrote:
> Add a private ioctl to add and remove vlan configuration on bridge port.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
One question below...
> ---
> include/linux/if_bridge.h | 2 +
> net/bridge/br_if.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
> net/bridge/br_ioctl.c | 31 ++++++++++++++++++++
> net/bridge/br_private.h | 2 +
> 4 files changed, 104 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index 288ff10..ab750dd 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -42,6 +42,8 @@
> #define BRCTL_SET_PORT_PRIORITY 16
> #define BRCTL_SET_PATH_COST 17
> #define BRCTL_GET_FDB_ENTRIES 18
> +#define BRCTL_ADD_VLAN 19
> +#define BRCTL_DEL_VLAN 20
>
> #define BR_STATE_DISABLED 0
> #define BR_STATE_LISTENING 1
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index e1144e1..90c1038 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -23,6 +23,7 @@
> #include <linux/if_ether.h>
> #include <linux/slab.h>
> #include <net/sock.h>
> +#include <linux/if_vlan.h>
>
> #include "br_private.h"
>
> @@ -441,6 +442,74 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
> return 0;
> }
>
> +/* Called with RTNL */
> +int br_set_port_vlan(struct net_bridge_port *p, unsigned long vlan)
> +{
> + unsigned long table_size = (VLAN_N_VID/sizeof(unsigned long)) + 1;
> + unsigned long *vid_map = NULL;
> + __u16 vid = (__u16) vlan + 1;
> +
> + /* We are under lock so we can check this without rcu.
> + * The vlan map is indexed by vid+1. This way we can store
> + * vid 0 (untagged) into the map as well.
> + */
> + if (!p->vlan_map) {
> + vid_map = kzalloc(table_size, GFP_KERNEL);
> + if (!vid_map)
> + return -ENOMEM;
> +
> + set_bit(vid, vid_map);
> + rcu_assign_pointer(p->vlan_map, vid_map);
> +
> + } else {
> + /* Map is already allocated */
> + set_bit(vid, p->vlan_map);
> + }
> +
> + return 0;
> +}
> +
> +
> +/* Called with RTNL */
> +int br_del_port_vlan(struct net_bridge_port *p, unsigned long vlan)
> +{
> + unsigned long first_bit;
> + unsigned long next_bit;
> + __u16 vid = (__u16) vlan+1;
> + unsigned long tbl_len = VLAN_N_VID+1;
> +
> + if (!p->vlan_map)
> + return -EINVAL;
> +
> + if (!test_bit(vlan, p->vlan_map))
> + return -EINVAL;
> +
> + /* Check to see if any other vlans are in this table. If this
> + * is the last vlan, delete the whole table. If this is not the
> + * last vlan, just clear the bit.
> + */
> + first_bit = find_first_bit(p->vlan_map, tbl_len);
> + next_bit = find_next_bit(p->vlan_map, tbl_len, (tbl_len - vid));
> +
> + if ((__u16)first_bit != vid || (__u16)next_bit < tbl_len) {
> + /* There are other vlans still configured. We can simply
> + * clear our bit and be safe.
> + */
> + clear_bit(vid, p->vlan_map);
> + } else {
> + /* This is the last vlan we are removing. Replace the
> + * map with a NULL pointer and free the old map
> + */
> + unsigned long *map = rcu_dereference(p->vlan_map);
> +
> + rcu_assign_pointer(p->vlan_map, NULL);
> + synchronize_net();
> + kfree(map);
> + }
> +
> + return 0;
> +}
> +
> void __net_exit br_net_exit(struct net *net)
> {
> struct net_device *dev;
> diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
> index 7222fe1..3a5b1f9 100644
> --- a/net/bridge/br_ioctl.c
> +++ b/net/bridge/br_ioctl.c
> @@ -289,6 +289,37 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> case BRCTL_GET_FDB_ENTRIES:
> return get_fdb_entries(br, (void __user *)args[1],
> args[2], args[3]);
> + case BRCTL_ADD_VLAN:
> + {
> + struct net_bridge_port *p;
> +
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
> +
> + rcu_read_lock();
> + if ((p = br_get_port(br, args[1])) == NULL) {
> + rcu_read_unlock();
> + return -EINVAL;
> + }
> + rcu_read_unlock();
Why is it safe to pass "p" out of the RCU read-side critical section?
I don't see that br_get_port() does anything to make this safe, at least
not in v3.5.
> + return br_set_port_vlan(p, args[2]);
> + }
> +
> + case BRCTL_DEL_VLAN:
> + {
> + struct net_bridge_port *p;
> +
> + if (!capable(CAP_NET_ADMIN))
> + return -EPERM;
> +
> + rcu_read_lock();
> + if ((p = br_get_port(br, args[1])) == NULL) {
> + rcu_read_unlock();
> + return -EINVAL;
> + }
> + rcu_read_unlock();
> + br_set_port_vlan(p, args[2]);
> + }
> }
>
> return -EOPNOTSUPP;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index b6c56ab..5639c1c 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -402,6 +402,8 @@ extern int br_del_if(struct net_bridge *br,
> extern int br_min_mtu(const struct net_bridge *br);
> extern netdev_features_t br_features_recompute(struct net_bridge *br,
> netdev_features_t features);
> +extern int br_set_port_vlan(struct net_bridge_port *p, unsigned long vid);
> +extern int br_del_port_vlan(struct net_bridge_port *p, unsigned long vid);
>
> /* br_input.c */
> extern int br_handle_frame_finish(struct sk_buff *skb);
> --
> 1.7.7.6
>
> --
> 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: [net-next 11/13] igb: Update PTP function names/variables and locations.
From: Joe Perches @ 2012-08-24 17:56 UTC (permalink / raw)
To: David Miller
Cc: richardcochran, matthew.vick, jeffrey.t.kirsher, netdev, gospo,
sassmann
In-Reply-To: <20120824.111238.1400056420671100883.davem@davemloft.net>
On Fri, 2012-08-24 at 11:12 -0400, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Fri, 24 Aug 2012 02:22:34 -0700
> > On Fri, 2012-08-24 at 08:32 +0200, Richard Cochran wrote:
> >> On Thu, Aug 23, 2012 at 08:02:25PM -0700, Joe Perches wrote:
> >> > Improving code clarity and consistency isn't churn.
> >> This patch series moves code around for no good reason. That is, by
> >> definition, churn.
> >
> > For your definition of good.
>
> I think the people doing all of the actual development and
> maintainence of the code get to decide how to define good. And in
> this case that's basically the Intel NIC development team, not you.
Just for clarity, you do mean Richard's view and not mine.
I agree with these proposed changes.
> Moving functions around is a very valuable and useful cleanup quite
> often.
>
> So you can count me in on their definition of "good" as well.
^ permalink raw reply
* Re: [PATCH] net-tcp: fix cwnd reduction for non-sack recovery
From: David Miller @ 2012-08-24 17:49 UTC (permalink / raw)
To: ncardwell; +Cc: nanditad, ycheng, mattmathis, netdev
In-Reply-To: <CADVnQykjK=7cL9pu1EqU+HyiVX_ujSbJ-T1Ohb+M41w4rm1wuA@mail.gmail.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Thu, 23 Aug 2012 13:25:50 -0400
> On Thu, Aug 23, 2012 at 1:21 PM, Nandita Dukkipati <nanditad@google.com> wrote:
>> On Thu, Aug 23, 2012 at 10:05 AM, Yuchung Cheng <ycheng@google.com> wrote:
>>> The cwnd reduction in fast recovery is based on the number of packets
>>> newly delivered per ACK. For non-sack connections every DUPACK
>>> signifies a packet has been delivered, but the sender mistakenly
>>> skips counting them for cwnd reduction.
>>>
>>> The fix is to compute newly_acked_sacked after DUPACKs are accounted
>>> in sacked_out for non-sack connections.
>>>
>>> Signed-off-by: Yuchung Cheng <ycheng@google.com>
...
>> Acked-by: Nandita Dukkipati <nanditad@google.com>
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
Applied, and queued up for -stable, thanks.
Please use plain "tcp: " as the prefix for pure tcp changes. Thanks.
^ permalink raw reply
* pull request: wireless-next 2012-08-24
From: John W. Linville @ 2012-08-24 17:31 UTC (permalink / raw)
To: davem; +Cc: linux-wireless, netdev
[-- Attachment #1: Type: text/plain, Size: 12046 bytes --]
commit f20b6213f193f455a62ef9299ceca11f5531dff8
Dave,
This is a batch of updates intended for 3.7. The bulk of it is
mac80211 changes, including some mesh work from Thomas Pederson and
some multi-channel work from Johannes. A variety of driver updates
and other bits are scattered in there as well.
Please let me know if there are problems!
Thanks,
John
---
The following changes since commit e6e94e392fddb8845bdd2e199f55ebf7d76cb58d:
Merge tag 'batman-adv-for-davem' of git://git.open-mesh.org/linux-merge (2012-08-24 11:30:50 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next.git for-davem
for you to fetch changes up to f20b6213f193f455a62ef9299ceca11f5531dff8:
Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next into for-davem (2012-08-24 12:25:30 -0400)
----------------------------------------------------------------
AceLan Kao (1):
Revert "rfkill: remove dead code"
Amit Beka (1):
iwlwifi: fix FW restart on init FW
Antonio Quartulli (2):
mac80211: add supported rates change notification in IBSS
ath9k_htc: implement sta_rc_update() mac80211 callback
Arik Nemtsov (2):
wlcore: don't get the hlid from a queued skb
wlcore: op_tx: pass sta explicitly when inferring frame hlid
Christian Lamparter (1):
carl9170: report A-MPDU status
Chun-Yeow Yeoh (1):
mac80211: don't re-init rate control when receiving mesh beacon
Eliad Peller (1):
mac80211: add PS flag to bss_conf
Emmanuel Grumbach (4):
iwlwifi: s/iwl_ucode_callback/iwl_req_fw_callback
iwlwifi: get the correct HCMD in the response handler
mac80211: don't call mgd_prepare_tx when associated
iwlwifi: fix the preparation of the card
Eyal Shapira (1):
mac80211: avoid using synchronize_rcu in ieee80211_set_probe_resp
Johannes Berg (38):
iwlwifi: fix aggregation check indentation
iwlwifi: reduce max remain-on-channel duration
mac80211: don't react to beacon loss if HW monitoring
mac80211: supress HT/VHT disable if not supported
mac80211: rename sta to new_sta
mac80211: set channel only once during auth/assoc
mac80211: use oper_channel in mesh
mac80211: use oper_channel in ibss
mac80211: use oper_channel in managed mlme
mac80211: use oper_channel in utils and config
mac80211: fix current vs. operating channel in preq/beacon
mac80211: use correct channel in TX
mac80211: manage carrier state in mesh
mac80211: enable WDS carrier only after adding station
mac80211: use eth_broadcast_addr
mac80211: reset station MLME flags upon new association
wireless: add radiotap A-MPDU status field
mac80211: support A-MPDU status reporting
mac80211: remove unneeded 'bssid' variable
mac80211: clean up CSA handling code
mac80211: fix CSA handling timer
mac80211: check size of channel switch IE when parsing
mac80211: make ieee80211_beacon_connection_loss_work static
cfg80211: add P2P Device abstraction
mac80211: support P2P Device abstraction
mac80211: add IEEE80211_HW_P2P_DEV_ADDR_FOR_INTF
mac80211_hwsim: add support for P2P Device
mac80211: simplify buffers in aes_128_cmac_vector
mac80211: mesh: don't use global channel type
mac80211: remove almost unused local variable
mac80211: remove freq/chantype from debugfs
mac80211: use oper_channel in rate init
mac80211: don't assume channel is set in tracing
mac80211: use RX status band instead of current band
mac80211: check operating channel in scan
mac80211: convert ops checks to WARN_ON
mac80211: pass channel to ieee80211_send_probe_req
mac80211: clean up ieee80211_subif_start_xmit
John W. Linville (4):
Merge branch 'for-john' of git://git.kernel.org/.../jberg/mac80211-next
Merge branch 'for-john' of git://git.kernel.org/.../iwlwifi/iwlwifi-next
Merge branch 'for-john' of git://git.kernel.org/.../jberg/mac80211-next
Merge branch 'master' of git://git.kernel.org/.../linville/wireless-next into for-davem
Mahesh Palivela (1):
mac80211: VHT (11ac) association
Meenakshi Venkataraman (2):
iwlwifi: clean up properly when registration with mac80211 fails
iwlwifi: rework the iwlwifi debugfs structure
Rafał Miłecki (2):
bcma: detect and register serial flash device
bcma: detect and register NAND flash device
Sujith Manoharan (1):
mac80211: Fix AP mode regression
Thomas Huehn (1):
mac80211: move TX station pointer and restructure TX
Thomas Pedersen (6):
mac80211: improve cleanup when leaving mesh
mac80211: remove ieee80211_clean_sdata()
mac80211: don't request ack for peering close
mac80211: skb leak in mesh_plink_frame_tx()
mac80211: use skb_queue_walk() in mesh_path_assign_nexthop
mac80211: clean up mpath_move_to_queue()
Vladimir Kondratiev (1):
cfg80211: unify IE search
drivers/bcma/Kconfig | 4 +-
drivers/bcma/bcma_private.h | 2 +
drivers/bcma/driver_chipcommon_nflash.c | 28 +-
drivers/bcma/driver_chipcommon_sflash.c | 123 ++++++++-
drivers/bcma/main.c | 17 ++
drivers/net/wireless/adm8211.c | 4 +-
drivers/net/wireless/at76c50x-usb.c | 4 +-
drivers/net/wireless/ath/ath5k/mac80211-ops.c | 3 +-
drivers/net/wireless/ath/ath9k/ath9k.h | 1 +
drivers/net/wireless/ath/ath9k/htc.h | 1 +
drivers/net/wireless/ath/ath9k/htc_drv_beacon.c | 2 +-
drivers/net/wireless/ath/ath9k/htc_drv_main.c | 35 ++-
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c | 2 +-
drivers/net/wireless/ath/ath9k/main.c | 5 +-
drivers/net/wireless/ath/ath9k/xmit.c | 9 +-
drivers/net/wireless/ath/carl9170/carl9170.h | 5 +-
drivers/net/wireless/ath/carl9170/rx.c | 15 +-
drivers/net/wireless/ath/carl9170/tx.c | 16 +-
drivers/net/wireless/b43/main.c | 3 +-
drivers/net/wireless/b43legacy/main.c | 1 +
.../net/wireless/brcm80211/brcmsmac/mac80211_if.c | 6 +-
drivers/net/wireless/iwlegacy/3945-mac.c | 12 +-
drivers/net/wireless/iwlegacy/4965-mac.c | 26 +-
drivers/net/wireless/iwlegacy/4965.h | 8 +-
drivers/net/wireless/iwlwifi/dvm/agn.h | 13 +-
drivers/net/wireless/iwlwifi/dvm/debugfs.c | 56 ++--
drivers/net/wireless/iwlwifi/dvm/mac80211.c | 8 +-
drivers/net/wireless/iwlwifi/dvm/main.c | 24 +-
drivers/net/wireless/iwlwifi/dvm/sta.c | 2 +-
drivers/net/wireless/iwlwifi/dvm/tx.c | 18 +-
drivers/net/wireless/iwlwifi/iwl-drv.c | 141 ++++++++--
drivers/net/wireless/iwlwifi/iwl-drv.h | 6 +-
drivers/net/wireless/iwlwifi/iwl-op-mode.h | 3 +-
drivers/net/wireless/iwlwifi/iwl-trans.h | 12 +-
drivers/net/wireless/iwlwifi/pcie/drv.c | 6 +
drivers/net/wireless/iwlwifi/pcie/internal.h | 1 +
drivers/net/wireless/iwlwifi/pcie/rx.c | 16 +-
drivers/net/wireless/iwlwifi/pcie/trans.c | 28 +-
drivers/net/wireless/iwlwifi/pcie/tx.c | 26 +-
drivers/net/wireless/libertas_tf/main.c | 4 +-
drivers/net/wireless/mac80211_hwsim.c | 8 +-
drivers/net/wireless/mwl8k.c | 17 +-
drivers/net/wireless/p54/lmac.h | 4 +-
drivers/net/wireless/p54/main.c | 2 +-
drivers/net/wireless/p54/txrx.c | 15 +-
drivers/net/wireless/rt2x00/rt2x00.h | 4 +-
drivers/net/wireless/rt2x00/rt2x00dev.c | 2 +-
drivers/net/wireless/rt2x00/rt2x00mac.c | 4 +-
drivers/net/wireless/rt2x00/rt2x00queue.c | 20 +-
drivers/net/wireless/rtl818x/rtl8180/dev.c | 6 +-
drivers/net/wireless/rtl818x/rtl8187/dev.c | 6 +-
drivers/net/wireless/rtlwifi/base.c | 3 +-
drivers/net/wireless/rtlwifi/core.c | 8 +-
drivers/net/wireless/rtlwifi/pci.c | 16 +-
drivers/net/wireless/rtlwifi/rtl8192ce/trx.c | 5 +-
drivers/net/wireless/rtlwifi/rtl8192ce/trx.h | 1 +
drivers/net/wireless/rtlwifi/rtl8192cu/trx.c | 5 +-
drivers/net/wireless/rtlwifi/rtl8192cu/trx.h | 4 +-
drivers/net/wireless/rtlwifi/rtl8192de/trx.c | 5 +-
drivers/net/wireless/rtlwifi/rtl8192de/trx.h | 1 +
drivers/net/wireless/rtlwifi/rtl8192se/trx.c | 5 +-
drivers/net/wireless/rtlwifi/rtl8192se/trx.h | 1 +
drivers/net/wireless/rtlwifi/usb.c | 15 +-
drivers/net/wireless/rtlwifi/wifi.h | 13 +-
drivers/net/wireless/ti/wl1251/main.c | 4 +-
drivers/net/wireless/ti/wlcore/main.c | 6 +-
drivers/net/wireless/ti/wlcore/tx.c | 61 ++---
drivers/net/wireless/ti/wlcore/tx.h | 4 +-
drivers/net/wireless/zd1211rw/zd_mac.c | 6 +-
drivers/staging/winbond/wbusb.c | 4 +-
include/linux/bcma/bcma_driver_chipcommon.h | 26 ++
include/linux/bcma/bcma_regs.h | 2 +
include/linux/nl80211.h | 30 ++-
include/linux/rfkill.h | 31 +++
include/net/cfg80211.h | 40 ++-
include/net/ieee80211_radiotap.h | 11 +
include/net/mac80211.h | 87 +++++--
net/mac80211/aes_cmac.c | 6 +-
net/mac80211/cfg.c | 66 +++--
net/mac80211/debugfs.c | 32 ---
net/mac80211/driver-ops.h | 11 +-
net/mac80211/ibss.c | 15 +-
net/mac80211/ieee80211_i.h | 30 ++-
net/mac80211/iface.c | 289 +++++++++++++--------
net/mac80211/main.c | 21 +-
net/mac80211/mesh.c | 28 +-
net/mac80211/mesh.h | 3 +
net/mac80211/mesh_hwmp.c | 2 -
net/mac80211/mesh_pathtbl.c | 44 ++--
net/mac80211/mesh_plink.c | 38 ++-
net/mac80211/mlme.c | 240 +++++++++++------
net/mac80211/offchannel.c | 6 +
net/mac80211/rate.h | 2 +-
net/mac80211/rx.c | 58 ++++-
net/mac80211/scan.c | 12 +-
net/mac80211/status.c | 22 +-
net/mac80211/trace.h | 11 +-
net/mac80211/tx.c | 71 +++--
net/mac80211/util.c | 57 ++--
net/rfkill/core.c | 14 +
net/wireless/chan.c | 7 +-
net/wireless/core.c | 53 +++-
net/wireless/mlme.c | 10 +-
net/wireless/nl80211.c | 122 ++++++++-
net/wireless/radiotap.c | 2 +
net/wireless/util.c | 36 +--
106 files changed, 1765 insertions(+), 721 deletions(-)
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [patch net-next 0/3] team: couple of fixes
From: David Miller @ 2012-08-24 17:47 UTC (permalink / raw)
To: jiri; +Cc: netdev, brouer, ogerlitz
In-Reply-To: <1345728413-10298-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Thu, 23 Aug 2012 15:26:50 +0200
> + vlan helper
>
> Jiri Pirko (3):
> teamd: don't print warn message on -ESRCH during event send
> vlan: add helper which can be called to se it device is used by vlan
> team: do not allow to add VLAN challenged port when vlan is used
Applied with typos corrected :-)
^ permalink raw reply
* Re: [PATCH 1/1] tcp: Wrong timeout for SYN segments
From: David Miller @ 2012-08-24 17:42 UTC (permalink / raw)
To: eric.dumazet; +Cc: alex, hkjerry.chu, netdev, linux-kernel
In-Reply-To: <1345726185.5904.835.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 23 Aug 2012 14:49:45 +0200
> On Thu, 2012-08-23 at 14:37 +0200, Alex Bergmann wrote:
>
>>
>> From be551f82499112e4775b6d579d58967510b6d492 Mon Sep 17 00:00:00 2001
>> From: Alexander Bergmann <alex@linlab.net>
>> Date: Thu, 23 Aug 2012 14:33:35 +0200
>> Subject: [PATCH 1/1] tcp: Increase timeout for SYN segments
>>
>> Commit 9ad7c049 changed the initRTO from 3secs to 1sec in accordance to
>> RFC6298 (former RFC2988bis). This reduced the time till the last SYN
>> retransmission packet gets sent from 93secs to 31secs.
>>
>> RFC1122 is stating that the retransmission should be done for at least 3
>> minutes, but this seems to be quite high.[1]
>>
>> This patch increases the value of TCP_SYN_RETRIES to the value of 6,
>> providing a retransmission window of 63secs.
>>
>> [1] RFC 1122 - 4.2.3.5 TCP Connection Failures
>>
>> Signed-off-by: Alexander Bergmann <alex@linlab.net>
>> ---
>> include/net/tcp.h | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>> index 1f000ff..f309e93 100644
>> --- a/include/net/tcp.h
>> +++ b/include/net/tcp.h
>> @@ -98,7 +98,7 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
>> * 15 is ~13-30min depending on RTO.
>> */
>>
>> -#define TCP_SYN_RETRIES 5 /* number of times to retry active opening a
>> +#define TCP_SYN_RETRIES 6 /* number of times to retry active opening a
>> * connection: ~180sec is RFC minimum */
>>
>> #define TCP_SYNACK_RETRIES 5 /* number of times to retry passive opening a
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
> A change of the comment might be good, to help future readers.
Alex, this patch doesn't apply, it was completely corrupted by your email
client.
Make a fresh submission, with this fixed. But before you do, email the
patch to yourself and make sure you can actually apply the patch you
receive in your inbox. Because that's exactly what I'm going to have
to do.
Thanks.
^ permalink raw reply
* Re: [PATCH 5/8] csiostor: Chelsio FCoE offload driver submission (sources part 5).
From: Naresh Kumar Inna @ 2012-08-24 17:40 UTC (permalink / raw)
To: Nicholas A. Bellinger
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <1345751301.10190.75.camel@haakon2.linux-iscsi.org>
On 8/24/2012 1:18 AM, Nicholas A. Bellinger wrote:
> On Fri, 2012-08-24 at 03:57 +0530, Naresh Kumar Inna wrote:
>> This patch contains code to implement the interrupt handling and the fast
>> path I/O functionality. The interrupt handling includes allocation of
>> MSIX vectors, registering and implemeting the interrupt service routines.
>> The fast path I/O functionality includes posting the I/O request to firmware
>> via Work Requests, tracking/completing them, and handling task management
>> requests. SCSI midlayer host template implementation is also covered by
>> this patch.
>>
>> Signed-off-by: Naresh Kumar Inna <naresh@chelsio.com>
>> ---
>
> Hi Naresh,
>
> My review comments are inline below..
Hi Nicholas,
Thanks for taking the time to review the driver. Please find my replies
inline.
Regards,
Naresh.
>
>> drivers/scsi/csiostor/csio_isr.c | 631 ++++++++++
>> drivers/scsi/csiostor/csio_scsi.c | 2498 +++++++++++++++++++++++++++++++++++++
>> 2 files changed, 3129 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/scsi/csiostor/csio_isr.c
>> create mode 100644 drivers/scsi/csiostor/csio_scsi.c
>>
>> diff --git a/drivers/scsi/csiostor/csio_isr.c b/drivers/scsi/csiostor/csio_isr.c
>> new file mode 100644
>> index 0000000..96633e9
>> --- /dev/null
>> +++ b/drivers/scsi/csiostor/csio_isr.c
>
> <SNIP>
>
>> +#define csio_extra_msix_desc(_desc, _len, _str, _arg1, _arg2, _arg3) \
>> +do { \
>> + memset((_desc), 0, (_len) + 1); \
>> + snprintf((_desc), (_len), (_str), (_arg1), (_arg2), (_arg3)); \
>> +} while (0)
>> +
>
> This type of macro usage is not necessary for just two users below.
> Please inline code like this.
>
OK, I will get rid of this macro.
>> +static void
>> +csio_add_msix_desc(struct csio_hw *hw)
>> +{
>> + int i;
>> + struct csio_msix_entries *entryp = &hw->msix_entries[0];
>> + int k = CSIO_EXTRA_VECS;
>> + int len = sizeof(entryp->desc) - 1;
>> + int cnt = hw->num_sqsets + k;
>> +
>> + /* Non-data vector */
>> + csio_extra_msix_desc(entryp->desc, len, "csio-%02x:%02x:%x-nondata",
>> + CSIO_PCI_BUS(hw), CSIO_PCI_DEV(hw),
>> + CSIO_PCI_FUNC(hw));
>> + entryp++;
>> + csio_extra_msix_desc(entryp->desc, len, "csio-%02x:%02x:%x-fwevt",
>> + CSIO_PCI_BUS(hw), CSIO_PCI_DEV(hw),
>> + CSIO_PCI_FUNC(hw));
>> + entryp++;
>> +
>> + /* Name SCSI vecs */
>> + for (i = k; i < cnt; i++, entryp++) {
>> + memset(entryp->desc, 0, len + 1);
>> + snprintf(entryp->desc, len, "csio-%02x:%02x:%x-scsi%d",
>> + CSIO_PCI_BUS(hw), CSIO_PCI_DEV(hw),
>> + CSIO_PCI_FUNC(hw), i - CSIO_EXTRA_VECS);
>> + }
>> +}
>> +
>
>
>> diff --git a/drivers/scsi/csiostor/csio_scsi.c b/drivers/scsi/csiostor/csio_scsi.c
>> new file mode 100644
>> index 0000000..0f87b00
>> --- /dev/null
>> +++ b/drivers/scsi/csiostor/csio_scsi.c
>
>> +
>> +/*
>> + * csio_scsi_match_io - Match an ioreq with the given SCSI level data.
>> + * @ioreq: The I/O request
>> + * @sld: Level information
>> + *
>> + * Should be called with lock held.
>> + *
>> + */
>> +static bool
>> +csio_scsi_match_io(struct csio_ioreq *ioreq, struct csio_scsi_level_data *sld)
>> +{
>> + struct scsi_cmnd *scmnd = csio_scsi_cmnd(ioreq);
>> +
>> + switch (sld->level) {
>> + case CSIO_LEV_LUN:
>> + if (scmnd == NULL)
>> + return CSIO_FALSE;
>> +
>> + return ((ioreq->lnode == sld->lnode) &&
>> + (ioreq->rnode == sld->rnode) &&
>> + ((uint64_t)scmnd->device->lun == sld->oslun));
>> +
>> + case CSIO_LEV_RNODE:
>> + return ((ioreq->lnode == sld->lnode) &&
>> + (ioreq->rnode == sld->rnode));
>> + case CSIO_LEV_LNODE:
>> + return (ioreq->lnode == sld->lnode);
>> + case CSIO_LEV_ALL:
>> + return CSIO_TRUE;
>> + default:
>> + return CSIO_FALSE;
>> + }
>> +}
>> +
>
> Why can't CSIO_[TRUE,FALSE] just use normal Boolean defines..?
>
Is there a TRUE/FALSE define in a standard header file? I see a lot of
kernel code defining their own TRUE/FALSE.
>> +/*
>> + * csio_scsi_fcp_cmnd - Frame the SCSI FCP command paylod.
>> + * @req: IO req structure.
>> + * @addr: DMA location to place the payload.
>> + *
>> + * This routine is shared between FCP_WRITE, FCP_READ and FCP_CMD requests.
>> + */
>> +static inline void
>> +csio_scsi_fcp_cmnd(struct csio_ioreq *req, void *addr)
>> +{
>> + struct csio_fcp_cmnd *fcp_cmnd = (struct csio_fcp_cmnd *)addr;
>> + struct scsi_cmnd *scmnd = csio_scsi_cmnd(req);
>> +
>> + /* Check for Task Management */
>> + if (likely(scmnd->SCp.Message == 0)) {
>> + int_to_scsilun(scmnd->device->lun,
>> + (struct scsi_lun *)fcp_cmnd->lun);
>> + fcp_cmnd->tm_flags = 0;
>> + fcp_cmnd->cmdref = 0;
>> + fcp_cmnd->pri_ta = 0;
>> +
>> + memcpy(fcp_cmnd->cdb, scmnd->cmnd, 16);
>> + csio_scsi_tag(scmnd, &fcp_cmnd->pri_ta,
>> + FCP_PTA_HEADQ, FCP_PTA_ORDERED, FCP_PTA_SIMPLE);
>> + fcp_cmnd->dl = cpu_to_be32(scsi_bufflen(scmnd));
>> +
>> + if (req->nsge)
>> + if (req->datadir == CSIO_IOREQF_DMA_WRITE)
>
> The same goes for CSIO_IOREQF_DMA_*...
>
> Why can't this just be DMA_* defs from include/linux/dma-direction.h..?
OK, I will get rid of them and use the standard defines.
>
>> + fcp_cmnd->flags = FCP_CFL_WRDATA;
>> + else
>> + fcp_cmnd->flags = FCP_CFL_RDDATA;
>> + else
>> + fcp_cmnd->flags = 0;
>> + } else {
>> + memset(fcp_cmnd, 0, sizeof(*fcp_cmnd));
>> + int_to_scsilun(scmnd->device->lun,
>> + (struct scsi_lun *)fcp_cmnd->lun);
>> + fcp_cmnd->tm_flags = (uint8_t)scmnd->SCp.Message;
>> + }
>> +}
>> +
>
>
>
>> +
>> +#define CSIO_SCSI_CMD_WR_SZ(_imm) \
>> + (sizeof(struct fw_scsi_cmd_wr) + /* WR size */ \
>> + ALIGN((_imm), 16)) /* Immed data */
>> +
>> +#define CSIO_SCSI_CMD_WR_SZ_16(_imm) \
>> + (ALIGN(CSIO_SCSI_CMD_WR_SZ((_imm)), 16))
>> +
>> +/*
>> + * csio_scsi_cmd - Create a SCSI CMD WR.
>> + * @req: IO req structure.
>> + *
>> + * Gets a WR slot in the ingress queue and initializes it with SCSI CMD WR.
>> + *
>> + */
>> +static inline void
>> +csio_scsi_cmd(struct csio_ioreq *req)
>> +{
>> + struct csio_wr_pair wrp;
>> + struct csio_hw *hw = req->lnode->hwp;
>> + struct csio_scsim *scsim = csio_hw_to_scsim(hw);
>> + uint32_t size = CSIO_SCSI_CMD_WR_SZ_16(scsim->proto_cmd_len);
>> +
>> + req->drv_status = csio_wr_get(hw, req->eq_idx, size, &wrp);
>> + if (unlikely(req->drv_status != CSIO_SUCCESS))
>> + return;
>> +
>> + if (wrp.size1 >= size) {
>> + /* Initialize WR in one shot */
>> + csio_scsi_init_cmd_wr(req, wrp.addr1, size);
>> + } else {
>> + uint8_t tmpwr[512];
>
> Mmmm, putting this large of a buffer on the local stack is probably not
> a good idea.
>
> This should become an allocation.. If it's a hot path then you'll
> probably want to set this up before-hand.
>
The else switch above is entered only when we near the end of the DMA
ring. This is not a frequent occurrence. If it is a strict no-no to have
so many on-stack bytes, I have to think of a way re-work it to use
pre-allocated memory. Please let me know.
>> + /*
>> + * Make a temporary copy of the WR and write back
>> + * the copy into the WR pair.
>> + */
>> + csio_scsi_init_cmd_wr(req, (void *)tmpwr, size);
>> + memcpy(wrp.addr1, tmpwr, wrp.size1);
>> + memcpy(wrp.addr2, tmpwr + wrp.size1, size - wrp.size1);
>> + }
>> +}
>> +
>> +/*
>> + * The following is fast path code. Therefore it is inlined with multi-line
>> + * macros using name substitution, thus avoiding if-else switches for
>> + * operation (read/write), as well as serving the purpose of code re-use.
>> + */
>> +/*
>> + * csio_scsi_init_ulptx_dsgl - Fill in a ULP_TX_SC_DSGL
>> + * @hw: HW module
>> + * @req: IO request
>> + * @sgl: ULP TX SGL pointer.
>> + *
>> + */
>> +#define csio_scsi_init_ultptx_dsgl(hw, req, sgl) \
>> +do { \
>> + struct ulptx_sge_pair *_sge_pair = NULL; \
>> + struct scatterlist *_sgel; \
>> + uint32_t _i = 0; \
>> + uint32_t _xfer_len; \
>> + struct list_head *_tmp; \
>> + struct csio_dma_buf *_dma_buf; \
>> + struct scsi_cmnd *scmnd = csio_scsi_cmnd((req)); \
>> + \
>> + (sgl)->cmd_nsge = htonl(ULPTX_CMD(ULP_TX_SC_DSGL) | ULPTX_MORE | \
>> + ULPTX_NSGE((req)->nsge)); \
>> + /* Now add the data SGLs */ \
>> + if (likely(!(req)->dcopy)) { \
>> + scsi_for_each_sg(scmnd, _sgel, (req)->nsge, _i) { \
>> + if (_i == 0) { \
>> + (sgl)->addr0 = cpu_to_be64( \
>> + sg_dma_address(_sgel)); \
>> + (sgl)->len0 = cpu_to_be32( \
>> + sg_dma_len(_sgel)); \
>> + _sge_pair = \
>> + (struct ulptx_sge_pair *)((sgl) + 1); \
>> + continue; \
>> + } \
>> + if ((_i - 1) & 0x1) { \
>> + _sge_pair->addr[1] = cpu_to_be64( \
>> + sg_dma_address(_sgel)); \
>> + _sge_pair->len[1] = cpu_to_be32( \
>> + sg_dma_len(_sgel)); \
>> + _sge_pair++; \
>> + } else { \
>> + _sge_pair->addr[0] = cpu_to_be64( \
>> + sg_dma_address(_sgel)); \
>> + _sge_pair->len[0] = cpu_to_be32( \
>> + sg_dma_len(_sgel)); \
>> + } \
>> + } \
>> + } else { \
>> + /* Program sg elements with driver's DDP buffer */ \
>> + _xfer_len = scsi_bufflen(scmnd); \
>> + list_for_each(_tmp, &(req)->gen_list) { \
>> + _dma_buf = (struct csio_dma_buf *)_tmp; \
>> + if (_i == 0) { \
>> + (sgl)->addr0 = cpu_to_be64(_dma_buf->paddr); \
>> + (sgl)->len0 = cpu_to_be32( \
>> + min(_xfer_len, _dma_buf->len)); \
>> + _sge_pair = \
>> + (struct ulptx_sge_pair *)((sgl) + 1); \
>> + } \
>> + else if ((_i - 1) & 0x1) { \
>> + _sge_pair->addr[1] = cpu_to_be64( \
>> + _dma_buf->paddr); \
>> + _sge_pair->len[1] = cpu_to_be32( \
>> + min(_xfer_len, _dma_buf->len)); \
>> + _sge_pair++; \
>> + } else { \
>> + _sge_pair->addr[0] = cpu_to_be64( \
>> + _dma_buf->paddr); \
>> + _sge_pair->len[0] = cpu_to_be32( \
>> + min(_xfer_len, _dma_buf->len)); \
>> + } \
>> + _xfer_len -= min(_xfer_len, _dma_buf->len); \
>> + _i++; \
>> + } \
>> + } \
>> +} while (0)
>> +
>
> I don't see any reason why this can't just be a static function..? Why
> is the macro usage necessary here..?
OK, I will make this static-inline.
>
>> +/*
>> + * csio_scsi_init_data_wr - Initialize the READ/WRITE SCSI WR.
>> + * @req: IO req structure.
>> + * @oper: read/write
>> + * @wrp: DMA location to place the payload.
>> + * @size: Size of WR (including FW WR + immed data + rsp SG entry + data SGL
>> + * @wrop: _READ_/_WRITE_
>> + *
>> + * Wrapper for populating fw_scsi_read_wr/fw_scsi_write_wr.
>> + */
>> +#define csio_scsi_init_data_wr(req, oper, wrp, size, wrop) \
>> +do { \
>> + struct csio_hw *_hw = (req)->lnode->hwp; \
>> + struct csio_rnode *_rn = (req)->rnode; \
>> + struct fw_scsi_##oper##_wr *__wr = (struct fw_scsi_##oper##_wr *)(wrp);\
>> + struct ulptx_sgl *_sgl; \
>> + struct csio_dma_buf *_dma_buf; \
>> + uint8_t _imm = csio_hw_to_scsim(_hw)->proto_cmd_len; \
>> + struct scsi_cmnd *scmnd = csio_scsi_cmnd((req)); \
>> + \
>> + __wr->op_immdlen = cpu_to_be32(FW_WR_OP(FW_SCSI##wrop##WR) | \
>> + FW_SCSI##wrop##WR_IMMDLEN(_imm)); \
>> + __wr->flowid_len16 = cpu_to_be32(FW_WR_FLOWID(_rn->flowid) | \
>> + FW_WR_LEN16( \
>> + CSIO_ROUNDUP((size), 16))); \
>> + __wr->cookie = (uintptr_t) (req); \
>> + __wr->iqid = (uint16_t)cpu_to_be16(csio_q_physiqid(_hw, \
>> + (req)->iq_idx));\
>> + __wr->tmo_val = (uint8_t)((req)->tmo); \
>> + __wr->use_xfer_cnt = 1; \
>> + __wr->xfer_cnt = cpu_to_be32(scsi_bufflen(scmnd)); \
>> + __wr->ini_xfer_cnt = cpu_to_be32(scsi_bufflen(scmnd)); \
>> + /* Get RSP DMA buffer */ \
>> + _dma_buf = &(req)->dma_buf; \
>> + \
>> + /* Prepare RSP SGL */ \
>> + __wr->rsp_dmalen = cpu_to_be32(_dma_buf->len); \
>> + __wr->rsp_dmaaddr = cpu_to_be64(_dma_buf->paddr); \
>> + \
>> + __wr->r4 = 0; \
>> + \
>> + __wr->u.fcoe.ctl_pri = 0; \
>> + __wr->u.fcoe.cp_en_class = 0; \
>> + __wr->u.fcoe.r3_lo[0] = 0; \
>> + __wr->u.fcoe.r3_lo[1] = 0; \
>> + csio_scsi_fcp_cmnd((req), (void *)((uintptr_t)(wrp) + \
>> + sizeof(struct fw_scsi_##oper##_wr))); \
>> + \
>> + /* Move WR pointer past command and immediate data */ \
>> + _sgl = (struct ulptx_sgl *) ((uintptr_t)(wrp) + \
>> + sizeof(struct fw_scsi_##oper##_wr) + \
>> + ALIGN(_imm, 16)); \
>> + \
>> + /* Fill in the DSGL */ \
>> + csio_scsi_init_ultptx_dsgl(_hw, (req), _sgl); \
>> + \
>> +} while (0)
>> +
>
> This one has four uses of CPP keys. Just turn those into macros, and
> leave the rest of the code in a static function.
>
So what you are suggesting is to have all the lines of the macro
csio_scsi_init_data_wr() added into a static function, but for the ones
with the 4 keys. csio_scsi_init_data_wr() will then invoke this new
function. Is that correct?
>> +/* Calculate WR size needed for fw_scsi_read_wr/fw_scsi_write_wr */
>> +#define csio_scsi_data_wrsz(req, oper, sz, imm) \
>> +do { \
>> + (sz) = sizeof(struct fw_scsi_##oper##_wr) + /* WR size */ \
>> + ALIGN((imm), 16) + /* Immed data */ \
>> + sizeof(struct ulptx_sgl); /* ulptx_sgl */ \
>> + \
>> + if (unlikely((req)->nsge > 1)) \
>> + (sz) += (sizeof(struct ulptx_sge_pair) * \
>> + (ALIGN(((req)->nsge - 1), 2) / 2)); \
>> + /* Data SGE */ \
>> +} while (0)
>> +
>> +/*
>> + * csio_scsi_data - Create a SCSI WRITE/READ WR.
>> + * @req: IO req structure.
>> + * @oper: read/write
>> + * @wrop: _READ_/_WRITE_ (string subsitutions to use with the FW bit field
>> + * macros).
>> + *
>> + * Gets a WR slot in the ingress queue and initializes it with
>> + * SCSI CMD READ/WRITE WR.
>> + *
>> + */
>> +#define csio_scsi_data(req, oper, wrop) \
>> +do { \
>> + struct csio_wr_pair _wrp; \
>> + uint32_t _size; \
>> + struct csio_hw *_hw = (req)->lnode->hwp; \
>> + struct csio_scsim *_scsim = csio_hw_to_scsim(_hw); \
>> + \
>> + csio_scsi_data_wrsz((req), oper, _size, _scsim->proto_cmd_len); \
>> + _size = ALIGN(_size, 16); \
>> + \
>> + (req)->drv_status = csio_wr_get(_hw, (req)->eq_idx, _size, &_wrp); \
>> + if (likely((req)->drv_status == CSIO_SUCCESS)) { \
>> + if (likely(_wrp.size1 >= _size)) { \
>> + /* Initialize WR in one shot */ \
>> + csio_scsi_init_data_wr((req), oper, _wrp.addr1, \
>> + _size, wrop); \
>> + } else { \
>> + uint8_t tmpwr[512]; \
>> + /* \
>> + * Make a temporary copy of the WR and write back \
>> + * the copy into the WR pair. \
>> + */ \
>> + csio_scsi_init_data_wr((req), oper, (void *)tmpwr, \
>> + _size, wrop); \
>> + memcpy(_wrp.addr1, tmpwr, _wrp.size1); \
>> + memcpy(_wrp.addr2, tmpwr + _wrp.size1, \
>> + _size - _wrp.size1); \
>> + } \
>> + } \
>> +} while (0)
>> +
>
> Ditto on this one, along with the tmpwr[512] stack usage..
>
>> +static inline void
>> +csio_scsi_abrt_cls(struct csio_ioreq *req, bool abort)
>> +{
>> + struct csio_wr_pair wrp;
>> + struct csio_hw *hw = req->lnode->hwp;
>> + uint32_t size = ALIGN(sizeof(struct fw_scsi_abrt_cls_wr), 16);
>> +
>> + req->drv_status = csio_wr_get(hw, req->eq_idx, size, &wrp);
>> + if (req->drv_status != CSIO_SUCCESS)
>> + return;
>> +
>> + if (wrp.size1 >= size) {
>> + /* Initialize WR in one shot */
>> + csio_scsi_init_abrt_cls_wr(req, wrp.addr1, size, abort);
>> + } else {
>> + uint8_t tmpwr[512];
>
> Ditto here on local scope stack usage..
>
>> + /*
>> + * Make a temporary copy of the WR and write back
>> + * the copy into the WR pair.
>> + */
>> + csio_scsi_init_abrt_cls_wr(req, (void *)tmpwr, size, abort);
>> + memcpy(wrp.addr1, tmpwr, wrp.size1);
>> + memcpy(wrp.addr2, tmpwr + wrp.size1, size - wrp.size1);
>> + }
>> +}
>> +
>> +/*****************************************************************************/
>> +/* START: SCSI SM */
>> +/*****************************************************************************/
>> +static void
>> +csio_scsis_uninit(struct csio_ioreq *req, enum csio_scsi_ev evt)
>> +{
>> + struct csio_hw *hw = req->lnode->hwp;
>> + struct csio_scsim *scsim = csio_hw_to_scsim(hw);
>> +
>> + switch (evt) {
>> +
>> + case CSIO_SCSIE_START_IO:
>
> Extra space between start of first switch case
OK, I will remove it. Is there any tool that catches such deviations?
checkpath.pl didnt report it.
>
>> +
>> + /* There is data */
>
> Point-less comment
I will remove it.
>
>> + if (req->nsge) {
>> + if (req->datadir == CSIO_IOREQF_DMA_WRITE) {
>> + req->dcopy = 0;
>> + csio_scsi_data(req, write, _WRITE_);
>> + } else
>> + csio_setup_ddp(scsim, req);
>> + } else {
>> + csio_scsi_cmd(req);
>> + }
>> +
>> + if (likely(req->drv_status == CSIO_SUCCESS)) {
>> + /* change state and enqueue on active_q */
>> + csio_set_state(&req->sm, csio_scsis_io_active);
>> + list_add_tail(&req->sm.sm_list, &scsim->active_q);
>> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
>> + csio_inc_stats(scsim, n_active);
>> +
>> + return;
>> + }
>> + break;
>> +
>> + case CSIO_SCSIE_START_TM:
>> + csio_scsi_cmd(req);
>> + if (req->drv_status == CSIO_SUCCESS) {
>> + /*
>> + * NOTE: We collect the affected I/Os prior to issuing
>> + * LUN reset, and not after it. This is to prevent
>> + * aborting I/Os that get issued after the LUN reset,
>> + * but prior to LUN reset completion (in the event that
>> + * the host stack has not blocked I/Os to a LUN that is
>> + * being reset.
>> + */
>> + csio_set_state(&req->sm, csio_scsis_tm_active);
>> + list_add_tail(&req->sm.sm_list, &scsim->active_q);
>> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
>> + csio_inc_stats(scsim, n_tm_active);
>> + }
>> + return;
>> +
>> + case CSIO_SCSIE_ABORT:
>> + case CSIO_SCSIE_CLOSE:
>> + /*
>> + * NOTE:
>> + * We could get here due to :
>> + * - a window in the cleanup path of the SCSI module
>> + * (csio_scsi_abort_io()). Please see NOTE in this function.
>> + * - a window in the time we tried to issue an abort/close
>> + * of a request to FW, and the FW completed the request
>> + * itself.
>> + * Print a message for now, and return INVAL either way.
>> + */
>> + req->drv_status = CSIO_INVAL;
>> + csio_warn(hw, "Trying to abort/close completed IO:%p!\n", req);
>> + break;
>> +
>> + default:
>> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
>> + CSIO_DB_ASSERT(0);
>> + }
>> +}
>> +
>> +static void
>> +csio_scsis_io_active(struct csio_ioreq *req, enum csio_scsi_ev evt)
>> +{
>> + struct csio_hw *hw = req->lnode->hwp;
>> + struct csio_scsim *scm = csio_hw_to_scsim(hw);
>> + struct csio_rnode *rn;
>> +
>> + switch (evt) {
>> +
>> + case CSIO_SCSIE_COMPLETED:
>
> Ditto
>
>> + csio_dec_stats(scm, n_active);
>> + list_del_init(&req->sm.sm_list);
>> + csio_set_state(&req->sm, csio_scsis_uninit);
>> + /*
>> + * In MSIX mode, with multiple queues, the SCSI compeltions
>> + * could reach us sooner than the FW events sent to indicate
>> + * I-T nexus loss (link down, remote device logo etc). We
>> + * dont want to be returning such I/Os to the upper layer
>> + * immediately, since we wouldnt have reported the I-T nexus
>> + * loss itself. This forces us to serialize such completions
>> + * with the reporting of the I-T nexus loss. Therefore, we
>> + * internally queue up such up such completions in the rnode.
>> + * The reporting of I-T nexus loss to the upper layer is then
>> + * followed by the returning of I/Os in this internal queue.
>> + * Having another state alongwith another queue helps us take
>> + * actions for events such as ABORT received while we are
>> + * in this rnode queue.
>> + */
>> + if (unlikely(req->wr_status != FW_SUCCESS)) {
>> + rn = req->rnode;
>> + /*
>> + * FW says remote device is lost, but rnode
>> + * doesnt reflect it.
>> + */
>> + if (csio_scsi_itnexus_loss_error(req->wr_status) &&
>> + csio_is_rnode_ready(rn)) {
>> + csio_set_state(&req->sm,
>> + csio_scsis_shost_cmpl_await);
>> + list_add_tail(&req->sm.sm_list,
>> + &rn->host_cmpl_q);
>> + }
>> + }
>> +
>> + break;
>> +
>> + case CSIO_SCSIE_ABORT:
>> + csio_scsi_abrt_cls(req, SCSI_ABORT);
>> + if (req->drv_status == CSIO_SUCCESS) {
>> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
>> + csio_set_state(&req->sm, csio_scsis_aborting);
>> + }
>> + break;
>> +
>> + case CSIO_SCSIE_CLOSE:
>> + csio_scsi_abrt_cls(req, SCSI_CLOSE);
>> + if (req->drv_status == CSIO_SUCCESS) {
>> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
>> + csio_set_state(&req->sm, csio_scsis_closing);
>> + }
>> + break;
>> +
>> + case CSIO_SCSIE_DRVCLEANUP:
>> + req->wr_status = FW_HOSTERROR;
>> + csio_dec_stats(scm, n_active);
>> + csio_set_state(&req->sm, csio_scsis_uninit);
>> + break;
>> +
>> + default:
>> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
>> + CSIO_DB_ASSERT(0);
>> + }
>> +}
>> +
>> +static void
>> +csio_scsis_tm_active(struct csio_ioreq *req, enum csio_scsi_ev evt)
>> +{
>> + struct csio_hw *hw = req->lnode->hwp;
>> + struct csio_scsim *scm = csio_hw_to_scsim(hw);
>> +
>> + switch (evt) {
>> +
>> + case CSIO_SCSIE_COMPLETED:
>
> Ditto
>
>> + csio_dec_stats(scm, n_tm_active);
>> + list_del_init(&req->sm.sm_list);
>> + csio_set_state(&req->sm, csio_scsis_uninit);
>> +
>> + break;
>> +
>> + case CSIO_SCSIE_ABORT:
>> + csio_scsi_abrt_cls(req, SCSI_ABORT);
>> + if (req->drv_status == CSIO_SUCCESS) {
>> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
>> + csio_set_state(&req->sm, csio_scsis_aborting);
>> + }
>> + break;
>> +
>> +
>> + case CSIO_SCSIE_CLOSE:
>> + csio_scsi_abrt_cls(req, SCSI_CLOSE);
>> + if (req->drv_status == CSIO_SUCCESS) {
>> + csio_wr_issue(hw, req->eq_idx, CSIO_FALSE);
>> + csio_set_state(&req->sm, csio_scsis_closing);
>> + }
>> + break;
>> +
>> + case CSIO_SCSIE_DRVCLEANUP:
>> + req->wr_status = FW_HOSTERROR;
>> + csio_dec_stats(scm, n_tm_active);
>> + csio_set_state(&req->sm, csio_scsis_uninit);
>> + break;
>> +
>> + default:
>> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
>> + CSIO_DB_ASSERT(0);
>> + }
>> +}
>> +
>> +static void
>> +csio_scsis_aborting(struct csio_ioreq *req, enum csio_scsi_ev evt)
>> +{
>> + struct csio_hw *hw = req->lnode->hwp;
>> + struct csio_scsim *scm = csio_hw_to_scsim(hw);
>> +
>> + switch (evt) {
>> +
>> + case CSIO_SCSIE_COMPLETED:
>
> Ditto
>
>> + csio_dbg(hw,
>> + "ioreq %p recvd cmpltd (wr_status:%d) "
>> + "in aborting st\n", req, req->wr_status);
>> + /*
>> + * Use CSIO_CANCELLED to explicitly tell the ABORTED event that
>> + * the original I/O was returned to driver by FW.
>> + * We dont really care if the I/O was returned with success by
>> + * FW (because the ABORT and completion of the I/O crossed each
>> + * other), or any other return value. Once we are in aborting
>> + * state, the success or failure of the I/O is unimportant to
>> + * us.
>> + */
>> + req->drv_status = CSIO_CANCELLED;
>> + break;
>> +
>> + case CSIO_SCSIE_ABORT:
>> + csio_inc_stats(scm, n_abrt_dups);
>> + break;
>> +
>> + case CSIO_SCSIE_ABORTED:
>> +
>> + csio_dbg(hw, "abort of %p return status:0x%x drv_status:%x\n",
>> + req, req->wr_status, req->drv_status);
>> + /*
>> + * Check if original I/O WR completed before the Abort
>> + * completion.
>> + */
>> + if (req->drv_status != CSIO_CANCELLED) {
>> + csio_fatal(hw,
>> + "Abort completed before original I/O,"
>> + " req:%p\n", req);
>> + CSIO_DB_ASSERT(0);
>> + }
>> +
>> + /*
>> + * There are the following possible scenarios:
>> + * 1. The abort completed successfully, FW returned FW_SUCCESS.
>> + * 2. The completion of an I/O and the receipt of
>> + * abort for that I/O by the FW crossed each other.
>> + * The FW returned FW_EINVAL. The original I/O would have
>> + * returned with FW_SUCCESS or any other SCSI error.
>> + * 3. The FW couldnt sent the abort out on the wire, as there
>> + * was an I-T nexus loss (link down, remote device logged
>> + * out etc). FW sent back an appropriate IT nexus loss status
>> + * for the abort.
>> + * 4. FW sent an abort, but abort timed out (remote device
>> + * didnt respond). FW replied back with
>> + * FW_SCSI_ABORT_TIMEDOUT.
>> + * 5. FW couldnt genuinely abort the request for some reason,
>> + * and sent us an error.
>> + *
>> + * The first 3 scenarios are treated as succesful abort
>> + * operations by the host, while the last 2 are failed attempts
>> + * to abort. Manipulate the return value of the request
>> + * appropriately, so that host can convey these results
>> + * back to the upper layer.
>> + */
>> + if ((req->wr_status == FW_SUCCESS) ||
>> + (req->wr_status == FW_EINVAL) ||
>> + csio_scsi_itnexus_loss_error(req->wr_status))
>> + req->wr_status = FW_SCSI_ABORT_REQUESTED;
>> +
>> + csio_dec_stats(scm, n_active);
>> + list_del_init(&req->sm.sm_list);
>> + csio_set_state(&req->sm, csio_scsis_uninit);
>> + break;
>> +
>> + case CSIO_SCSIE_DRVCLEANUP:
>> + req->wr_status = FW_HOSTERROR;
>> + csio_dec_stats(scm, n_active);
>> + csio_set_state(&req->sm, csio_scsis_uninit);
>> + break;
>> +
>> + case CSIO_SCSIE_CLOSE:
>> + /*
>> + * We can receive this event from the module
>> + * cleanup paths, if the FW forgot to reply to the ABORT WR
>> + * and left this ioreq in this state. For now, just ignore
>> + * the event. The CLOSE event is sent to this state, as
>> + * the LINK may have already gone down.
>> + */
>> + break;
>> +
>> + default:
>> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
>> + CSIO_DB_ASSERT(0);
>> + }
>> +}
>> +
>> +static void
>> +csio_scsis_closing(struct csio_ioreq *req, enum csio_scsi_ev evt)
>> +{
>> + struct csio_hw *hw = req->lnode->hwp;
>> + struct csio_scsim *scm = csio_hw_to_scsim(hw);
>> +
>> + switch (evt) {
>> +
>> + case CSIO_SCSIE_COMPLETED:
>
> Ditto
>
>> + csio_dbg(hw,
>> + "ioreq %p recvd cmpltd (wr_status:%d) "
>> + "in closing st\n", req, req->wr_status);
>> + /*
>> + * Use CSIO_CANCELLED to explicitly tell the CLOSED event that
>> + * the original I/O was returned to driver by FW.
>> + * We dont really care if the I/O was returned with success by
>> + * FW (because the CLOSE and completion of the I/O crossed each
>> + * other), or any other return value. Once we are in aborting
>> + * state, the success or failure of the I/O is unimportant to
>> + * us.
>> + */
>> + req->drv_status = CSIO_CANCELLED;
>> + break;
>> +
>> + case CSIO_SCSIE_CLOSED:
>> + /*
>> + * Check if original I/O WR completed before the Close
>> + * completion.
>> + */
>> + if (req->drv_status != CSIO_CANCELLED) {
>> + csio_fatal(hw,
>> + "Close completed before original I/O,"
>> + " req:%p\n", req);
>> + CSIO_DB_ASSERT(0);
>> + }
>> +
>> + /*
>> + * Either close succeeded, or we issued close to FW at the
>> + * same time FW compelted it to us. Either way, the I/O
>> + * is closed.
>> + */
>> + CSIO_DB_ASSERT((req->wr_status == FW_SUCCESS) ||
>> + (req->wr_status == FW_EINVAL));
>> + req->wr_status = FW_SCSI_CLOSE_REQUESTED;
>> +
>> + csio_dec_stats(scm, n_active);
>> + list_del_init(&req->sm.sm_list);
>> + csio_set_state(&req->sm, csio_scsis_uninit);
>> + break;
>> +
>> + case CSIO_SCSIE_CLOSE:
>> + break;
>> +
>> + case CSIO_SCSIE_DRVCLEANUP:
>> + req->wr_status = FW_HOSTERROR;
>> + csio_dec_stats(scm, n_active);
>> + csio_set_state(&req->sm, csio_scsis_uninit);
>> + break;
>> +
>> + default:
>> + csio_dbg(hw, "Unhandled event:%d sent to req:%p\n", evt, req);
>> + CSIO_DB_ASSERT(0);
>> + }
>> +}
>> +
>> +static void
>> +csio_scsis_shost_cmpl_await(struct csio_ioreq *req, enum csio_scsi_ev evt)
>> +{
>> + switch (evt) {
>> + case CSIO_SCSIE_ABORT:
>> + case CSIO_SCSIE_CLOSE:
>> + /*
>> + * Just succeed the abort request, and hope that
>> + * the remote device unregister path will cleanup
>> + * this I/O to the upper layer within a sane
>> + * amount of time.
>> + */
>> + /*
>> + * A close can come in during a LINK DOWN. The FW would have
>> + * returned us the I/O back, but not the remote device lost
>> + * FW event. In this interval, if the I/O times out at the upper
>> + * layer, a close can come in. Take the same action as abort:
>> + * return success, and hope that the remote device unregister
>> + * path will cleanup this I/O. If the FW still doesnt send
>> + * the msg, the close times out, and the upper layer resorts
>> + * to the next level of error recovery.
>> + */
>> + req->drv_status = CSIO_SUCCESS;
>> + break;
>> + case CSIO_SCSIE_DRVCLEANUP:
>> + csio_set_state(&req->sm, csio_scsis_uninit);
>> + break;
>> + default:
>> + csio_dbg(req->lnode->hwp, "Unhandled event:%d sent to req:%p\n",
>> + evt, req);
>> + CSIO_DB_ASSERT(0);
>> + }
>> +}
>> +
>
>
>> +
>> +/**
>> + * csio_queuecommand_lck - Entry point to kickstart an I/O request.
>> + * @cmnd: The I/O request from ML.
>> + * @done: The ML callback routine.
>> + *
>> + * This routine does the following:
>> + * - Checks for HW and Rnode module readiness.
>> + * - Gets a free ioreq structure (which is already initialized
>> + * to uninit during its allocation).
>> + * - Maps SG elements.
>> + * - Initializes ioreq members.
>> + * - Kicks off the SCSI state machine for this IO.
>> + * - Returns busy status on error.
>> + */
>> +static int
>> +csio_queuecommand_lck(struct scsi_cmnd *cmnd, void (*done)(struct scsi_cmnd *))
>> +{
>> + struct csio_lnode *ln = shost_priv(cmnd->device->host);
>> + struct csio_hw *hw = csio_lnode_to_hw(ln);
>> + struct csio_scsim *scsim = csio_hw_to_scsim(hw);
>> + struct csio_rnode *rn = (struct csio_rnode *)(cmnd->device->hostdata);
>> + struct csio_ioreq *ioreq = NULL;
>> + unsigned long flags;
>> + int nsge = 0;
>> + int rv = SCSI_MLQUEUE_HOST_BUSY, nr;
>> + csio_retval_t retval;
>> + int cpu;
>> + struct csio_scsi_qset *sqset;
>> + struct fc_rport *rport = starget_to_rport(scsi_target(cmnd->device));
>> +
>> + if (!blk_rq_cpu_valid(cmnd->request))
>> + cpu = smp_processor_id();
>> + else
>> + cpu = cmnd->request->cpu;
>> +
>> + sqset = &hw->sqset[ln->portid][cpu];
>> +
>> + nr = fc_remote_port_chkready(rport);
>> + if (nr) {
>> + cmnd->result = nr;
>> + csio_inc_stats(scsim, n_rn_nr_error);
>> + goto err_done;
>> + }
>> +
>> + if (unlikely(!csio_is_hw_ready(hw))) {
>> + cmnd->result = (DID_REQUEUE << 16);
>> + csio_inc_stats(scsim, n_hw_nr_error);
>> + goto err_done;
>> + }
>> +
>> + /* Get req->nsge, if there are SG elements to be mapped */
>> + nsge = scsi_dma_map(cmnd);
>> + if (unlikely(nsge < 0)) {
>> + csio_inc_stats(scsim, n_dmamap_error);
>> + goto err;
>> + }
>> +
>> + /* Do we support so many mappings? */
>> + if (unlikely(nsge > scsim->max_sge)) {
>> + csio_warn(hw,
>> + "More SGEs than can be supported."
>> + " SGEs: %d, Max SGEs: %d\n", nsge, scsim->max_sge);
>> + csio_inc_stats(scsim, n_unsupp_sge_error);
>> + goto err_dma_unmap;
>> + }
>> +
>> + /* Get a free ioreq structure - SM is already set to uninit */
>> + ioreq = csio_get_scsi_ioreq_lock(hw, scsim);
>> + if (!ioreq) {
>> + csio_err(hw, "Out of I/O request elements. Active #:%d\n",
>> + scsim->stats.n_active);
>> + csio_inc_stats(scsim, n_no_req_error);
>> + goto err_dma_unmap;
>> + }
>> +
>> + ioreq->nsge = nsge;
>> + ioreq->lnode = ln;
>> + ioreq->rnode = rn;
>> + ioreq->iq_idx = sqset->iq_idx;
>> + ioreq->eq_idx = sqset->eq_idx;
>> + ioreq->wr_status = 0;
>> + ioreq->drv_status = CSIO_SUCCESS;
>> + csio_scsi_cmnd(ioreq) = (void *)cmnd;
>> + ioreq->tmo = 0;
>> +
>> + switch (cmnd->sc_data_direction) {
>> + case DMA_BIDIRECTIONAL:
>> + ioreq->datadir = CSIO_IOREQF_DMA_BIDI;
>> + csio_inc_stats(ln, n_control_requests);
>> + break;
>> + case DMA_TO_DEVICE:
>> + ioreq->datadir = CSIO_IOREQF_DMA_WRITE;
>> + csio_inc_stats(ln, n_output_requests);
>> + ln->stats.n_output_bytes += scsi_bufflen(cmnd);
>> + break;
>> + case DMA_FROM_DEVICE:
>> + ioreq->datadir = CSIO_IOREQF_DMA_READ;
>> + csio_inc_stats(ln, n_input_requests);
>> + ln->stats.n_input_bytes += scsi_bufflen(cmnd);
>> + break;
>> + case DMA_NONE:
>> + ioreq->datadir = CSIO_IOREQF_DMA_NONE;
>> + csio_inc_stats(ln, n_control_requests);
>> + break;
>> + default:
>> + CSIO_DB_ASSERT(0);
>> + break;
>> + }
>> +
>> + /* Set cbfn */
>> + ioreq->io_cbfn = csio_scsi_cbfn;
>> +
>> + /* Needed during abort */
>> + cmnd->host_scribble = (unsigned char *)ioreq;
>> + cmnd->scsi_done = done;
>> + cmnd->SCp.Message = 0;
>> +
>> + /* Kick off SCSI IO SM on the ioreq */
>> + spin_lock_irqsave(&hw->lock, flags);
>> + retval = csio_scsi_start_io(ioreq);
>> + spin_unlock_irqrestore(&hw->lock, flags);
>> +
>> + if (retval != CSIO_SUCCESS) {
>> + csio_err(hw, "ioreq: %p couldnt be started, status:%d\n",
>> + ioreq, retval);
>> + csio_inc_stats(scsim, n_busy_error);
>> + goto err_put_req;
>> + }
>> +
>> + return 0;
>> +
>> +err_put_req:
>> + csio_put_scsi_ioreq_lock(hw, scsim, ioreq);
>> +err_dma_unmap:
>> + if (nsge > 0)
>> + scsi_dma_unmap(cmnd);
>> +err:
>> + return rv;
>> +
>> +err_done:
>> + done(cmnd);
>> + return 0;
>> +}
>> +
>> +static DEF_SCSI_QCMD(csio_queuecommand);
>> +
>
> This means that your running with the host_lock held.. I'm not sure if
> that is really what you want to do as it really end's up killing
> multi-lun small packet performance..
>
> How about dropping DEF_SCSI_QCMD usage here, and figure out what
> actually needs to be protected by the SCSI host_lock within
> csio_queuecommand_lck()..?
It is on my TODO list for the next version of the driver, after the
initial submission. Per the current design, we shouldnt need the
host_lock to be held, but I would like to test this change thoroughly
before I submit it.
>> +static csio_retval_t
>> +csio_do_abrt_cls(struct csio_hw *hw, struct csio_ioreq *ioreq, bool abort)
>> +{
>> + csio_retval_t rv;
>> + int cpu = smp_processor_id();
>> + struct csio_lnode *ln = ioreq->lnode;
>> + struct csio_scsi_qset *sqset = &hw->sqset[ln->portid][cpu];
>> +
>> + ioreq->tmo = CSIO_SCSI_ABRT_TMO_MS;
>> + /*
>> + * Use current processor queue for posting the abort/close, but retain
>> + * the ingress queue ID of the original I/O being aborted/closed - we
>> + * need the abort/close completion to be received on the same queue
>> + * as the original I/O.
>> + */
>> + ioreq->eq_idx = sqset->eq_idx;
>> +
>> + if (abort == SCSI_ABORT)
>> + rv = csio_scsi_abort(ioreq);
>> + else /* close */
>
> Point-less comment.
I will remove it.
>
>> + rv = csio_scsi_close(ioreq);
>> +
>> + return rv;
>> +}
>> +
>> +static int
>> +csio_eh_abort_handler(struct scsi_cmnd *cmnd)
>> +{
>> + struct csio_ioreq *ioreq;
>> + struct csio_lnode *ln = shost_priv(cmnd->device->host);
>> + struct csio_hw *hw = csio_lnode_to_hw(ln);
>> + struct csio_scsim *scsim = csio_hw_to_scsim(hw);
>> + int ready = 0, ret;
>> + unsigned long tmo = 0;
>> + csio_retval_t rv;
>> + struct csio_rnode *rn = (struct csio_rnode *)(cmnd->device->hostdata);
>> +
>> + ret = fc_block_scsi_eh(cmnd);
>> + if (ret)
>> + return ret;
>> +
>> + ioreq = (struct csio_ioreq *)cmnd->host_scribble;
>> + if (!ioreq)
>> + return SUCCESS;
>> +
>> + if (!rn)
>> + return FAILED;
>> +
>> + csio_dbg(hw,
>> + "Request to abort ioreq:%p cmd:%p cdb:%08llx"
>> + " ssni:0x%x lun:%d iq:0x%x\n",
>> + ioreq, cmnd, *((uint64_t *)cmnd->cmnd), rn->flowid,
>> + cmnd->device->lun, csio_q_physiqid(hw, ioreq->iq_idx));
>> +
>> + if (((struct scsi_cmnd *)csio_scsi_cmnd(ioreq)) != cmnd) {
>> + csio_inc_stats(scsim, n_abrt_race_comp);
>> + return SUCCESS;
>> + }
>> +
>> + ready = csio_is_lnode_ready(ln);
>> + tmo = CSIO_SCSI_ABRT_TMO_MS;
>> +
>> + spin_lock_irq(&hw->lock);
>> + rv = csio_do_abrt_cls(hw, ioreq, (ready ? SCSI_ABORT : SCSI_CLOSE));
>> + spin_unlock_irq(&hw->lock);
>> +
>> + if (rv != CSIO_SUCCESS) {
>> + if (rv == CSIO_INVAL) {
>> + /* Return success, if abort/close request issued on
>> + * already completed IO
>> + */
>> + return SUCCESS;
>> + }
>> + if (ready)
>> + csio_inc_stats(scsim, n_abrt_busy_error);
>> + else
>> + csio_inc_stats(scsim, n_cls_busy_error);
>> +
>> + goto inval_scmnd;
>> + }
>> +
>> + /* Wait for completion */
>> + init_completion(&ioreq->cmplobj);
>> + wait_for_completion_timeout(&ioreq->cmplobj, msecs_to_jiffies(tmo));
>> +
>> + /* FW didnt respond to abort within our timeout */
>> + if (((struct scsi_cmnd *)csio_scsi_cmnd(ioreq)) == cmnd) {
>> +
>> + csio_err(hw, "Abort timed out -- req: %p\n", ioreq);
>> + csio_inc_stats(scsim, n_abrt_timedout);
>> +
>> +inval_scmnd:
>> + if (ioreq->nsge > 0)
>> + scsi_dma_unmap(cmnd);
>> +
>> + spin_lock_irq(&hw->lock);
>> + csio_scsi_cmnd(ioreq) = NULL;
>> + spin_unlock_irq(&hw->lock);
>> +
>> + cmnd->result = (DID_ERROR << 16);
>> + cmnd->scsi_done(cmnd);
>> +
>> + return FAILED;
>> + }
>> +
>> + /* FW successfully aborted the request */
>> + if (host_byte(cmnd->result) == DID_REQUEUE) {
>> + csio_info(hw,
>> + "Aborted SCSI command to (%d:%d) serial#:0x%lx\n",
>> + cmnd->device->id, cmnd->device->lun,
>> + cmnd->serial_number);
>> + return SUCCESS;
>> + } else {
>> + csio_info(hw,
>> + "Failed to abort SCSI command, (%d:%d) serial#:0x%lx\n",
>> + cmnd->device->id, cmnd->device->lun,
>> + cmnd->serial_number);
>> + return FAILED;
>> + }
>> +}
>> +
>
>> +struct scsi_host_template csio_fcoe_shost_template = {
>> + .module = THIS_MODULE,
>> + .name = CSIO_DRV_DESC,
>> + .proc_name = KBUILD_MODNAME,
>> + .queuecommand = csio_queuecommand,
>> + .eh_abort_handler = csio_eh_abort_handler,
>> + .eh_device_reset_handler = csio_eh_lun_reset_handler,
>> + .slave_alloc = csio_slave_alloc,
>> + .slave_configure = csio_slave_configure,
>> + .slave_destroy = csio_slave_destroy,
>> + .scan_finished = csio_scan_finished,
>> + .this_id = -1,
>> + .sg_tablesize = CSIO_SCSI_MAX_SGE,
>> + .cmd_per_lun = CSIO_MAX_CMD_PER_LUN,
>> + .use_clustering = ENABLE_CLUSTERING,
>> + .shost_attrs = csio_fcoe_lport_attrs,
>> + .max_sectors = CSIO_MAX_SECTOR_SIZE,
>> +};
>> +
>> +struct scsi_host_template csio_fcoe_shost_vport_template = {
>> + .module = THIS_MODULE,
>> + .name = CSIO_DRV_DESC,
>> + .proc_name = KBUILD_MODNAME,
>> + .queuecommand = csio_queuecommand,
>> + .eh_abort_handler = csio_eh_abort_handler,
>> + .eh_device_reset_handler = csio_eh_lun_reset_handler,
>> + .slave_alloc = csio_slave_alloc,
>> + .slave_configure = csio_slave_configure,
>> + .slave_destroy = csio_slave_destroy,
>> + .scan_finished = csio_scan_finished,
>> + .this_id = -1,
>> + .sg_tablesize = CSIO_SCSI_MAX_SGE,
>> + .cmd_per_lun = CSIO_MAX_CMD_PER_LUN,
>> + .use_clustering = ENABLE_CLUSTERING,
>> + .shost_attrs = csio_fcoe_vport_attrs,
>> + .max_sectors = CSIO_MAX_SECTOR_SIZE,
>> +};
>> +
>> +/*
>> + * csio_scsi_alloc_ddp_bufs - Allocate buffers for DDP of unaligned SGLs.
>> + * @scm: SCSI Module
>> + * @hw: HW device.
>> + * @buf_size: buffer size
>> + * @num_buf : Number of buffers.
>> + *
>> + * This routine allocates DMA buffers required for SCSI Data xfer, if
>> + * each SGL buffer for a SCSI Read request posted by SCSI midlayer are
>> + * not virtually contiguous.
>> + */
>> +static csio_retval_t
>> +csio_scsi_alloc_ddp_bufs(struct csio_scsim *scm, struct csio_hw *hw,
>> + int buf_size, int num_buf)
>> +{
>> + int n = 0;
>> + struct list_head *tmp;
>> + struct csio_dma_buf *ddp_desc = NULL;
>> + uint32_t unit_size = 0;
>> +
>> + if (!num_buf)
>> + return CSIO_SUCCESS;
>
> Just return 0..?
Since there is a comment about driver-internal return values in your
review of patch 6, I would like to take up this discussion in that thread.
>
>> +
>> + if (!buf_size)
>> + return CSIO_INVAL;
>
> Just return -EINVAL..?
>
>> +
>> + INIT_LIST_HEAD(&scm->ddp_freelist);
>> +
>> + /* Align buf size to page size */
>> + buf_size = (buf_size + PAGE_SIZE - 1) & PAGE_MASK;
>> + /* Initialize dma descriptors */
>> + for (n = 0; n < num_buf; n++) {
>> + /* Set unit size to request size */
>> + unit_size = buf_size;
>> + ddp_desc = kzalloc(sizeof(struct csio_dma_buf), GFP_KERNEL);
>> + if (!ddp_desc) {
>> + csio_err(hw,
>> + "Failed to allocate ddp descriptors,"
>> + " Num allocated = %d.\n",
>> + scm->stats.n_free_ddp);
>> + goto no_mem;
>> + }
>> +
>> + /* Allocate Dma buffers for DDP */
>> + ddp_desc->vaddr = pci_alloc_consistent(hw->pdev, unit_size,
>> + &ddp_desc->paddr);
>> + if (!ddp_desc->vaddr) {
>> + csio_err(hw,
>> + "SCSI response DMA buffer (ddp) allocation"
>> + " failed!\n");
>> + kfree(ddp_desc);
>> + goto no_mem;
>> + }
>> +
>> + ddp_desc->len = unit_size;
>> +
>> + /* Added it to scsi ddp freelist */
>> + list_add_tail(&ddp_desc->list, &scm->ddp_freelist);
>> + csio_inc_stats(scm, n_free_ddp);
>> + }
>> +
>> + return CSIO_SUCCESS;
>
> Just return 0..?
>
>> +no_mem:
>> + /* release dma descs back to freelist and free dma memory */
>> + list_for_each(tmp, &scm->ddp_freelist) {
>> + ddp_desc = (struct csio_dma_buf *) tmp;
>> + tmp = csio_list_prev(tmp);
>> + pci_free_consistent(hw->pdev, ddp_desc->len, ddp_desc->vaddr,
>> + ddp_desc->paddr);
>> + list_del_init(&ddp_desc->list);
>> + kfree(ddp_desc);
>> + }
>> + scm->stats.n_free_ddp = 0;
>> +
>> + return CSIO_NOMEM;
>
> This should be just -ENOMEM..?
>
>> +}
>> +
>> +/*
>> + * csio_scsi_free_ddp_bufs - free DDP buffers of unaligned SGLs.
>> + * @scm: SCSI Module
>> + * @hw: HW device.
>> + *
>> + * This routine frees ddp buffers.
>> + */
>> +static csio_retval_t
>> +csio_scsi_free_ddp_bufs(struct csio_scsim *scm, struct csio_hw *hw)
>> +{
>> + struct list_head *tmp;
>> + struct csio_dma_buf *ddp_desc;
>> +
>> + /* release dma descs back to freelist and free dma memory */
>> + list_for_each(tmp, &scm->ddp_freelist) {
>> + ddp_desc = (struct csio_dma_buf *) tmp;
>> + tmp = csio_list_prev(tmp);
>> + pci_free_consistent(hw->pdev, ddp_desc->len, ddp_desc->vaddr,
>> + ddp_desc->paddr);
>> + list_del_init(&ddp_desc->list);
>> + kfree(ddp_desc);
>> + }
>> + scm->stats.n_free_ddp = 0;
>> +
>> + return CSIO_NOMEM;
>> +}
>
> Ditto.. Just -ENOMEM..?
>
>
^ permalink raw reply
* [PATCH 1/1] net: ipv4: ipmr_expire_timer causes crash when removing net namespace
From: Francesco Ruggeri @ 2012-08-24 17:38 UTC (permalink / raw)
To: netdev
When tearing down a net namespace, ipv4 mr_table structures are freed
without first deactivating their timers. This can result in a crash in
run_timer_softirq.
This patch mimics the corresponding behaviour in ipv6.
Locking and synchronization seem to be adequate.
We are about to kfree mrt, so existing code should already make sure that
no other references to mrt are pending or can be created by incoming traffic.
The functions invoked here do not cause new references to mrt or other
race conditions to be created.
Invoking del_timer_sync guarantees that ipmr_expire_timer is inactive.
Both ipmr_expire_process (whose completion we may have to wait in
del_timer_sync) and mroute_clean_tables internally use mfc_unres_lock
or other synchronizations when needed, and they both only modify mrt.
Tested in Linux 3.4.8.
Signed-off-by: Francesco Ruggeri <fruggeri@aristanetworks.com>
Index: linux-3.4.x86_64/net/ipv4/ipmr.c
===================================================================
--- linux-3.4.x86_64.orig/net/ipv4/ipmr.c
+++ linux-3.4.x86_64/net/ipv4/ipmr.c
@@ -124,6 +124,8 @@ static DEFINE_SPINLOCK(mfc_unres_lock);
static struct kmem_cache *mrt_cachep __read_mostly;
static struct mr_table *ipmr_new_table(struct net *net, u32 id);
+static void ipmr_free_table(struct mr_table *mrt);
+
static int ip_mr_forward(struct net *net, struct mr_table *mrt,
struct sk_buff *skb, struct mfc_cache *cache,
int local);
@@ -131,6 +133,7 @@ static int ipmr_cache_report(struct mr_t
struct sk_buff *pkt, vifi_t vifi, int assert);
static int __ipmr_fill_mroute(struct mr_table *mrt, struct sk_buff *skb,
struct mfc_cache *c, struct rtmsg *rtm);
+static void mroute_clean_tables(struct mr_table *mrt);
static void ipmr_expire_process(unsigned long arg);
#ifdef CONFIG_IP_MROUTE_MULTIPLE_TABLES
@@ -271,7 +274,7 @@ static void __net_exit ipmr_rules_exit(s
list_for_each_entry_safe(mrt, next, &net->ipv4.mr_tables, list) {
list_del(&mrt->list);
- kfree(mrt);
+ ipmr_free_table(mrt);
}
fib_rules_unregister(net->ipv4.mr_rules_ops);
}
@@ -299,7 +302,7 @@ static int __net_init ipmr_rules_init(st
static void __net_exit ipmr_rules_exit(struct net *net)
{
- kfree(net->ipv4.mrt);
+ ipmr_free_table(net->ipv4.mrt);
}
#endif
@@ -336,6 +339,13 @@ static struct mr_table *ipmr_new_table(s
return mrt;
}
+static void ipmr_free_table(struct mr_table *mrt)
+{
+ del_timer_sync(&mrt->ipmr_expire_timer);
+ mroute_clean_tables(mrt);
+ kfree(mrt);
+}
+
/* Service routines creating virtual interfaces: DVMRP tunnels and PIMREG */
static void ipmr_del_tunnel(struct net_device *dev, struct vifctl *v)
^ permalink raw reply
* Re: [PATCH] netlink: fix possible spoofing from non-root processes
From: David Miller @ 2012-08-24 17:36 UTC (permalink / raw)
To: pablo; +Cc: netdev
In-Reply-To: <1345723751-2924-1-git-send-email-pablo@netfilter.org>
From: pablo@netfilter.org
Date: Thu, 23 Aug 2012 14:09:11 +0200
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> Non-root user-space processes can send Netlink messages to other
> processes that are well-known for being subscribed to Netlink
> asynchronous notifications. This allows ilegitimate non-root
> process to send forged messages to Netlink subscribers.
>
> The userspace process usually verifies the legitimate origin in
> two ways:
>
> a) Socket credentials. If UID != 0, then the message comes from
> some ilegitimate process and the message needs to be dropped.
>
> b) Netlink portID. In general, portID == 0 means that the origin
> of the messages comes from the kernel. Thus, discarding any
> message not coming from the kernel.
>
> However, ctnetlink sets the portID in event messages that has
> been triggered by some user-space process, eg. conntrack utility.
> So other processes subscribed to ctnetlink events, eg. conntrackd,
> know that the event was triggered by some user-space action.
>
> Neither of the two ways to discard ilegitimate messages coming
> from non-root processes can help for ctnetlink.
>
> This patch adds capability validation in case that dst_pid is set
> in netlink_sendmsg(). This approach is aggressive since existing
> applications using any Netlink bus to deliver messages between
> two user-space processes will break. Note that the exception is
> NETLINK_USERSOCK, since it is reserved for netlink-to-netlink
> userspace communication.
>
> Still, if anyone wants that his Netlink bus allows netlink-to-netlink
> userspace, then they can set NL_NONROOT_SEND. However, by default,
> I don't think it makes sense to allow to use NETLINK_ROUTE to
> communicate two processes that are sending no matter what information
> that is not related to link/neighbouring/routing. They should be using
> NETLINK_USERSOCK instead for that.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Applied and queued up for -stable, thanks Pablo.
^ permalink raw reply
* Re: [PATCH net-next] w5300: using eth_hw_addr_random() for random MAC and set device flag
From: David Miller @ 2012-08-24 17:31 UTC (permalink / raw)
To: weiyj.lk; +Cc: yongjun_wei, netdev
In-Reply-To: <CAPgLHd-v=M6QrhT3YnTqUXWqyUa3TgVtNN3Cao1sPXd7stAEww@mail.gmail.com>
From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Thu, 23 Aug 2012 15:28:40 +0800
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Using eth_hw_addr_random() to generate a random Ethernet address
> (MAC) to be used by a net device and set addr_assign_type.
> Not need to duplicating its implementation.
>
> spatch with a semantic match is used to found this problem.
> (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] w5100: using eth_hw_addr_random() for random MAC and set device flag
From: David Miller @ 2012-08-24 17:30 UTC (permalink / raw)
To: weiyj.lk; +Cc: yongjun_wei, netdev
In-Reply-To: <CAPgLHd_akGWHksgn_zR5T6CxO54tPSojzAfkPr+kJKe9i8B9KQ@mail.gmail.com>
From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Thu, 23 Aug 2012 15:28:19 +0800
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Using eth_hw_addr_random() to generate a random Ethernet address
> (MAC) to be used by a net device and set addr_assign_type.
> Not need to duplicating its implementation.
>
> spatch with a semantic match is used to found this problem.
> (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH] drivers/net: use is_zero_ether_addr() instead of memcmp()
From: David Miller @ 2012-08-24 17:30 UTC (permalink / raw)
To: weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w
Cc: j, linville-2XuSBdqkA4R54TAoqtyWWQ,
yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAPgLHd_L1=1zaq=NUAjr+3822WfR2gV-sMA2tzKowHEezPmvOA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Wei Yongjun <weiyj.lk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Thu, 23 Aug 2012 15:16:44 +0800
> From: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
>
> Using is_zero_ether_addr() instead of directly use
> memcmp() to determine if the ethernet address is all
> zeros.
>
> spatch with a semantic match is used to found this problem.
> (http://coccinelle.lip6.fr/)
>
> Signed-off-by: Wei Yongjun <yongjun_wei-zrsr2BFq86L20UzCJQGyNP8+0UxHXcjY@public.gmane.org>
Since this is wireless, John will pick it up.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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
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