* [net-next v2 00/10][pull request] Intel Wired LAN Driver Updates 2014-11-11
From: Jeff Kirsher @ 2014-11-11 14:44 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene, john.ronciak
This series contains updates to i40e, i40evf and ixgbe.
Kamil updated the i40e and i40evf driver to poll the firmware slower
since we were polling faster than the firmware could respond.
Shannon updates i40e to add a check to keep the service_task from
running the periodic tasks more than once per second, while still
allowing quick action to service the events.
Jesse cleans up the throttle rate code by fixing the minimum interrupt
throttle rate and removing some unused defines.
Mitch makes the early init admin queue message receive code more robust
by handling messages in a loop and ignoring those that we are not
interested in. This also gets rid of some scary log messages that
really do not indicate a problem.
Don provides several ixgbe patches, first fixes an issue with x540
completion timeout where on topologies including few levels of PCIe
switching for x540 can run into an unexpected completion error. Cleans
up the functionality in ixgbe_ndo_set_vf_vlan() in preparation for
future work. Adds support for x550 MAC's to the driver.
v2:
- Remove code comment in patch 01 of the series, based on feedback from
David Liaght
- Updated the "goto" to "break" statements in patch 06 of the series,
based on feedback from Sergei Shtylyov
- Initialized the variable err due to the possibility of use before
being assigned a value in patch 07 of the series
- Added patch "ixgbe: add helper function for setting RSS key in
preparation of X550" since it is needed for the addition of X550 MAC
support
The following are changes since commit 2e1af7d74f4f7b4d4c1b0fbf5da3b5f92d9c332f:
mlx4: restore conditional call to napi_complete_done()
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Don Skidmore (5):
ixgbe: fix X540 Completion timeout
ixgbe: cleanup ixgbe_ndo_set_vf_vlan
ixgbe: cleanup move setting PFQDE.HIDE_VLAN to support function.
ixgbe: Add new support for X550 MAC's
ixgbe: add helper function for setting RSS key in perperation of X550
Jesse Brandeburg (1):
i40e: clean up throttle rate code
Kamil Krawczyk (1):
i40e: poll firmware slower
Mitch Williams (2):
i40evf: make early init processing more robust
i40evf: don't use more queues than CPUs
Shannon Nelson (1):
i40e: don't do link_status or stats collection on every ARQ
drivers/net/ethernet/intel/i40e/i40e.h | 3 +-
drivers/net/ethernet/intel/i40e/i40e_adminq.c | 6 +-
drivers/net/ethernet/intel/i40e/i40e_adminq.h | 2 +-
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 11 +-
drivers/net/ethernet/intel/i40e/i40e_main.c | 14 ++-
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 5 +-
drivers/net/ethernet/intel/i40evf/i40e_adminq.c | 6 +-
drivers/net/ethernet/intel/i40evf/i40e_adminq.h | 2 +-
drivers/net/ethernet/intel/i40evf/i40e_txrx.h | 5 +-
drivers/net/ethernet/intel/i40evf/i40evf.h | 1 +
drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 16 +--
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 50 ++++----
.../net/ethernet/intel/i40evf/i40evf_virtchnl.c | 65 +++++-----
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 50 ++++++--
drivers/net/ethernet/intel/ixgbe/ixgbe_dcb.c | 8 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 37 +++++-
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 2 +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 122 +++++++++++++++----
drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.c | 4 +
drivers/net/ethernet/intel/ixgbe/ixgbe_phy.c | 64 +++++-----
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 131 ++++++++++++++-------
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 45 +++++--
23 files changed, 441 insertions(+), 209 deletions(-)
--
1.9.3
^ permalink raw reply
* [net-next v2 08/10] ixgbe: cleanup move setting PFQDE.HIDE_VLAN to support function.
From: Jeff Kirsher @ 2014-11-11 14:44 UTC (permalink / raw)
To: davem; +Cc: Don Skidmore, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1415717086-3441-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Don Skidmore <donald.c.skidmore@intel.com>
Move setting of drop enable to support function. This not only makes the
code more readable but is also prep for following patches that add
additional MAC support.
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 31 ++++++++++++++++++--------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 07d1c04..0c25df5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -618,6 +618,27 @@ int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask)
return 0;
}
+static inline void ixgbe_write_qde(struct ixgbe_adapter *adapter, u32 vf,
+ u32 qde)
+{
+ struct ixgbe_hw *hw = &adapter->hw;
+ struct ixgbe_ring_feature *vmdq = &adapter->ring_feature[RING_F_VMDQ];
+ u32 q_per_pool = __ALIGN_MASK(1, ~vmdq->mask);
+ int i;
+
+ for (i = vf * q_per_pool; i < ((vf + 1) * q_per_pool); i++) {
+ u32 reg;
+
+ /* flush previous write */
+ IXGBE_WRITE_FLUSH(hw);
+
+ /* indicate to hardware that we want to set drop enable */
+ reg = IXGBE_QDE_WRITE | IXGBE_QDE_ENABLE;
+ reg |= i << IXGBE_QDE_IDX_SHIFT;
+ IXGBE_WRITE_REG(hw, IXGBE_QDE, reg);
+ }
+}
+
static int ixgbe_vf_reset_msg(struct ixgbe_adapter *adapter, u32 vf)
{
struct ixgbe_ring_feature *vmdq = &adapter->ring_feature[RING_F_VMDQ];
@@ -647,15 +668,7 @@ static int ixgbe_vf_reset_msg(struct ixgbe_adapter *adapter, u32 vf)
IXGBE_WRITE_REG(hw, IXGBE_VFTE(reg_offset), reg);
/* force drop enable for all VF Rx queues */
- for (i = vf * q_per_pool; i < ((vf + 1) * q_per_pool); i++) {
- /* flush previous write */
- IXGBE_WRITE_FLUSH(hw);
-
- /* indicate to hardware that we want to set drop enable */
- reg = IXGBE_QDE_WRITE | IXGBE_QDE_ENABLE;
- reg |= i << IXGBE_QDE_IDX_SHIFT;
- IXGBE_WRITE_REG(hw, IXGBE_QDE, reg);
- }
+ ixgbe_write_qde(adapter, vf, IXGBE_QDE_ENABLE);
/* enable receive for vf */
reg = IXGBE_READ_REG(hw, IXGBE_VFRE(reg_offset));
--
1.9.3
^ permalink raw reply related
* [net-next v2 07/10] ixgbe: cleanup ixgbe_ndo_set_vf_vlan
From: Jeff Kirsher @ 2014-11-11 14:44 UTC (permalink / raw)
To: davem; +Cc: Don Skidmore, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1415717086-3441-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Don Skidmore <donald.c.skidmore@intel.com>
Clean up functionality in ixgbe_ndo_set_vf_vlan that will simplify later
patches.
Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
v2 initialized err in the first function because there was a possibility
of it being used before it being assigned a value
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 94 +++++++++++++++++---------
1 file changed, 61 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 97c85b8..07d1c04 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -1079,52 +1079,80 @@ int ixgbe_ndo_set_vf_mac(struct net_device *netdev, int vf, u8 *mac)
return ixgbe_set_vf_mac(adapter, vf, mac);
}
+static int ixgbe_enable_port_vlan(struct ixgbe_adapter *adapter, int vf,
+ u16 vlan, u8 qos)
+{
+ struct ixgbe_hw *hw = &adapter->hw;
+ int err = 0;
+
+ if (adapter->vfinfo[vf].pf_vlan)
+ err = ixgbe_set_vf_vlan(adapter, false,
+ adapter->vfinfo[vf].pf_vlan,
+ vf);
+ if (err)
+ goto out;
+ ixgbe_set_vmvir(adapter, vlan, qos, vf);
+ ixgbe_set_vmolr(hw, vf, false);
+ if (adapter->vfinfo[vf].spoofchk_enabled)
+ hw->mac.ops.set_vlan_anti_spoofing(hw, true, vf);
+ adapter->vfinfo[vf].vlan_count++;
+ adapter->vfinfo[vf].pf_vlan = vlan;
+ adapter->vfinfo[vf].pf_qos = qos;
+ dev_info(&adapter->pdev->dev,
+ "Setting VLAN %d, QOS 0x%x on VF %d\n", vlan, qos, vf);
+ if (test_bit(__IXGBE_DOWN, &adapter->state)) {
+ dev_warn(&adapter->pdev->dev,
+ "The VF VLAN has been set, but the PF device is not up.\n");
+ dev_warn(&adapter->pdev->dev,
+ "Bring the PF device up before attempting to use the VF device.\n");
+ }
+
+out:
+ return err;
+}
+
+static int ixgbe_disable_port_vlan(struct ixgbe_adapter *adapter, int vf)
+{
+ struct ixgbe_hw *hw = &adapter->hw;
+ int err;
+
+ err = ixgbe_set_vf_vlan(adapter, false,
+ adapter->vfinfo[vf].pf_vlan, vf);
+ ixgbe_clear_vmvir(adapter, vf);
+ ixgbe_set_vmolr(hw, vf, true);
+ hw->mac.ops.set_vlan_anti_spoofing(hw, false, vf);
+ if (adapter->vfinfo[vf].vlan_count)
+ adapter->vfinfo[vf].vlan_count--;
+ adapter->vfinfo[vf].pf_vlan = 0;
+ adapter->vfinfo[vf].pf_qos = 0;
+
+ return err;
+}
+
int ixgbe_ndo_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan, u8 qos)
{
int err = 0;
struct ixgbe_adapter *adapter = netdev_priv(netdev);
- struct ixgbe_hw *hw = &adapter->hw;
if ((vf >= adapter->num_vfs) || (vlan > 4095) || (qos > 7))
return -EINVAL;
if (vlan || qos) {
+ /* Check if there is already a port VLAN set, if so
+ * we have to delete the old one first before we
+ * can set the new one. The usage model had
+ * previously assumed the user would delete the
+ * old port VLAN before setting a new one but this
+ * is not necessarily the case.
+ */
if (adapter->vfinfo[vf].pf_vlan)
- err = ixgbe_set_vf_vlan(adapter, false,
- adapter->vfinfo[vf].pf_vlan,
- vf);
- if (err)
- goto out;
- err = ixgbe_set_vf_vlan(adapter, true, vlan, vf);
+ err = ixgbe_disable_port_vlan(adapter, vf);
if (err)
goto out;
- ixgbe_set_vmvir(adapter, vlan, qos, vf);
- ixgbe_set_vmolr(hw, vf, false);
- if (adapter->vfinfo[vf].spoofchk_enabled)
- hw->mac.ops.set_vlan_anti_spoofing(hw, true, vf);
- adapter->vfinfo[vf].vlan_count++;
- adapter->vfinfo[vf].pf_vlan = vlan;
- adapter->vfinfo[vf].pf_qos = qos;
- dev_info(&adapter->pdev->dev,
- "Setting VLAN %d, QOS 0x%x on VF %d\n", vlan, qos, vf);
- if (test_bit(__IXGBE_DOWN, &adapter->state)) {
- dev_warn(&adapter->pdev->dev,
- "The VF VLAN has been set,"
- " but the PF device is not up.\n");
- dev_warn(&adapter->pdev->dev,
- "Bring the PF device up before"
- " attempting to use the VF device.\n");
- }
+ err = ixgbe_enable_port_vlan(adapter, vf, vlan, qos);
} else {
- err = ixgbe_set_vf_vlan(adapter, false,
- adapter->vfinfo[vf].pf_vlan, vf);
- ixgbe_clear_vmvir(adapter, vf);
- ixgbe_set_vmolr(hw, vf, true);
- hw->mac.ops.set_vlan_anti_spoofing(hw, false, vf);
- if (adapter->vfinfo[vf].vlan_count)
- adapter->vfinfo[vf].vlan_count--;
- adapter->vfinfo[vf].pf_vlan = 0;
- adapter->vfinfo[vf].pf_qos = 0;
+ err = ixgbe_disable_port_vlan(adapter, vf);
}
+
out:
return err;
}
--
1.9.3
^ permalink raw reply related
* Re: [patch net-next v2 05/10] rocker: introduce rocker switch driver
From: Thomas Graf @ 2014-11-11 14:29 UTC (permalink / raw)
To: John Fastabend
Cc: Jiri Pirko, netdev, davem, nhorman, andy, dborkman, ogerlitz,
jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
f.fainelli, roopa, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo
In-Reply-To: <5461366A.9050900@gmail.com>
On 11/10/14 at 02:04pm, John Fastabend wrote:
> On 11/09/2014 02:51 AM, Jiri Pirko wrote:
> >+static int rocker_port_sw_parent_id_get(struct net_device *dev,
> >+ struct netdev_phys_item_id *psid)
> >+{
> >+ struct rocker_port *rocker_port = netdev_priv(dev);
> >+ struct rocker *rocker = rocker_port->rocker;
> >+
>
> hmm looks like you read this out of a magic switch register :) but
> my switch doesn't have this magic reg. I suposse the switch MAC address
> should work.
This needs more work afterwards. Either we define that the switch ID
is only unique in combination with the parent ifindex or we need to
introduce a notation of uniquness into the switch ID itself.
Is the goal to expose a hardware ID here to allow identification of
the hardware chip?
MAC is tempting but I'm pretty sure that we'll have pure L3 devices
being handled by this API at some point.
^ permalink raw reply
* Re: [PATCH] drivers: atm: eni: Add pci_dma_mapping_error() call
From: Tina Johnson @ 2014-11-11 14:22 UTC (permalink / raw)
To: David Miller
Cc: tinajohnson.1234, chas, linux-atm-general, netdev, linux-kernel,
julia.lawall
In-Reply-To: <20141110.153228.2187951801277440496.davem@davemloft.net>
On Mon, 10 Nov 2014, David Miller wrote:
> The 'trouble' label assumes that it is recovering and unwinding state
> when an error occurs after the DMA buffer is successfully mapped.
>
> It unconditionally does pci_unmap_single() if 'paddr' is non-zero
> which it might be in the error case depending upon how DMA errors
> are represented on a given platform.
Sorry for the trouble and thankyou for your very helpful comment. I
will send the revised patch.
^ permalink raw reply
* Re: [patch net-next v2 08/10] bridge: add API to notify bridge driver of learned FBD on offloaded device
From: Roopa Prabhu @ 2014-11-11 14:21 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
f.fainelli, linville, jasowang, ebiederm, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl
In-Reply-To: <1415530280-9190-9-git-send-email-jiri@resnulli.us>
On 11/9/14, 2:51 AM, Jiri Pirko wrote:
> From: Scott Feldman <sfeldma@gmail.com>
>
> When the swdev device learns a new mac/vlan on a port, it sends some async
> notification to the driver and the driver installs an FDB in the device.
> To give a holistic system view, the learned mac/vlan should be reflected
> in the bridge's FBD table, so the user, using normal iproute2 cmds, can view
> what is currently learned by the device. This API on the bridge driver gives
> a way for the swdev driver to install an FBD entry in the bridge FBD table.
> (And remove one).
>
> This is equivalent to the device running these cmds:
>
> bridge fdb [add|del] <mac> dev <dev> vid <vlan id> master
>
> This patch needs some extra eyeballs for review, in paricular around the
> locking and contexts.
scott/jiri, love that you have handled this case!, This will be useful.
But, quick question, Cant this also be done using the same ndo_op that
is done to add the static fdb..?
Thanks!.
>
> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> include/linux/if_bridge.h | 18 ++++++++++
> net/bridge/br_fdb.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 102 insertions(+)
>
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index 808dcb8..27ab217 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -37,6 +37,24 @@ extern void brioctl_set(int (*ioctl_hook)(struct net *, unsigned int, void __use
> typedef int br_should_route_hook_t(struct sk_buff *skb);
> extern br_should_route_hook_t __rcu *br_should_route_hook;
>
> +#if IS_ENABLED(CONFIG_BRIDGE)
> +int br_fdb_learn_add(struct net_device *dev,
> + const unsigned char *addr, u16 vid);
> +int br_fdb_learn_del(struct net_device *dev,
> + const unsigned char *addr, u16 vid);
> +#else
> +static inline int br_fdb_learn_add(struct net_device *dev,
> + const unsigned char *addr, u16 vid)
> +{
> + return 0;
> +}
> +static inline int br_fdb_learn_del(struct net_device *dev,
> + const unsigned char *addr, u16 vid)
> +{
> + return 0;
> +}
> +#endif
> +
> #if IS_ENABLED(CONFIG_BRIDGE) && IS_ENABLED(CONFIG_BRIDGE_IGMP_SNOOPING)
> int br_multicast_list_adjacent(struct net_device *dev,
> struct list_head *br_ip_list);
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index f6f8bb5..e02d21b 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -1022,3 +1022,87 @@ void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p)
> }
> }
> }
> +
> +int br_fdb_learn_add(struct net_device *dev, const unsigned char *addr,
> + u16 vid)
> +{
> + struct net_bridge_port *p;
> + struct net_bridge *br;
> + struct hlist_head *head;
> + struct net_bridge_fdb_entry *fdb;
> + int err = 0;
> +
> + rtnl_lock();
> +
> + p = br_port_get_rtnl(dev);
> + if (p == NULL) {
> + pr_info("bridge: %s not a bridge port\n", dev->name);
> + err = -EINVAL;
> + goto err_rtnl_unlock;
> + }
> +
> + br = p->br;
> +
> + spin_lock(&br->hash_lock);
> +
> + head = &br->hash[br_mac_hash(addr, vid)];
> + fdb = fdb_find(head, addr, vid);
> + if (fdb == NULL) {
> + fdb = fdb_create(head, p, addr, vid);
> + if (!fdb) {
> + err = -ENOMEM;
> + goto err_unlock;
> + }
> + fdb->is_local = 1;
> + fdb->used = jiffies;
> + fdb->updated = jiffies;
> + fdb_notify(br, fdb, RTM_NEWNEIGH);
> + } else {
> + err = -EEXIST;
> + }
> +
> +err_unlock:
> + spin_unlock(&br->hash_lock);
> +err_rtnl_unlock:
> + rtnl_unlock();
> +
> + return err;
> +}
> +EXPORT_SYMBOL(br_fdb_learn_add);
> +
> +int br_fdb_learn_del(struct net_device *dev, const unsigned char *addr,
> + u16 vid)
> +{
> + struct net_bridge_port *p;
> + struct net_bridge *br;
> + struct hlist_head *head;
> + struct net_bridge_fdb_entry *fdb;
> + int err = 0;
> +
> + rtnl_lock();
> +
> + p = br_port_get_rtnl(dev);
> + if (p == NULL) {
> + pr_info("bridge: %s not a bridge port\n", dev->name);
> + err = -EINVAL;
> + goto err_rtnl_unlock;
> + }
> +
> + br = p->br;
> +
> + spin_lock(&br->hash_lock);
> +
> + head = &br->hash[br_mac_hash(addr, vid)];
> + fdb = fdb_find(head, addr, vid);
> + if (fdb)
> + fdb_delete(br, fdb);
> + else
> + err = -ENOENT;
> +
> + spin_unlock(&br->hash_lock);
> +err_rtnl_unlock:
> + rtnl_unlock();
> +
> + return err;
> +}
> +EXPORT_SYMBOL(br_fdb_learn_del);
^ permalink raw reply
* Re: [patch net-next v2 03/10] rtnl: expose physical switch id for particular device
From: Roopa Prabhu @ 2014-11-11 13:55 UTC (permalink / raw)
To: Scott Feldman
Cc: Jiri Pirko, Netdev, David S. Miller, nhorman, Andy Gospodarek,
Thomas Graf, dborkman, ogerlitz, jesse, pshelar, azhou, ben,
stephen, Kirsher, Jeffrey T, vyasevic, Cong Wang,
Fastabend, John R, Eric Dumazet, Jamal Hadi Salim,
Florian Fainelli, John Linville, jasowang, ebiederm,
Nicolas Dichtel, ryazanov.s.a, buytenh, aviadr, nbd,
Alexei Starovoitov
In-Reply-To: <CAE4R7bA_u=E-Vm5Zx74NJugV1JgSj1dOmBGkBZG18kcnkXH1OQ@mail.gmail.com>
On 11/10/14, 12:02 PM, Scott Feldman wrote:
> On Mon, Nov 10, 2014 at 7:58 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>> On 11/9/14, 2:51 AM, Jiri Pirko wrote:
>>> The netdevice represents a port in a switch, it will expose
>>> IFLA_PHYS_SWITCH_ID value via rtnl. Two netdevices with the same value
>>> belong to one physical switch.
>>>
>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>> ---
>>> include/uapi/linux/if_link.h | 1 +
>>> net/core/rtnetlink.c | 26 +++++++++++++++++++++++++-
>>> 2 files changed, 26 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>>> index 7072d83..4163753 100644
>>> --- a/include/uapi/linux/if_link.h
>>> +++ b/include/uapi/linux/if_link.h
>>> @@ -145,6 +145,7 @@ enum {
>>> IFLA_CARRIER,
>>> IFLA_PHYS_PORT_ID,
>>> IFLA_CARRIER_CHANGES,
>>> + IFLA_PHYS_SWITCH_ID,
>>
>> Jiri, since we have not really converged on the switchdev class or having a
>> separate switchdev instance,
>> am thinking it is better if we dont expose any such switch_id to userspace
>> yet until absolutely needed. Do you need it today ?
>> There is no real in kernel hw switch driver that will use it today. And
>> quite likely this will need to change when we introduce real hw switch
>> drivers.
> How will it change when real hw switch drivers are introduced? Will
> the real sw driver not be able to give a up unique ID for the switch?
With my question i was trying to see if there are other ways to manage
the relationship between the
switch device and the ports, instead of an random id provided by each
switch driver. Today the switch id namespace seems
to be with each switch driver. On my systems on the first switch chip
quite likely i will choose an id 0.,,and possibly some other
driver will choose the same id. The switch id namespace handling was not
clear to me.
But, to your question, am sure we will have some id to go in there since
the field is now available.
Thanks,
Roopa
^ permalink raw reply
* [PATCH v2 net-next 2/2] net: introduce SO_INCOMING_CPU
From: Eric Dumazet @ 2014-11-11 13:54 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Neal Cardwell, Willem de Bruijn, Ying Cai, Eric Dumazet
In-Reply-To: <1415714068-21028-1-git-send-email-edumazet@google.com>
Alternative to RPS/RFS is to use hardware support for multiple
queues.
Then split a set of million of sockets into worker threads, each
one using epoll() to manage events on its own socket pool.
Ideally, we want one thread per RX/TX queue/cpu, but we have no way to
know after accept() or connect() on which queue/cpu a socket is managed.
We normally use one cpu per RX queue (IRQ smp_affinity being properly
set), so remembering on socket structure which cpu delivered last packet
is enough to solve the problem.
After accept(), connect(), or even file descriptor passing around
processes, applications can use :
int cpu;
socklen_t len = sizeof(cpu);
getsockopt(fd, SOL_SOCKET, SO_INCOMING_CPU, &cpu, &len);
And use this information to put the socket into the right silo
for optimal performance, as all networking stack should run
on the appropriate cpu, without need to send IPI (RPS/RFS).
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
arch/alpha/include/uapi/asm/socket.h | 2 ++
arch/avr32/include/uapi/asm/socket.h | 2 ++
arch/cris/include/uapi/asm/socket.h | 2 ++
arch/frv/include/uapi/asm/socket.h | 2 ++
arch/ia64/include/uapi/asm/socket.h | 2 ++
arch/m32r/include/uapi/asm/socket.h | 2 ++
arch/mips/include/uapi/asm/socket.h | 2 ++
arch/mn10300/include/uapi/asm/socket.h | 2 ++
arch/parisc/include/uapi/asm/socket.h | 2 ++
arch/powerpc/include/uapi/asm/socket.h | 2 ++
arch/s390/include/uapi/asm/socket.h | 2 ++
arch/sparc/include/uapi/asm/socket.h | 2 ++
arch/xtensa/include/uapi/asm/socket.h | 2 ++
include/net/sock.h | 12 ++++++++++++
include/uapi/asm-generic/socket.h | 2 ++
net/core/sock.c | 5 +++++
net/ipv4/tcp_ipv4.c | 1 +
net/ipv4/udp.c | 1 +
net/ipv6/tcp_ipv6.c | 1 +
net/ipv6/udp.c | 1 +
net/sctp/ulpqueue.c | 5 +++--
21 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index 3de1394bcab8..e2fe0700b3b4 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -87,4 +87,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/avr32/include/uapi/asm/socket.h b/arch/avr32/include/uapi/asm/socket.h
index 6e6cd159924b..92121b0f5b98 100644
--- a/arch/avr32/include/uapi/asm/socket.h
+++ b/arch/avr32/include/uapi/asm/socket.h
@@ -80,4 +80,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _UAPI__ASM_AVR32_SOCKET_H */
diff --git a/arch/cris/include/uapi/asm/socket.h b/arch/cris/include/uapi/asm/socket.h
index ed94e5ed0a23..60f60f5b9b35 100644
--- a/arch/cris/include/uapi/asm/socket.h
+++ b/arch/cris/include/uapi/asm/socket.h
@@ -82,6 +82,8 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _ASM_SOCKET_H */
diff --git a/arch/frv/include/uapi/asm/socket.h b/arch/frv/include/uapi/asm/socket.h
index ca2c6e6f31c6..2c6890209ea6 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -80,5 +80,7 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _ASM_SOCKET_H */
diff --git a/arch/ia64/include/uapi/asm/socket.h b/arch/ia64/include/uapi/asm/socket.h
index a1b49bac7951..09a93fb566f6 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -89,4 +89,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h b/arch/m32r/include/uapi/asm/socket.h
index 6c9a24b3aefa..e8589819c274 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -80,4 +80,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index a14baa218c76..2e9ee8c55a10 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -98,4 +98,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h b/arch/mn10300/include/uapi/asm/socket.h
index 6aa3ce1854aa..f3492e8c9f70 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -80,4 +80,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index fe35ceacf0e7..7984a1cab3da 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -79,4 +79,6 @@
#define SO_BPF_EXTENSIONS 0x4029
+#define SO_INCOMING_CPU 0x402A
+
#endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/powerpc/include/uapi/asm/socket.h b/arch/powerpc/include/uapi/asm/socket.h
index a9c3e2e18c05..3474e4ef166d 100644
--- a/arch/powerpc/include/uapi/asm/socket.h
+++ b/arch/powerpc/include/uapi/asm/socket.h
@@ -87,4 +87,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _ASM_POWERPC_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h b/arch/s390/include/uapi/asm/socket.h
index e031332096d7..8457636c33e1 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -86,4 +86,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 54d9608681b6..4a8003a94163 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -76,6 +76,8 @@
#define SO_BPF_EXTENSIONS 0x0032
+#define SO_INCOMING_CPU 0x0033
+
/* Security levels - as per NRL IPv6 - don't actually do anything */
#define SO_SECURITY_AUTHENTICATION 0x5001
#define SO_SECURITY_ENCRYPTION_TRANSPORT 0x5002
diff --git a/arch/xtensa/include/uapi/asm/socket.h b/arch/xtensa/include/uapi/asm/socket.h
index 39acec0cf0b1..c46f6a696849 100644
--- a/arch/xtensa/include/uapi/asm/socket.h
+++ b/arch/xtensa/include/uapi/asm/socket.h
@@ -91,4 +91,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* _XTENSA_SOCKET_H */
diff --git a/include/net/sock.h b/include/net/sock.h
index 7db3db112baa..ff2c3f11fb8f 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -273,6 +273,7 @@ struct cg_proto;
* @sk_rcvtimeo: %SO_RCVTIMEO setting
* @sk_sndtimeo: %SO_SNDTIMEO setting
* @sk_rxhash: flow hash received from netif layer
+ * @sk_incoming_cpu: record cpu processing incoming packets
* @sk_txhash: computed flow hash for use on transmit
* @sk_filter: socket filtering instructions
* @sk_protinfo: private area, net family specific, when not using slab
@@ -350,6 +351,12 @@ struct sock {
#ifdef CONFIG_RPS
__u32 sk_rxhash;
#endif
+ u16 sk_incoming_cpu;
+ /* 16bit hole
+ * Warned : sk_incoming_cpu can be set from softirq,
+ * Do not use this hole without fully understanding possible issues.
+ */
+
__u32 sk_txhash;
#ifdef CONFIG_NET_RX_BUSY_POLL
unsigned int sk_napi_id;
@@ -833,6 +840,11 @@ static inline int sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
return sk->sk_backlog_rcv(sk, skb);
}
+static inline void sk_incoming_cpu_update(struct sock *sk)
+{
+ sk->sk_incoming_cpu = raw_smp_processor_id();
+}
+
static inline void sock_rps_record_flow_hash(__u32 hash)
{
#ifdef CONFIG_RPS
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index ea0796bdcf88..f541ccefd4ac 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -82,4 +82,6 @@
#define SO_BPF_EXTENSIONS 48
+#define SO_INCOMING_CPU 49
+
#endif /* __ASM_GENERIC_SOCKET_H */
diff --git a/net/core/sock.c b/net/core/sock.c
index 15e0c67b1069..14998b161035 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1213,6 +1213,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
v.val = sk->sk_max_pacing_rate;
break;
+ case SO_INCOMING_CPU:
+ v.val = sk->sk_incoming_cpu;
+ break;
+
default:
return -ENOPROTOOPT;
}
@@ -1517,6 +1521,7 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
newsk->sk_err = 0;
newsk->sk_priority = 0;
+ newsk->sk_incoming_cpu = raw_smp_processor_id();
/*
* Before updating sk_refcnt, we must commit prior changes to memory
* (Documentation/RCU/rculist_nulls.txt for details)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 8893598a4124..2c6a955fd5c3 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1663,6 +1663,7 @@ process:
if (sk_filter(sk, skb))
goto discard_and_relse;
+ sk_incoming_cpu_update(sk);
skb->dev = NULL;
bh_lock_sock_nested(sk);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index cd0db5471bb5..52235ca1f352 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1445,6 +1445,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
if (inet_sk(sk)->inet_daddr) {
sock_rps_save_rxhash(sk, skb);
sk_mark_napi_id(sk, skb);
+ sk_incoming_cpu_update(sk);
}
rc = sock_queue_rcv_skb(sk, skb);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index fd8e50b380e7..1985b4933a6b 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1456,6 +1456,7 @@ process:
if (sk_filter(sk, skb))
goto discard_and_relse;
+ sk_incoming_cpu_update(sk);
skb->dev = NULL;
bh_lock_sock_nested(sk);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index f6ba535b6feb..2c7790c9ac65 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -577,6 +577,7 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
if (!ipv6_addr_any(&sk->sk_v6_daddr)) {
sock_rps_save_rxhash(sk, skb);
sk_mark_napi_id(sk, skb);
+ sk_incoming_cpu_update(sk);
}
rc = sock_queue_rcv_skb(sk, skb);
diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
index d49dc2ed30ad..ce469d648ffb 100644
--- a/net/sctp/ulpqueue.c
+++ b/net/sctp/ulpqueue.c
@@ -205,9 +205,10 @@ int sctp_ulpq_tail_event(struct sctp_ulpq *ulpq, struct sctp_ulpevent *event)
if (sock_flag(sk, SOCK_DEAD) || (sk->sk_shutdown & RCV_SHUTDOWN))
goto out_free;
- if (!sctp_ulpevent_is_notification(event))
+ if (!sctp_ulpevent_is_notification(event)) {
sk_mark_napi_id(sk, skb);
-
+ sk_incoming_cpu_update(sk);
+ }
/* Check if the user wishes to receive this event. */
if (!sctp_ulpevent_is_enabled(event, &sctp_sk(sk)->subscribe))
goto out_free;
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH v2 net-next 1/2] tcp: move sk_mark_napi_id() at the right place
From: Eric Dumazet @ 2014-11-11 13:54 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Neal Cardwell, Willem de Bruijn, Ying Cai, Eric Dumazet
In-Reply-To: <1415714068-21028-1-git-send-email-edumazet@google.com>
sk_mark_napi_id() is used to record for a flow napi id of incoming
packets for busypoll sake.
We should do this only on established flows, not on listeners.
This was 'working' by virtue of the socket cloning, but doing
this on SYN packets in unecessary cache line dirtying.
Even if we move sk_napi_id in the same cache line than sk_lock,
we are working to make SYN processing lockless, so it is desirable
to set sk_napi_id only for established flows.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_ipv4.c | 3 ++-
net/ipv6/tcp_ipv6.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 9c7d7621466b..8893598a4124 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1429,6 +1429,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
struct dst_entry *dst = sk->sk_rx_dst;
sock_rps_save_rxhash(sk, skb);
+ sk_mark_napi_id(sk, skb);
if (dst) {
if (inet_sk(sk)->rx_dst_ifindex != skb->skb_iif ||
dst->ops->check(dst, 0) == NULL) {
@@ -1450,6 +1451,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
if (nsk != sk) {
sock_rps_save_rxhash(nsk, skb);
+ sk_mark_napi_id(sk, skb);
if (tcp_child_process(sk, nsk, skb)) {
rsk = nsk;
goto reset;
@@ -1661,7 +1663,6 @@ process:
if (sk_filter(sk, skb))
goto discard_and_relse;
- sk_mark_napi_id(sk, skb);
skb->dev = NULL;
bh_lock_sock_nested(sk);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index ace29b60813c..fd8e50b380e7 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1293,6 +1293,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
struct dst_entry *dst = sk->sk_rx_dst;
sock_rps_save_rxhash(sk, skb);
+ sk_mark_napi_id(sk, skb);
if (dst) {
if (inet_sk(sk)->rx_dst_ifindex != skb->skb_iif ||
dst->ops->check(dst, np->rx_dst_cookie) == NULL) {
@@ -1322,6 +1323,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
*/
if (nsk != sk) {
sock_rps_save_rxhash(nsk, skb);
+ sk_mark_napi_id(sk, skb);
if (tcp_child_process(sk, nsk, skb))
goto reset;
if (opt_skb)
@@ -1454,7 +1456,6 @@ process:
if (sk_filter(sk, skb))
goto discard_and_relse;
- sk_mark_napi_id(sk, skb);
skb->dev = NULL;
bh_lock_sock_nested(sk);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH v2 net-next 0/2] net: SO_INCOMING_CPU support
From: Eric Dumazet @ 2014-11-11 13:54 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Neal Cardwell, Willem de Bruijn, Ying Cai, Eric Dumazet
SO_INCOMING_CPU socket option (read by getsockopt()) provides
an alternative to RPS/RFS for high performance servers using
multi queues NIC.
TCP should use sk_mark_napi_id() for established sockets only.
Eric Dumazet (2):
tcp: move sk_mark_napi_id() at the right place
net: introduce SO_INCOMING_CPU
arch/alpha/include/uapi/asm/socket.h | 2 ++
arch/avr32/include/uapi/asm/socket.h | 2 ++
arch/cris/include/uapi/asm/socket.h | 2 ++
arch/frv/include/uapi/asm/socket.h | 2 ++
arch/ia64/include/uapi/asm/socket.h | 2 ++
arch/m32r/include/uapi/asm/socket.h | 2 ++
arch/mips/include/uapi/asm/socket.h | 2 ++
arch/mn10300/include/uapi/asm/socket.h | 2 ++
arch/parisc/include/uapi/asm/socket.h | 2 ++
arch/powerpc/include/uapi/asm/socket.h | 2 ++
arch/s390/include/uapi/asm/socket.h | 2 ++
arch/sparc/include/uapi/asm/socket.h | 2 ++
arch/xtensa/include/uapi/asm/socket.h | 2 ++
include/net/sock.h | 12 ++++++++++++
include/uapi/asm-generic/socket.h | 2 ++
net/core/sock.c | 5 +++++
net/ipv4/tcp_ipv4.c | 4 +++-
net/ipv4/udp.c | 1 +
net/ipv6/tcp_ipv6.c | 4 +++-
net/ipv6/udp.c | 1 +
net/sctp/ulpqueue.c | 5 +++--
21 files changed, 56 insertions(+), 4 deletions(-)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply
* Re: [PATCH 3/4] i40e: remove pci_assigned_vfs() check while disabling VFs
From: Jeff Kirsher @ 2014-11-11 13:52 UTC (permalink / raw)
To: Sathya Perla
Cc: linux-pci@vger.kernel.org, netdev, ariel.elior, linux.nics,
shahed.shaikh, ddutile
In-Reply-To: <1415620410-4937-4-git-send-email-sathya.perla@emulex.com>
On Mon, Nov 10, 2014 at 3:53 AM, Sathya Perla <sathya.perla@emulex.com> wrote:
> From: Vasundhara Volam <vasundhara.volam@emulex.com>
>
> The pci_assigned_vfs() check (while disabling VFs) is being moved to the
> pci-sysfs.c file and will be done before invoking sriov_configure().
>
> Signed-off-by: Vasundhara Volam <vasundhara.volam@emulex.com>
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 7 +------
> 1 files changed, 1 insertions(+), 6 deletions(-)
Thanks I will add your patch to my queue.
^ permalink raw reply
* Re: [PATCH -next 2/2] seq_putc: Convert to return void and convert uses too.
From: Petr Mladek @ 2014-11-11 13:47 UTC (permalink / raw)
To: Joe Perches
Cc: Steven Rostedt, Corey Minyard, Alexander Viro, Pablo Neira Ayuso,
Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
openipmi-developer, linux-kernel, linux-fsdevel, netfilter-devel,
coreteam, netdev
In-Reply-To: <56c4c0d5ec721134cff4913e5e3f8923169c35ef.1415645477.git.joe@perches.com>
On Mon 2014-11-10 10:58:57, Joe Perches wrote:
> Using the return value of seq_putc is error-prone, so
> make it return void instead.
>
> Reverse the logic in seq_putc to make it like seq_puts.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Reviewed-by: Petr Mladek <pmladek@suse.cz>
The changes are correct. The show() functions should return 0
even when there is an overflow. They are called by traverse()
from seq_read() that might increase the buffer size and try again.
Best Regards,
Petr
^ permalink raw reply
* Re: [patch iproute2] tc: add support for vlan tc action
From: Jiri Pirko @ 2014-11-11 13:46 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, davem, pshelar, therbert, edumazet, willemb, dborkman,
mst, fw, Paul.Durrant, tgraf, Stephen Hemminger
In-Reply-To: <546210EA.7090201@mojatatu.com>
Tue, Nov 11, 2014 at 02:36:42PM CET, jhs@mojatatu.com wrote:
>
>Ive run out of time - but will test; so far looks good.
>Attached a small patchlet on top of yours.
Will squash it in and send v2.
>
>cheers,
>jamal
>diff --git a/tc/m_vlan.c b/tc/m_vlan.c
>index 54c0ce7..2755e20 100644
>--- a/tc/m_vlan.c
>+++ b/tc/m_vlan.c
>@@ -13,6 +13,7 @@
> #include <stdlib.h>
> #include <unistd.h>
> #include <string.h>
>+#include <linux/if_ether.h>
> #include "utils.h"
> #include "rt_names.h"
> #include "tc_util.h"
>@@ -22,6 +23,8 @@ static void explain(void)
> {
> fprintf(stderr, "Usage: vlan pop\n");
> fprintf(stderr, " vlan push [ protocol VLANPROTO ] id VLANID\n");
>+ fprintf(stderr, " VLANPROTO is one of 802.1Q or 802.1ad\n");
>+ fprintf(stderr, " with default: 802.1Q\n");
> }
>
> static void usage(void)
>@@ -121,7 +124,7 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
> if (matches(*argv, "index") == 0) {
> NEXT_ARG();
> if (get_u32(&parm.index, *argv, 10)) {
>- fprintf(stderr, "Pedit: Illegal \"index\"\n");
>+ fprintf(stderr, "vlan: Illegal \"index\"\n");
> return -1;
> }
> argc--;
>@@ -141,8 +144,15 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
> addattr_l(n, MAX_MSG, TCA_VLAN_PARMS, &parm, sizeof(parm));
> if (id_set)
> addattr_l(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_ID, &id, 2);
>- if (proto_set)
>+ if (proto_set) {
>+ if (proto != ETH_P_8021Q && proto != ETH_P_8021AD) {
>+ fprintf(stderr, "protocol id 0x%x not supported\n", proto);
>+ explain();
>+ return -1;
>+ }
>+
> addattr_l(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_PROTOCOL, &proto, 2);
>+ }
> tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
>
> *argc_p = argc;
^ permalink raw reply
* Re: [patch iproute2] tc: add support for vlan tc action
From: Jamal Hadi Salim @ 2014-11-11 13:36 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, pshelar, therbert, edumazet, willemb, dborkman, mst, fw,
Paul.Durrant, tgraf, Stephen Hemminger
In-Reply-To: <1415700966-9225-1-git-send-email-jiri@resnulli.us>
[-- Attachment #1: Type: text/plain, Size: 115 bytes --]
Ive run out of time - but will test; so far looks good.
Attached a small patchlet on top of yours.
cheers,
jamal
[-- Attachment #2: patch-for-jiri --]
[-- Type: text/plain, Size: 1493 bytes --]
diff --git a/tc/m_vlan.c b/tc/m_vlan.c
index 54c0ce7..2755e20 100644
--- a/tc/m_vlan.c
+++ b/tc/m_vlan.c
@@ -13,6 +13,7 @@
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
+#include <linux/if_ether.h>
#include "utils.h"
#include "rt_names.h"
#include "tc_util.h"
@@ -22,6 +23,8 @@ static void explain(void)
{
fprintf(stderr, "Usage: vlan pop\n");
fprintf(stderr, " vlan push [ protocol VLANPROTO ] id VLANID\n");
+ fprintf(stderr, " VLANPROTO is one of 802.1Q or 802.1ad\n");
+ fprintf(stderr, " with default: 802.1Q\n");
}
static void usage(void)
@@ -121,7 +124,7 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
if (matches(*argv, "index") == 0) {
NEXT_ARG();
if (get_u32(&parm.index, *argv, 10)) {
- fprintf(stderr, "Pedit: Illegal \"index\"\n");
+ fprintf(stderr, "vlan: Illegal \"index\"\n");
return -1;
}
argc--;
@@ -141,8 +144,15 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
addattr_l(n, MAX_MSG, TCA_VLAN_PARMS, &parm, sizeof(parm));
if (id_set)
addattr_l(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_ID, &id, 2);
- if (proto_set)
+ if (proto_set) {
+ if (proto != ETH_P_8021Q && proto != ETH_P_8021AD) {
+ fprintf(stderr, "protocol id 0x%x not supported\n", proto);
+ explain();
+ return -1;
+ }
+
addattr_l(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_PROTOCOL, &proto, 2);
+ }
tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
*argc_p = argc;
^ permalink raw reply related
* Re: [Patch net-next] net: remove netif_set_real_num_rx_queues()
From: Edward Cree @ 2014-11-11 13:10 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Eric Dumazet, David S. Miller
In-Reply-To: <1415648136-4167-1-git-send-email-xiyou.wangcong@gmail.com>
On 10/11/14 19:35, Cong Wang wrote:
> vlan was the only user of netif_set_real_num_rx_queues(),
> but it no longer calls it after
> commit 4af429d29b341bb1735f04c2fb960178 ("vlan: lockless transmit path").
> So we can just remove it.
>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>
> ---
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 888d551..4a6f770 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2762,23 +2762,6 @@ static inline int netif_set_real_num_rx_queues(struct net_device *dev,
> }
> #endif
>
> -static inline int netif_copy_real_num_queues(struct net_device *to_dev,
> - const struct net_device *from_dev)
Patch title says you're removing _set_ but body only removes _copy_.
Which one is right?
> -{
> - int err;
> -
> - err = netif_set_real_num_tx_queues(to_dev,
> - from_dev->real_num_tx_queues);
> - if (err)
> - return err;
> -#ifdef CONFIG_SYSFS
> - return netif_set_real_num_rx_queues(to_dev,
> - from_dev->real_num_rx_queues);
> -#else
> - return 0;
> -#endif
> -}
> -
> #ifdef CONFIG_SYSFS
> static inline unsigned int get_netdev_rx_queue_index(
> struct netdev_rx_queue *queue)
> --
> 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 net-next 1/2] net: move vlan pop/push functions into common code
From: Eric Dumazet @ 2014-11-11 13:06 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, jhs, pshelar, therbert, edumazet, willemb,
dborkman, mst, fw, Paul.Durrant, tgraf
In-Reply-To: <1415700789-9171-1-git-send-email-jiri@resnulli.us>
On Tue, 2014-11-11 at 11:13 +0100, Jiri Pirko wrote:
> So it can be used from out of openvswitch code.
> Did couple of cosmetic changes on the way, namely variable naming and
> adding support for 8021AD proto.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> include/linux/skbuff.h | 2 ++
> net/core/skbuff.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++
> net/openvswitch/actions.c | 76 ++---------------------------------------
> 3 files changed, 90 insertions(+), 74 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 103fbe8..3b0445c 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2673,6 +2673,8 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet);
> unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
> struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
> struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
> +int skb_vlan_pop(struct sk_buff *skb);
> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
>
> struct skb_checksum_ops {
> __wsum (*update)(const void *mem, int len, __wsum wsum);
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 7001896..75e53d4 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4151,6 +4151,92 @@ err_free:
> }
> EXPORT_SYMBOL(skb_vlan_untag);
>
> +/* remove VLAN header from packet and update csum accordingly. */
> +static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
> +{
> + struct vlan_hdr *vhdr;
> + int err;
> +
> + if (!pskb_may_pull(skb, VLAN_ETH_HLEN))
> + return -ENOMEM;
> +
> + if (!skb_cloned(skb) || skb_clone_writable(skb, VLAN_ETH_HLEN))
> + return 0;
> +
> + err = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
> + if (unlikely(err))
> + return err;
All this should be a function of its own (OVS calls this
make_writable()).
Its too bad netfilter has a different skb_make_writable()
> +
> + if (skb->ip_summed == CHECKSUM_COMPLETE)
> + skb->csum = csum_sub(skb->csum, csum_partial(skb->data
> + + (2 * ETH_ALEN), VLAN_HLEN, 0));
This looks like skb_postpull_rcsum()
BTW, calling csum_partial() for 4 bytes is quite expensive.
^ permalink raw reply
* Re: [patch iproute2] tc: add support for vlan tc action
From: Jamal Hadi Salim @ 2014-11-11 12:35 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, pshelar, therbert, edumazet, willemb, dborkman, mst, fw,
Paul.Durrant, tgraf
In-Reply-To: <1415700966-9225-1-git-send-email-jiri@resnulli.us>
On 11/11/14 05:16, Jiri Pirko wrote:
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Looks good.
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply
* Re: [patch net-next 2/2] sched: introduce vlan action
From: Jamal Hadi Salim @ 2014-11-11 12:34 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, pshelar, therbert, edumazet, willemb, dborkman, mst, fw,
Paul.Durrant, tgraf
In-Reply-To: <1415700789-9171-2-git-send-email-jiri@resnulli.us>
On 11/11/14 05:13, Jiri Pirko wrote:
> This tc action allows to work with vlan tagged skbs. Two supported
> sub-actions are header pop and header push.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Looks good. I am going to test it - but dont see any obvious issues I
from inspection; willing to say:
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
cheers,
jamal
^ permalink raw reply
* Re: 802.3x pause frames
From: Ben Hutchings @ 2014-11-11 12:29 UTC (permalink / raw)
To: Julia Niewiejska; +Cc: netdev
In-Reply-To: <545B70C9.7090702@fkie.fraunhofer.de>
[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]
On Thu, 2014-11-06 at 13:59 +0100, Julia Niewiejska wrote:
> Hello,
>
> I indend to do some experiments with 802.3x pause frames, but I have yet
> to find a setup that works. I used a tool that generates pause frames
> [1] in two setups. First one consists of two virtual machines on a
> VMWare ESXi 5.5 server connected with each other through a virtual
> switch. In second setup two physical machines (Desktop PCs) were
> connected directly by an ethernet cable. The VMs are running Debian
> Wheezy 32bit while Debian Testing (Jessie) 64bit is installed on both
> desktop PCs. Below you will find some information on the ethernet
> adapters and drivers used in the VM [2], the desktop PC with the more up
> to date hardware [3] and the older machine [4]. I used the following
> command to activate flow control:
>
> ethtool -A <eth> autoneg off rx on tx on
>
> First of all I noticed some discrepancies between the output of
> mii-tools or ethtool -a and the attempt to change the flow control
> settings with the command above. Only the Realtek adapter actually
> output that it didn't support the operation, the other adapters accepted
> the settings without any error message.
>
> In both setups pause frames were generated on one machine while a ping
> was sent simultaneously, as suggested in [1]. While the VM connection
> was set to 1 Gbps the whole time, I also tested a 10 Mbps setting on the
> physical connection. Even though the pause frames were always visible in
> tcpdump at the receiver, I didn't notice any influence whatsoever in the
> ping results.
[...]
MACs that implement flow control will not pass pause frames to the host;
therefore these MACs do not implement flow control.
Ben.
--
Ben Hutchings
Experience is directly proportional to the value of equipment destroyed.
- Carolyn Scheppner
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply
* Re: [PATCH net] net: sctp: fix memory leak in auth key management
From: Neil Horman @ 2014-11-11 12:06 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, linux-sctp, netdev, Vlad Yasevich
In-Reply-To: <1415638809-20288-1-git-send-email-dborkman@redhat.com>
On Mon, Nov 10, 2014 at 06:00:09PM +0100, Daniel Borkmann wrote:
> A very minimal and simple user space application allocating an SCTP
> socket, setting SCTP_AUTH_KEY setsockopt(2) on it and then closing
> the socket again will leak the memory containing the authentication
> key from user space:
>
> unreferenced object 0xffff8800837047c0 (size 16):
> comm "a.out", pid 2789, jiffies 4296954322 (age 192.258s)
> hex dump (first 16 bytes):
> 01 00 00 00 04 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffff816d7e8e>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811c88d8>] __kmalloc+0xe8/0x270
> [<ffffffffa0870c23>] sctp_auth_create_key+0x23/0x50 [sctp]
> [<ffffffffa08718b1>] sctp_auth_set_key+0xa1/0x140 [sctp]
> [<ffffffffa086b383>] sctp_setsockopt+0xd03/0x1180 [sctp]
> [<ffffffff815bfd94>] sock_common_setsockopt+0x14/0x20
> [<ffffffff815beb61>] SyS_setsockopt+0x71/0xd0
> [<ffffffff816e58a9>] system_call_fastpath+0x12/0x17
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> This is bad because of two things, we can bring down a machine from
> user space when auth_enable=1, but also we would leave security sensitive
> keying material in memory without clearing it after use. The issue is
> that sctp_auth_create_key() already sets the refcount to 1, but after
> allocation sctp_auth_set_key() does an additional refcount on it, and
> thus leaving it around when we free the socket.
>
> Fixes: 65b07e5d0d0 ("[SCTP]: API updates to suport SCTP-AUTH extensions.")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> ---
> net/sctp/auth.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index 0e85291..fb7976a 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -862,8 +862,6 @@ int sctp_auth_set_key(struct sctp_endpoint *ep,
> list_add(&cur_key->key_list, sh_keys);
>
> cur_key->key = key;
> - sctp_auth_key_hold(key);
> -
> return 0;
> nomem:
> if (!replace)
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH net] net: sctp: fix NULL pointer dereference in af->from_addr_param on malformed packet
From: Neil Horman @ 2014-11-11 12:05 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, linux-sctp, netdev, Vlad Yasevich
In-Reply-To: <1415638466-20176-1-git-send-email-dborkman@redhat.com>
On Mon, Nov 10, 2014 at 05:54:26PM +0100, Daniel Borkmann wrote:
> An SCTP server doing ASCONF will panic on malformed INIT ping-of-death
> in the form of:
>
> ------------ INIT[PARAM: SET_PRIMARY_IP] ------------>
>
> While the INIT chunk parameter verification dissects through many things
> in order to detect malformed input, it misses to actually check parameters
> inside of parameters. E.g. RFC5061, section 4.2.4 proposes a 'set primary
> IP address' parameter in ASCONF, which has as a subparameter an address
> parameter.
>
> So an attacker may send a parameter type other than SCTP_PARAM_IPV4_ADDRESS
> or SCTP_PARAM_IPV6_ADDRESS, param_type2af() will subsequently return 0
> and thus sctp_get_af_specific() returns NULL, too, which we then happily
> dereference unconditionally through af->from_addr_param().
>
> The trace for the log:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000078
> IP: [<ffffffffa01e9c62>] sctp_process_init+0x492/0x990 [sctp]
> PGD 0
> Oops: 0000 [#1] SMP
> [...]
> Pid: 0, comm: swapper Not tainted 2.6.32-504.el6.x86_64 #1 Bochs Bochs
> RIP: 0010:[<ffffffffa01e9c62>] [<ffffffffa01e9c62>] sctp_process_init+0x492/0x990 [sctp]
> [...]
> Call Trace:
> <IRQ>
> [<ffffffffa01f2add>] ? sctp_bind_addr_copy+0x5d/0xe0 [sctp]
> [<ffffffffa01e1fcb>] sctp_sf_do_5_1B_init+0x21b/0x340 [sctp]
> [<ffffffffa01e3751>] sctp_do_sm+0x71/0x1210 [sctp]
> [<ffffffffa01e5c09>] ? sctp_endpoint_lookup_assoc+0xc9/0xf0 [sctp]
> [<ffffffffa01e61f6>] sctp_endpoint_bh_rcv+0x116/0x230 [sctp]
> [<ffffffffa01ee986>] sctp_inq_push+0x56/0x80 [sctp]
> [<ffffffffa01fcc42>] sctp_rcv+0x982/0xa10 [sctp]
> [<ffffffffa01d5123>] ? ipt_local_in_hook+0x23/0x28 [iptable_filter]
> [<ffffffff8148bdc9>] ? nf_iterate+0x69/0xb0
> [<ffffffff81496d10>] ? ip_local_deliver_finish+0x0/0x2d0
> [<ffffffff8148bf86>] ? nf_hook_slow+0x76/0x120
> [<ffffffff81496d10>] ? ip_local_deliver_finish+0x0/0x2d0
> [...]
>
> A minimal way to address this is to check for NULL as we do on all
> other such occasions where we know sctp_get_af_specific() could
> possibly return with NULL.
>
> Fixes: d6de3097592b ("[SCTP]: Add the handling of "Set Primary IP Address" parameter to INIT")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> ---
> net/sctp/sm_make_chunk.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index ab734be..9f32741 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2609,6 +2609,9 @@ do_addr_param:
> addr_param = param.v + sizeof(sctp_addip_param_t);
>
> af = sctp_get_af_specific(param_type2af(param.p->type));
> + if (af == NULL)
> + break;
> +
> af->from_addr_param(&addr, addr_param,
> htons(asoc->peer.port), 0);
>
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH] smsc911x: power-up phydev before doing a software reset.
From: Javier Martinez Canillas @ 2014-11-11 11:20 UTC (permalink / raw)
To: Enric Balletbo i Serra; +Cc: netdev, Steve Glendinning, Enrico Butera
In-Reply-To: <1415643789-5367-1-git-send-email-eballetbo@iseebcn.com>
Hello Enric,
On Mon, Nov 10, 2014 at 7:23 PM, Enric Balletbo i Serra
<eballetbo@iseebcn.com> wrote:
> With commit be9dad1f9f26604fb ("net: phy: suspend phydev when going
> to HALTED"), the PHY device will be put in a low-power mode using
> BMCR_PDOWN if the the interface is set down. The smsc911x driver does
> a software_reset opening the device driver (ndo_open). In such case,
> the PHY must be powered-up before access to any register and before
> calling the software_reset function. Otherwise, as the PHY is powered
> down the software reset fails and the interface can not be enabled
> again.
>
The patch looks good to me, I just have a small comment.
> +
> + /* If the PHY general power-down bit is not set is not necessary to
> + * disable the general power down-mode.
> + */
> + if (rc & BMCR_PDOWN) {
> + rc = phy_write(pdata->phy_dev, MII_BMCR, rc & ~BMCR_PDOWN);
> + if (rc < 0) {
> + SMSC_WARN(pdata, drv, "Failed writing PHY control reg");
> + return rc;
> + }
> +
> + mdelay(1);
According to Documentation/timers/timers-howto.txt, the *delay()
family of functions should only be used in atomic context since those
are implemented as a busy-wait loop to achieve the desired delay.
AFAICT smsc911x_soft_reset() is only called from process context so
usleep_range() should be used instead.
Best regards,
Javier
^ permalink raw reply
* Re: [PATCH] brcmfmac: unlink URB when request timed out
From: Arend van Spriel @ 2014-11-11 11:05 UTC (permalink / raw)
To: Mathy Vanhoef, brudley, frankyl, meuleman, linville, pieterpg,
linux-wireless, brcm80211-dev-list, netdev, Oliver Neukum
In-Reply-To: <5460E2FA.40201@gmail.com>
On 10-11-14 17:08, Mathy Vanhoef wrote:
> On 11/10/2014 06:18 AM, Arend van Spriel wrote:
>> On 09-11-14 19:10, Mathy Vanhoef wrote:
>>> From: Mathy Vanhoef <vanhoefm@gmail.com>
>>>
>>> Unlink the submitted URB in brcmf_usb_dl_cmd if the request timed out. This
>>> assures the URB is never submitted twice, preventing a driver crash.
>>
>> Hi Mathy,
>>
>> What driver crash are you referring to? The log only shows the WARNING
>> ending in a USB disconnect but no actual crash. Does your patch get the
>> driver running properly or does it only avoid the warning.
>
> Hi Arend,
>
> It shows a warning, after which the device doesn't work (but the computer is
> still usable). But I've noticed that when *unplugging* the USB cable the OS may
> freeze. This doesn't always happen though, sometimes unplugging works OK. The
> patch both avoids the warning, and gets the device/driver running properly
> (unplugging also works OK).
>
>>
>> With that said, it seems there is some need for improvement, but I also
>> notice you are running this on a virtual machine so could that affect
>> the timeout to kick in before completion. Could you try to increase the
>> timeout. Still when a timeout occurs this needs to be handled properly.
>> Could you also try the following patch?
>
> I did a few additional tests:
>
> 1. When increasing IOCTL_RESP_TIMEOUT to 20000 (ten times the normal value) the
> timeout and warning still occur. Device/driver doesn't work.
> 2. When increasing BRCMF_USB_RESET_GETVER_SPINWAIT to 1000 (ten timers the
> normal value) everything works. Device/driver works.
This means the ctl_urb completes on your system within 3sec, but not
within 2.1sec. After discussing this with my colleague, we think you
should use usb_kill_urb() instead of usb_unlink_urb() as it assures the
completion handler is called. Could you retest that and let us know.
Regards,
Arend
> 3. Quick test using backports-3.18-rc1-1 (aka unpatched driver) on a non-
> virtualized Linux install: In that case everything worked fine. So the bug
> may only be triggered in a virtualized environment / VMWare.
> 4. When applying your patch, the driver stops early during initialization of
> the device. I included a WARN_ONCE before returning EINVAL and got the
> output below.
>
> Kind regards,
> Mathy
> ---
> [ 220.955647] usb 1-1: new high-speed USB device number 3 using ehci-pci
> [ 221.487797] usb 1-1: New USB device found, idVendor=043e, idProduct=3004
> [ 221.487802] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> [ 221.487804] usb 1-1: Product: Remote Download Wireless Adapter
> [ 221.487806] usb 1-1: Manufacturer: Broadcom
> [ 221.487808] usb 1-1: SerialNumber: 000000000001
> [ 221.490472] brcmfmac: brcmf_usb_probe Enter 0x043e:0x3004
> [ 221.490476] brcmfmac: brcmf_usb_probe Broadcom high speed USB WLAN interface detected
> [ 221.490477] brcmfmac: brcmf_usb_probe_cb Enter
> [ 221.490480] brcmfmac: brcmf_usb_attach Enter
> [ 221.490494] brcmfmac: brcmf_usb_dlneeded Enter
> [ 221.490495] ------------[ cut here ]------------
> [ 221.490503] WARNING: CPU: 0 PID: 100 at drivers/net/wireless/brcm80211/brcmfmac/usb.c:716 brcmf_usb_dl_cmd+0x75/0x1a0 [brcmfmac]()
> [ 221.490505] EINVAL devinfo=c0044000 ctl_rub=ef898380 completed=0
> [ 221.490506] Modules linked in: brcmfmac brcmutil vmw_pvscsi pcnet32 mptspi mptscsih mptbase
> [ 221.490514] CPU: 0 PID: 100 Comm: kworker/0:1 Not tainted 3.18.0-rc3-wl+ #2
> [ 221.490515] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/31/2013
> [ 221.490528] Workqueue: usb_hub_wq hub_event
> [ 221.490530] 00000000 00000000 eecffb58 c1711f4a eecffb98 eecffb88 c103edaf f11cbc58
> [ 221.490534] eecffbb4 00000064 f11cbc84 000002cc f11c1595 f11c1595 c0044000 ffffffea
> [ 221.490537] ef726000 eecffba0 c103ee4e 00000009 eecffb98 f11cbc58 eecffbb4 eecffbd0
> [ 221.490541] Call Trace:
> [ 221.490550] [<c1711f4a>] dump_stack+0x41/0x52
> [ 221.490558] [<c103edaf>] warn_slowpath_common+0x7f/0xa0
> [ 221.490563] [<f11c1595>] ? brcmf_usb_dl_cmd+0x75/0x1a0 [brcmfmac]
> [ 221.490567] [<f11c1595>] ? brcmf_usb_dl_cmd+0x75/0x1a0 [brcmfmac]
> [ 221.490570] [<c103ee4e>] warn_slowpath_fmt+0x2e/0x30
> [ 221.490575] [<f11c1595>] brcmf_usb_dl_cmd+0x75/0x1a0 [brcmfmac]
> [ 221.490580] [<f11c2cd8>] brcmf_usb_probe+0x3c8/0x640 [brcmfmac]
> [ 221.490583] [<c1717d53>] ? mutex_lock+0x13/0x32
> [ 221.490586] [<c1493ae3>] usb_probe_interface+0xa3/0x180
> [ 221.490590] [<c13f5690>] ? __driver_attach+0x90/0x90
> [ 221.490592] [<c13f546e>] driver_probe_device+0x5e/0x1f0
> [ 221.490595] [<c13f5690>] ? __driver_attach+0x90/0x90
> [ 221.490597] [<c13f56c9>] __device_attach+0x39/0x50
> [ 221.490600] [<c13f3d84>] bus_for_each_drv+0x34/0x70
> [ 221.490602] [<c13f53db>] device_attach+0x7b/0x90
> [ 221.490604] [<c13f5690>] ? __driver_attach+0x90/0x90
> [ 221.490607] [<c13f4b8f>] bus_probe_device+0x6f/0x90
> [ 221.490609] [<c13f3256>] device_add+0x426/0x520
> [ 221.490611] [<c1491503>] ? usb_control_msg+0xb3/0xd0
> [ 221.490614] [<c1717d53>] ? mutex_lock+0x13/0x32
> [ 221.490627] [<c14922f8>] usb_set_configuration+0x3f8/0x700
> [ 221.490630] [<c13f5690>] ? __driver_attach+0x90/0x90
> [ 221.490633] [<c149ac7b>] generic_probe+0x2b/0x90
> [ 221.490637] [<c1188bc0>] ? sysfs_create_link+0x20/0x40
> [ 221.490639] [<c1492bec>] usb_probe_device+0xc/0x10
> [ 221.490641] [<c13f546e>] driver_probe_device+0x5e/0x1f0
> [ 221.490644] [<c13f5690>] ? __driver_attach+0x90/0x90
> [ 221.490646] [<c13f56c9>] __device_attach+0x39/0x50
> [ 221.490649] [<c13f3d84>] bus_for_each_drv+0x34/0x70
> [ 221.490651] [<c13f53db>] device_attach+0x7b/0x90
> [ 221.490653] [<c13f5690>] ? __driver_attach+0x90/0x90
> [ 221.490656] [<c13f4b8f>] bus_probe_device+0x6f/0x90
> [ 221.490658] [<c13f3256>] device_add+0x426/0x520
> [ 221.490661] [<c148aa2e>] ? usb_new_device+0x16e/0x3a0
> [ 221.490663] [<c148aad7>] usb_new_device+0x217/0x3a0
> [ 221.490666] [<c148bff7>] hub_event+0xa17/0xda0
> [ 221.490668] [<c1716918>] ? __schedule+0x2f8/0x710
> [ 221.490672] [<c105127c>] ? pwq_dec_nr_in_flight+0x3c/0x90
> [ 221.490674] [<c10513ee>] process_one_work+0x11e/0x360
> [ 221.490677] [<c1051750>] worker_thread+0xf0/0x3c0
> [ 221.490680] [<c106e14a>] ? __wake_up_locked+0x1a/0x20
> [ 221.490682] [<c1051660>] ? process_scheduled_works+0x30/0x30
> [ 221.490685] [<c1055b56>] kthread+0x96/0xb0
> [ 221.490687] [<c1050000>] ? put_unbound_pool+0x110/0x170
> [ 221.490691] [<c1719c81>] ret_from_kernel_thread+0x21/0x30
> [ 221.490693] [<c1055ac0>] ? kthread_worker_fn+0x110/0x110
> [ 221.490695] ---[ end trace 9befd914693f3083 ]---
> [ 221.490697] brcmfmac: brcmf_usb_dlneeded chip 57005 rev 0xf11cfcec
> [ 221.490699] brcmfmac: brcmf_fw_get_firmwares enter: dev=1-1
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
> index 5265aa7..15b1aa7 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
> @@ -709,8 +709,13 @@ static int brcmf_usb_dl_cmd(struct brcmf_usbdev_info *devinfo, u8 cmd,
> char *tmpbuf;
> u16 size;
>
> - if ((!devinfo) || (devinfo->ctl_urb == NULL))
> + if (!devinfo || !devinfo->ctl_urb || !devinfo->ctl_completed) {
> + WARN_ONCE(1, "EINVAL devinfo=%p ctl_rub=%p completed=%d\n",
> + devinfo,
> + devinfo ? devinfo->ctl_urb : NULL,
> + devinfo ? devinfo->ctl_completed : -1);
> return -EINVAL;
> + }
>
> tmpbuf = kmalloc(buflen, GFP_ATOMIC);
> if (!tmpbuf)
>
>
>>
>> Regards,
>> Arend
>> ---
>> drivers/net/wireless/brcm80211/brcmfmac/usb.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>> b/drivers/net/wireles
>> index dc13591..786c40b 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>> @@ -640,7 +640,7 @@ static int brcmf_usb_dl_cmd(struct brcmf_usbdev_info
>> *devinf
>> char *tmpbuf;
>> u16 size;
>>
>> - if ((!devinfo) || (devinfo->ctl_urb == NULL))
>> + if (!devinfo || !devinfo->ctl_urb || !devinfo->ctl_completed)
>> return -EINVAL;
>>
>> tmpbuf = kmalloc(buflen, GFP_ATOMIC);
>>
>>> Signed-off-by: Mathy Vanhoef <vanhoefm@gmail.com>
>>> ---
>>> Currently brcmfmac may crash when a USB device is attached (tested with a LG
>>> TWFM-B003D). In particular it fails on the second call to brcmf_usb_dl_cmd in
>>> the while loop of brcmf_usb_resetcfg. The problem is that an URB is being
>>> submitted twice:
>>>
>>> [ 169.861800] brcmfmac: brcmf_usb_dl_writeimage Enter, fw f14db000, len 348160
>>> [ 171.787791] brcmfmac: brcmf_usb_dl_writeimage Exit, err=0
>>> [ 171.787797] brcmfmac: brcmf_usb_dlstart Exit, err=0
>>> [ 171.787799] brcmfmac: brcmf_usb_dlrun Enter
>>> [ 171.791794] brcmfmac: brcmf_usb_resetcfg Enter
>>> [ 173.988072] ------------[ cut here ]------------
>>> [ 173.988083] WARNING: CPU: 0 PID: 369 at drivers/usb/core/urb.c:339 usb_submit_urb+0x4e6/0x500()
>>> [ 173.988085] URB eaf45f00 submitted while active
>>> [ 173.988086] Modules linked in: brcmfmac brcmutil vmw_pvscsi pcnet32 mptspi mptscsih mptbase
>>> [ 173.988100] CPU: 0 PID: 369 Comm: kworker/0:2 Not tainted 3.18.0-rc3-wl #1
>>> [ 173.988102] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/31/2013
>>> [ 173.988106] Workqueue: events request_firmware_work_func
>>> [ 173.988108] 00000000 00000000 ee747db8 c1711f4a ee747df8 ee747de8 c103edaf c18d1e10
>>> [ 173.988112] ee747e14 00000171 c18a8b29 00000153 c1490556 c1490556 eaf45f00 eafdc660
>>> [ 173.988115] f14b8fa0 ee747e00 c103ee4e 00000009 ee747df8 c18d1e10 ee747e14 ee747e50
>>> [ 173.988119] Call Trace:
>>> [ 173.988129] [<c1711f4a>] dump_stack+0x41/0x52
>>> [ 173.988136] [<c103edaf>] warn_slowpath_common+0x7f/0xa0
>>> [ 173.988139] [<c1490556>] ? usb_submit_urb+0x4e6/0x500
>>> [ 173.988141] [<c1490556>] ? usb_submit_urb+0x4e6/0x500
>>> [ 173.988147] [<f14b8fa0>] ? brcmf_usb_ioctl_resp_wake+0x40/0x40 [brcmfmac]
>>> [ 173.988150] [<c103ee4e>] warn_slowpath_fmt+0x2e/0x30
>>> [ 173.988152] [<c1490556>] usb_submit_urb+0x4e6/0x500
>>> [ 173.988156] [<c1123de1>] ? __kmalloc+0x21/0x140
>>> [ 173.988161] [<f14b91c3>] ? brcmf_usb_dl_cmd+0x33/0x120 [brcmfmac]
>>> [ 173.988166] [<f14b9243>] brcmf_usb_dl_cmd+0xb3/0x120 [brcmfmac]
>>> [ 173.988170] [<f14ba6c4>] brcmf_usb_probe_phase2+0x4e4/0x640 [brcmfmac]
>>> [ 173.988176] [<f14b4900>] brcmf_fw_request_code_done+0xd0/0xf0 [brcmfmac]
>>> [ 173.988178] [<c1400876>] request_firmware_work_func+0x26/0x50
>>> [ 173.988182] [<c10513ee>] process_one_work+0x11e/0x360
>>> [ 173.988184] [<c1051750>] worker_thread+0xf0/0x3c0
>>> [ 173.988205] [<c106e14a>] ? __wake_up_locked+0x1a/0x20
>>> [ 173.988208] [<c1051660>] ? process_scheduled_works+0x30/0x30
>>> [ 173.988211] [<c1055b56>] kthread+0x96/0xb0
>>> [ 173.988214] [<c1719c81>] ret_from_kernel_thread+0x21/0x30
>>> [ 173.988217] [<c1055ac0>] ? kthread_worker_fn+0x110/0x110
>>> [ 173.988219] ---[ end trace 0c88bf46801de083 ]---
>>> [ 173.988221] brcmf_usb_dl_cmd: usb_submit_urb failed -16
>>> [ 173.988396] brcmfmac: brcmf_usb_probe_phase2 failed: dev=1-1, err=-19
>>> [ 173.989503] brcmfmac: brcmf_usb_disconnect Enter
>>>
>>> This patch fixes the brcmf_usb_dl_cmd function to prevent an URB from being
>>> submitted twice. Tested using a LG TWFM-B003D, which now works properly.
>>>
>>>
>>> drivers/net/wireless/brcm80211/brcmfmac/usb.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>>> index 5265aa7..1bc7858 100644
>>> --- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
>>> @@ -738,10 +738,12 @@ static int brcmf_usb_dl_cmd(struct brcmf_usbdev_info *devinfo, u8 cmd,
>>> goto finalize;
>>> }
>>>
>>> - if (!brcmf_usb_ioctl_resp_wait(devinfo))
>>> + if (!brcmf_usb_ioctl_resp_wait(devinfo)) {
>>> + usb_unlink_urb(devinfo->ctl_urb);
>>> ret = -ETIMEDOUT;
>>> - else
>>> + } else {
>>> memcpy(buffer, tmpbuf, buflen);
>>> + }
>>>
>>> finalize:
>>> kfree(tmpbuf);
>>>
>>
^ permalink raw reply
* Re: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver
From: Arend van Spriel @ 2014-11-11 10:44 UTC (permalink / raw)
To: Johannes Berg
Cc: Gautam (Gautam Kumar) Shukla, linville@tuxdriver.com,
linux-wireless@vger.kernel.org, davem@davemloft.net,
linux-api@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, Jithu Jance, Sreenath S,
Vladimir Kondratiev
In-Reply-To: <1415702338.2163.5.camel@sipsolutions.net>
On 11-11-14 11:38, Johannes Berg wrote:
> On Tue, 2014-11-11 at 11:35 +0100, Arend van Spriel wrote:
>
>> What did pop up is the wiphy flags vs. nl80211 feature flags. When that
>> comes up it looks like 'potAtoes, potaetoes' to me.
>>
>> So is there are clear design rule for when to use which flag. For me the
>> wiphy object represents the device/firmware and 4-way handshake offload
>> support is determined by what the device/firmware supports.
>
> There are three types of flags:
>
> * wiphy flag attributes - deprecated as far as I'm concerned
Ok. deprecated is clear enough ;-)
> * wiphy nl80211 feature flags - much easier to use in kernel (and
> userspace)
> * nl80211 protocol flags - only one exists
> (NL80211_PROTOCOL_FEATURE_SPLIT_WIPHY_DUMP)
Thanks,
Arend
^ permalink raw reply
* Re: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver
From: Johannes Berg @ 2014-11-11 10:38 UTC (permalink / raw)
To: Arend van Spriel
Cc: Gautam (Gautam Kumar) Shukla, linville@tuxdriver.com,
linux-wireless@vger.kernel.org, davem@davemloft.net,
linux-api@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, Jithu Jance, Sreenath S,
Vladimir Kondratiev
In-Reply-To: <5461E67E.7010408@broadcom.com>
On Tue, 2014-11-11 at 11:35 +0100, Arend van Spriel wrote:
> What did pop up is the wiphy flags vs. nl80211 feature flags. When that
> comes up it looks like 'potAtoes, potaetoes' to me.
>
> So is there are clear design rule for when to use which flag. For me the
> wiphy object represents the device/firmware and 4-way handshake offload
> support is determined by what the device/firmware supports.
There are three types of flags:
* wiphy flag attributes - deprecated as far as I'm concerned
* wiphy nl80211 feature flags - much easier to use in kernel (and
userspace)
* nl80211 protocol flags - only one exists
(NL80211_PROTOCOL_FEATURE_SPLIT_WIPHY_DUMP)
johannes
^ 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