* Re: UDP wierdness around skb_copy_and_csum_datagram_msg()
From: Jay Smith @ 2016-09-29 2:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Christian Lamparter, Alan Curry, Al Viro
In-Reply-To: <1475112246.28155.122.camel@edumazet-glaptop3.roam.corp.google.com>
Actually, on a little more searching of this list's archives, I think
that this discussion: https://patchwork.kernel.org/patch/9260733/ is
about exactly the same issue I've found, except from the TCP side. I'm
cc'ing a few of the participants from that discussion.
So is the patch proposed there (copying and restoring the entire
iov_iter in skb_copy_and_csum_datagram_msg()) being considered as a
fix? If not, would an alternate one that concealed the
save-and-restore logic inside iov_iter.c be more acceptable? I'd be
happy to produce whatever's needed, or yield to someone with stronger
feelings about where it should go...
On Wed, Sep 28, 2016 at 6:24 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2016-09-28 at 17:18 -0700, Jay Smith wrote:
>> I've spent the last week or so trying to track down a recurring
>> problem I'm seeing with UDP datagram handling. I'm new to the
>> internals of the Linux network stack, but it appears to me that
>> there's a substantial error in recent kernels' handling of UDP
>> checksum errors.
>>
>> The behavior I'm seeing: I have a UDP server that receives lots of
>> datagrams from many devices on a single port. A small number of those
>> devices occasionally send packets with bad UDP checksums. After I
>> receive one of these bad packets, the next recvmsg() made on the
>> socket returns data from the bad-checksum packet, but the
>> source-address and length of the next (good) packet that arrived at
>> the port.
>>
>> I believe this issue was introduced between linux 3.18 and 3.19, by a
>> set of changes to net/core/datagram.c that made
>> skb_copy_and_csum_datagram_msg() and related functions use the
>> iov_iter structure to copy data to user buffers. In the case where
>> those functions copy a datagram and then conclude that the checksum is
>> invalid, they don't remove the already-copied data from the user's
>> iovec; when udp_recvmsg() calls skb_copy_and_csum_datagram_msg() for a
>> second time, looking at the next datagram on the queue, that second
>> datagram's data is appended to the first datagram's. So when
>> recvmsg() returns to the user, the return value and msg_name reflect
>> the second datagram, but the first bytes in the user's iovec come from
>> the first (bad) datagram.
>>
>> (I've attached a test program that demonstrates the problem. Note
>> that it sees correct behavior unless the bad-checksum packet has > 68
>> bytes of UDP data; otherwise, the packet doesn't make it past the
>> CHECKSUM_BREAK test, and never enters
>> skp_copy_and_csum_datagram_msg().)
>>
>> The fix for UDP seems pretty simple; the iov_iter's iov_offset member
>> just needs to be set back to zero on a checksum failure. But it looks
>> like skb_copy_and_csum_datagram_msg() is also called from tcp_input.c,
>> where I assume that multiple sk_buffs can be copied-and-csum'd into
>> the same iov -- if that's right, it seems like iov_iter needs some
>> additional state to support rolling-back the most recent copy without
>> losing previous ones.
>>
>> Any thoughts?
>
> Nice catch !
>
> What about clearing iov_offset from UDP (v4 & v6) only ?
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 7d96dc2d3d08fa909f247dfbcbd0fc1eeb59862b..928da2fbb3caa6de4d0e1d889c237590f71607ea 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1342,6 +1342,7 @@ csum_copy_err:
> /* starting over for a new packet, but check if we need to yield */
> cond_resched();
> msg->msg_flags &= ~MSG_TRUNC;
> + msg->msg_iter.iov_offset = 0;
> goto try_again;
> }
>
>
> (similar for ipv6)
>
>
^ permalink raw reply
* Re: [PATCH] netfilter: bridge: clarify bridge/netfilter message
From: Florian Westphal @ 2016-09-29 2:25 UTC (permalink / raw)
To: Stefan Agner; +Cc: stephen, davem, bridge, netdev, linux-kernel, pablo
In-Reply-To: <20160928220528.17478-1-stefan@agner.ch>
Stefan Agner <stefan@agner.ch> wrote:
> When using bridge without bridge netfilter enabled the message
> displayed is rather confusing and leads to belive that a deprecated
> feature is in use. Use IS_MODULE to be explicit that the message only
> affects users which use bridge netfilter as module and reword the
> message.
Acked-by: Florian Westphal <fw@strlen.de>
^ permalink raw reply
* Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
From: R. Parameswaran @ 2016-09-29 2:36 UTC (permalink / raw)
To: David Miller
Cc: parameswaran.r7, kleptog, jchapman, netdev, linux-kernel,
nprachan, rshearma, dfawcus, stephen, acme, lboccass
In-Reply-To: <20160928.034841.997752666320557327.davem@davemloft.net>
Hi David,
Please see inline:
On Wed, 28 Sep 2016, David Miller wrote:
> From: "R. Parameswaran" <parameswaran.r7@gmail.com>
> Date: Tue, 27 Sep 2016 12:17:21 -0700 (PDT)
>
> > Later, in vxlan_dev_configure(), called from vxlan_dev_create(), it gets
> > adjusted to account for the headers:
> >
> > vxlan_dev_configure():
> > ...
> > if (!conf->mtu)
> > dev->mtu = lowerdev->mtu - (use_ipv6 ?
> > VXLAN6_HEADROOM : VXLAN_HEADROOM);
> >
> >
> > where VXLAN_HEADROOM is defined as follows:
> >
> > /* IP header + UDP + VXLAN + Ethernet header */
> > #define VXLAN_HEADROOM (20 + 8 + 8 + 14)
> > /* IPv6 header + UDP + VXLAN + Ethernet header */
> > #define VXLAN6_HEADROOM (40 + 8 + 8 + 14)
>
> Right but I don't see it going through the effort to make use of the
> PMTU like you are.
>
> I have another strong concern related to this. There seems to be no
> mechanism used to propagate any PMTU events into the device's MTU.
>
> Because if there is a limiting nexthop in the route to the other end
> of the UDP tunnel, you won't learn the PMTU until you (or some other
> entity on the machine) actually starts sending traffic to the tunnel's
> endpoint.
>
> If the PMTU events aren't propagated into the tunnel's MTU or similar
> I think this is an ad-hoc solution.
>
> I would suggest that you either:
>
> 1) Do what VXLAN appears to do an ignore the PMTu
>
I'd like to point out one difference with VXLAN - in VXLAN, the
local physical interface is directly specified at the time of
creation of the tunnel, and the data structure seems to have the ifindex
of the local interface with which it is able to directly pull up the
underlay interface device. Whereas in L2TP, we only have the IP
address of the remote tunnel end-point and thus only the socket and the
dst from which we need to derive this.
Also, dst_mtu references dst->ops->mtu, which if I followed the pointer
chain correctly, will dereference to ipv4_mtu() (for the IPv4 case, as
an example). The code in ipv4_mtu looks like the following:
ipv4_mtu():
unsigned int mtu = rt->rt_pmtu;
if (!mtu || time_after_eq(jiffies, rt->dst.expires))
mtu = dst_metric_raw(dst, RTAX_MTU);
if (mtu)
return mtu;
mtu = dst->dev->mtu;
if (unlikely(dst_metric_locked(dst, RTAX_MTU))) {
if (rt->rt_uses_gateway && mtu > 576)
mtu = 576;
}
return min_t(unsigned int, mtu, IP_MAX_MTU);
The code above does not depend on PMTU to be working. If no PMTU
discovered MTU exists, it eventually falls back to the local
underlay device MTU - and this is the mode in which I tested the fix - PMTU
was off in my testbed, but it was picking up the local device MTU correctly.
Basically, this looks better than the VXLAN handling as far as I can
tell - at least it will pick up the existing discovered PMTU on a best
effort basis, while falling back to the underlay device if all else fails.
I agree that something like 2. below would be needed in the long run (it
will need some effort and redesign -e.g. how do I lookup the parent tunnel
from the socket when receiving a PMTU update, existing pointer chain runs
from tunnel to socket).
But since the existing (Ethernet over L2TP) MTU derivation is incorrect, I am
hoping this may be acceptable as an interim solution.
thanks,
Ramkumar
> 2) Add code to handle PMTU events that land on the UDP tunnel
> socket.
>
> Thanks.
>
^ permalink raw reply
* [PATCH v2 net-next 0/5] xps_flows: XPS flow steering when there is no socket
From: Tom Herbert @ 2016-09-29 3:54 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team, rick.jones2, alexander.duyck
This patch set introduces transmit flow steering for socketless packets.
The idea is that we record the transmit queues in a flow table that is
indexed by skbuff hash. The flow table entries have two values: the
queue_index and the head cnt of packets from the TX queue. We only allow
a queue to change for a flow if the tail cnt in the TX queue advances
beyond the recorded head cnt. That is the condition that should indicate
that all outstanding packets for the flow have completed transmission so
the queue can change.
Tracking the inflight queue is performed as part of DQL. Two fields are
added to the dql structure: num_enqueue_ops and num_completed_ops.
num_enqueue_ops incremented in dql_queued and num_completed_ops is
incremented in dql_completed by the number of operations completed (a
new argument to the function).
This patch set creates /sys/class/net/eth*/xps_dev_flow_table_cnt
which number of entries in the XPS flow table.
Note that the functionality here is technically best effort (for
instance we don't obtain a lock while processing a flow table entry).
Under high load it is possible that OOO packets can still be generated
due to XPS if two threads are hammering on the same flow table entry.
The assumption of these patches is that OOO packets are not the end of
the world and these should prevent OOO in most common use cases with
XPS.
This is a followup to previous RFC version. Fixes from RFC are:
- Move counters to DQL
- Fixed typo
- Simplified get flow index funtion
- Fixed sysfs flow_table_cnt to properly use DEVICE_ATTR_RW
- Renamed the mechanism
V2:
- Added documentation in scaling.txt and sysfs documentation
- Call skb_tx_hash directly from get_xps_queue. This allows
the socketless transmit flow steering to work properly if
a flow is bouncing between non-XPS and XPS CPUS. (suggested
by Alexander Duyck).
- Added a whold bunch of tested results provided by Rick Jones
(Thanks Rick!)
Tested:
Manually forced all packets to go through the xps_flows path.
Observed that some flows were deferred to change queues because
packets were in flight witht the flow bucket.
Testing done by Rick Jones:
Here is a quick look at performance tests for the result of trying the
prototype fix for the packet reordering problem with VMs sending over
an XPS-configured NIC. In particular, the Emulex/Avago/Broadcom
Skyhawk. The fix was applied to a 4.4 kernel.
Before: 3884 Mbit/s
After: 8897 Mbit/s
That was from a VM on a node with a Skyhawk and 2 E5-2640 processors
to baremetal E5-2640 with a BE3. Physical MTU was 1500, the VM's
vNIC's MTU was 1400. Systems were HPE ProLiants in OS Control Mode
for power management, with the "performance" frequency governor
loaded. An OpenStack Mitaka setup with Distributed Virtual Router.
We had some other NIC types in the setup as well. XPS was also
enabled on the ConnectX3-Pro. It was not enabled on the 82599ES (a
function of the kernel being used, which had it disabled from the
first reports of XPS negatively affecting VM traffic at the beginning
of the year)
Average Mbit/s From NIC type To Bare Metal BE3:
NIC Type,
CPU on VM Host Before After
------------------------------------------------
ConnectX-3 Pro,E5-2670v3 9224 9271
BE3, E5-2640 9016 9022
82599, E5-2640 9192 9003
BCM57840, E5-2640 9213 9153
Skyhawk, E5-2640 3884 8897
For completeness:
Average Mbit/s To NIC type from Bare Metal BE3:
NIC Type,
CPU on VM Host Before After
------------------------------------------------
ConnectX-3 Pro,E5-2670v3 9322 9144
BE3, E5-2640 9074 9017
82599, E5-2640 8670 8564
BCM57840, E5-2640 2468 * 7979
Skyhawk, E5-2640 8897 9269
* This is the Busted bnx2x NIC FW GRO implementation issue. It was
not visible in the "After" because the system was setup to disable
the NIC FW GRO by the time it booted on the fix kernel.
Average Transactions/s Between NIC type and Bare Metal BE3:
NIC Type,
CPU on VM Host Before After
------------------------------------------------
ConnectX-3 Pro,E5-2670v3 12421 12612
BE3, E5-2640 8178 8484
82599, E5-2640 8499 8549
BCM57840, E5-2640 8544 8560
Skyhawk, E5-2640 8537 8701
Tom Herbert (5):
net: Set SW hash in skb_set_hash_from_sk
dql: Add counters for number of queuing and completion operations
net: Add xps_dev_flow_table_cnt
xps_flows: XPS for packets that don't have a socket
xps: Documentation for transmit socketles flow steering
Documentation/ABI/testing/sysfs-class-net | 8 +++
Documentation/networking/scaling.txt | 26 ++++++++
include/linux/dynamic_queue_limits.h | 7 +-
include/linux/netdevice.h | 26 +++++++-
include/net/sock.h | 6 +-
lib/dynamic_queue_limits.c | 3 +-
net/Kconfig | 6 ++
net/core/dev.c | 87 +++++++++++++++++++------
net/core/net-sysfs.c | 103 ++++++++++++++++++++++++++++++
9 files changed, 246 insertions(+), 26 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH v2 net-next 1/5] net: Set SW hash in skb_set_hash_from_sk
From: Tom Herbert @ 2016-09-29 3:54 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team, rick.jones2, alexander.duyck
In-Reply-To: <20160929035428.204355-1-tom@herbertland.com>
Use the __skb_set_sw_hash to set the hash in an skbuff from the socket
txhash.
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
include/net/sock.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index ebf75db..17d379a 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1920,10 +1920,8 @@ static inline void sock_poll_wait(struct file *filp,
static inline void skb_set_hash_from_sk(struct sk_buff *skb, struct sock *sk)
{
- if (sk->sk_txhash) {
- skb->l4_hash = 1;
- skb->hash = sk->sk_txhash;
- }
+ if (sk->sk_txhash)
+ __skb_set_sw_hash(skb, sk->sk_txhash, true);
}
void skb_set_owner_w(struct sk_buff *skb, struct sock *sk);
--
2.9.3
^ permalink raw reply related
* [PATCH v2 net-next 2/5] dql: Add counters for number of queuing and completion operations
From: Tom Herbert @ 2016-09-29 3:54 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team, rick.jones2, alexander.duyck
In-Reply-To: <20160929035428.204355-1-tom@herbertland.com>
Add two new counters to struct dql that are num_enqueue_ops and
num_completed_ops. num_enqueue_ops is incremented by one in each call to
dql_queued. num_enqueue_ops is incremented in dql_completed which takes
an argument indicating number of operations completed. These counters
are only intended for statistics and do not impact the BQL algorithm.
We add a new sysfs entry in byte_queue_limits named inflight_pkts.
This provides the number of packets in flight for the queue by
dql->num_enqueue_ops - dql->num_completed_ops.
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
include/linux/dynamic_queue_limits.h | 7 ++++++-
include/linux/netdevice.h | 2 +-
lib/dynamic_queue_limits.c | 3 ++-
net/core/net-sysfs.c | 14 ++++++++++++++
4 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/include/linux/dynamic_queue_limits.h b/include/linux/dynamic_queue_limits.h
index a4be703..b6a4804 100644
--- a/include/linux/dynamic_queue_limits.h
+++ b/include/linux/dynamic_queue_limits.h
@@ -43,6 +43,8 @@ struct dql {
unsigned int adj_limit; /* limit + num_completed */
unsigned int last_obj_cnt; /* Count at last queuing */
+ unsigned int num_enqueue_ops; /* Number of queue operations */
+
/* Fields accessed only by completion path (dql_completed) */
unsigned int limit ____cacheline_aligned_in_smp; /* Current limit */
@@ -55,6 +57,8 @@ struct dql {
unsigned int lowest_slack; /* Lowest slack found */
unsigned long slack_start_time; /* Time slacks seen */
+ unsigned int num_completed_ops; /* Number of complete ops */
+
/* Configuration */
unsigned int max_limit; /* Max limit */
unsigned int min_limit; /* Minimum limit */
@@ -83,6 +87,7 @@ static inline void dql_queued(struct dql *dql, unsigned int count)
barrier();
dql->num_queued += count;
+ dql->num_enqueue_ops++;
}
/* Returns how many objects can be queued, < 0 indicates over limit. */
@@ -92,7 +97,7 @@ static inline int dql_avail(const struct dql *dql)
}
/* Record number of completed objects and recalculate the limit. */
-void dql_completed(struct dql *dql, unsigned int count);
+void dql_completed(struct dql *dql, unsigned int count, unsigned int ops);
/* Reset dql state */
void dql_reset(struct dql *dql);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 136ae6bb..9567107 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3015,7 +3015,7 @@ static inline void netdev_tx_completed_queue(struct netdev_queue *dev_queue,
if (unlikely(!bytes))
return;
- dql_completed(&dev_queue->dql, bytes);
+ dql_completed(&dev_queue->dql, bytes, pkts);
/*
* Without the memory barrier there is a small possiblity that
diff --git a/lib/dynamic_queue_limits.c b/lib/dynamic_queue_limits.c
index f346715..d5e7a27 100644
--- a/lib/dynamic_queue_limits.c
+++ b/lib/dynamic_queue_limits.c
@@ -14,7 +14,7 @@
#define AFTER_EQ(A, B) ((int)((A) - (B)) >= 0)
/* Records completed count and recalculates the queue limit */
-void dql_completed(struct dql *dql, unsigned int count)
+void dql_completed(struct dql *dql, unsigned int count, unsigned int ops)
{
unsigned int inprogress, prev_inprogress, limit;
unsigned int ovlimit, completed, num_queued;
@@ -108,6 +108,7 @@ void dql_completed(struct dql *dql, unsigned int count)
dql->prev_ovlimit = ovlimit;
dql->prev_last_obj_cnt = dql->last_obj_cnt;
dql->num_completed = completed;
+ dql->num_completed_ops += ops;
dql->prev_num_queued = num_queued;
}
EXPORT_SYMBOL(dql_completed);
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 6e4f347..ab7b0b6 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -1147,6 +1147,19 @@ static ssize_t bql_show_inflight(struct netdev_queue *queue,
static struct netdev_queue_attribute bql_inflight_attribute =
__ATTR(inflight, S_IRUGO, bql_show_inflight, NULL);
+static ssize_t bql_show_inflight_pkts(struct netdev_queue *queue,
+ struct netdev_queue_attribute *attr,
+ char *buf)
+{
+ struct dql *dql = &queue->dql;
+
+ return sprintf(buf, "%u\n",
+ dql->num_enqueue_ops - dql->num_completed_ops);
+}
+
+static struct netdev_queue_attribute bql_inflight_pkts_attribute =
+ __ATTR(inflight_pkts, S_IRUGO, bql_show_inflight_pkts, NULL);
+
#define BQL_ATTR(NAME, FIELD) \
static ssize_t bql_show_ ## NAME(struct netdev_queue *queue, \
struct netdev_queue_attribute *attr, \
@@ -1176,6 +1189,7 @@ static struct attribute *dql_attrs[] = {
&bql_limit_min_attribute.attr,
&bql_hold_time_attribute.attr,
&bql_inflight_attribute.attr,
+ &bql_inflight_pkts_attribute.attr,
NULL
};
--
2.9.3
^ permalink raw reply related
* [PATCH v2 net-next 3/5] net: Add xps_dev_flow_table_cnt
From: Tom Herbert @ 2016-09-29 3:54 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team, rick.jones2, alexander.duyck
In-Reply-To: <20160929035428.204355-1-tom@herbertland.com>
Add infrastructure and definitions to create XFS flow tables. This
creates the new sys entry /sys/class/net/eth*/xps_dev_flow_table_cnt
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
include/linux/netdevice.h | 24 +++++++++++++
net/core/net-sysfs.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 113 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9567107..60063b3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -736,6 +736,27 @@ struct xps_dev_maps {
(nr_cpu_ids * sizeof(struct xps_map *)))
#endif /* CONFIG_XPS */
+#ifdef CONFIG_XPS_FLOWS
+struct xps_dev_flow {
+ union {
+ u64 v64;
+ struct {
+ int queue_index;
+ unsigned int queue_ptr;
+ };
+ };
+};
+
+struct xps_dev_flow_table {
+ unsigned int mask;
+ struct rcu_head rcu;
+ struct xps_dev_flow flows[0];
+};
+#define XPS_DEV_FLOW_TABLE_SIZE(_num) (sizeof(struct xps_dev_flow_table) + \
+ ((_num) * sizeof(struct xps_dev_flow)))
+
+#endif /* CONFIG_XPS_FLOWS */
+
#define TC_MAX_QUEUE 16
#define TC_BITMASK 15
/* HW offloaded queuing disciplines txq count and offset maps */
@@ -1825,6 +1846,9 @@ struct net_device {
#ifdef CONFIG_XPS
struct xps_dev_maps __rcu *xps_maps;
#endif
+#ifdef CONFIG_XPS_FLOWS
+ struct xps_dev_flow_table __rcu *xps_flow_table;
+#endif
#ifdef CONFIG_NET_CLS_ACT
struct tcf_proto __rcu *egress_cl_list;
#endif
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index ab7b0b6..0d00b9c 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -503,6 +503,92 @@ static ssize_t phys_switch_id_show(struct device *dev,
}
static DEVICE_ATTR_RO(phys_switch_id);
+#ifdef CONFIG_XPS_FLOWS
+static void xps_dev_flow_table_release(struct rcu_head *rcu)
+{
+ struct xps_dev_flow_table *table = container_of(rcu,
+ struct xps_dev_flow_table, rcu);
+ vfree(table);
+}
+
+static int change_xps_dev_flow_table_cnt(struct net_device *dev,
+ unsigned long count)
+{
+ unsigned long mask;
+ struct xps_dev_flow_table *table, *old_table;
+ static DEFINE_SPINLOCK(xps_dev_flow_lock);
+
+ if (!capable(CAP_NET_ADMIN))
+ return -EPERM;
+
+ if (count) {
+ mask = count - 1;
+ /* mask = roundup_pow_of_two(count) - 1;
+ * without overflows...
+ */
+ while ((mask | (mask >> 1)) != mask)
+ mask |= (mask >> 1);
+ /* On 64 bit arches, must check mask fits in table->mask (u32),
+ * and on 32bit arches, must check
+ * XPS_DEV_FLOW_TABLE_SIZE(mask + 1) doesn't overflow.
+ */
+#if BITS_PER_LONG > 32
+ if (mask > (unsigned long)(u32)mask)
+ return -EINVAL;
+#else
+ if (mask > (ULONG_MAX - XPS_DEV_FLOW_TABLE_SIZE(1))
+ / sizeof(struct xps_dev_flow)) {
+ /* Enforce a limit to prevent overflow */
+ return -EINVAL;
+ }
+#endif
+ table = vmalloc(XPS_DEV_FLOW_TABLE_SIZE(mask + 1));
+ if (!table)
+ return -ENOMEM;
+
+ table->mask = mask;
+ for (count = 0; count <= mask; count++)
+ table->flows[count].queue_index = -1;
+ } else
+ table = NULL;
+
+ spin_lock(&xps_dev_flow_lock);
+ old_table = rcu_dereference_protected(dev->xps_flow_table,
+ lockdep_is_held(&xps_dev_flow_lock));
+ rcu_assign_pointer(dev->xps_flow_table, table);
+ spin_unlock(&xps_dev_flow_lock);
+
+ if (old_table)
+ call_rcu(&old_table->rcu, xps_dev_flow_table_release);
+
+ return 0;
+}
+
+static ssize_t xps_dev_flow_table_cnt_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ return netdev_store(dev, attr, buf, len, change_xps_dev_flow_table_cnt);
+}
+
+static ssize_t xps_dev_flow_table_cnt_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct net_device *netdev = to_net_dev(dev);
+ struct xps_dev_flow_table *table;
+ unsigned int cnt = 0;
+
+ rcu_read_lock();
+ table = rcu_dereference(netdev->xps_flow_table);
+ if (table)
+ cnt = table->mask + 1;
+ rcu_read_unlock();
+
+ return sprintf(buf, fmt_dec, cnt);
+}
+DEVICE_ATTR_RW(xps_dev_flow_table_cnt);
+#endif /* CONFIG_XPS_FLOWS */
+
static struct attribute *net_class_attrs[] = {
&dev_attr_netdev_group.attr,
&dev_attr_type.attr,
@@ -531,6 +617,9 @@ static struct attribute *net_class_attrs[] = {
&dev_attr_phys_port_name.attr,
&dev_attr_phys_switch_id.attr,
&dev_attr_proto_down.attr,
+#ifdef CONFIG_XPS_FLOWS
+ &dev_attr_xps_dev_flow_table_cnt.attr,
+#endif
NULL,
};
ATTRIBUTE_GROUPS(net_class);
--
2.9.3
^ permalink raw reply related
* [PATCH v2 net-next 4/5] xps_flows: XPS for packets that don't have a socket
From: Tom Herbert @ 2016-09-29 3:54 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team, rick.jones2, alexander.duyck
In-Reply-To: <20160929035428.204355-1-tom@herbertland.com>
xps_flows maintains a per device flow table that is indexed by the
skbuff hash. The table is only consulted when there is no queue saved in
a transmit socket for an skbuff.
Each entry in the flow table contains a queue index and a queue
pointer. The queue pointer is set when a queue is chosen using a
flow table entry. This pointer is set to the head pointer in the
transmit queue (which is maintained by BQL).
The new function get_xps_flows_index that looks up flows in the
xps_flows table. The entry returned gives the last queue a matching flow
used. The returned queue is compared against the normal XPS queue. If
they are different, then we only switch if the tail pointer in the TX
queue has advanced past the pointer saved in the entry. In this
way OOO should be avoided when XPS wants to use a different queue.
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
net/Kconfig | 6 ++++
net/core/dev.c | 87 +++++++++++++++++++++++++++++++++++++++++++++-------------
2 files changed, 74 insertions(+), 19 deletions(-)
diff --git a/net/Kconfig b/net/Kconfig
index 7b6cd34..f77fad1 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -255,6 +255,12 @@ config XPS
depends on SMP
default y
+config XPS_FLOWS
+ bool
+ depends on XPS
+ depends on BQL
+ default y
+
config HWBM
bool
diff --git a/net/core/dev.c b/net/core/dev.c
index c0c291f..1ca08b9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3210,6 +3210,7 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
}
#endif /* CONFIG_NET_EGRESS */
+/* Must be called with RCU read_lock */
static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
{
#ifdef CONFIG_XPS
@@ -3217,7 +3218,6 @@ static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
struct xps_map *map;
int queue_index = -1;
- rcu_read_lock();
dev_maps = rcu_dereference(dev->xps_maps);
if (dev_maps) {
map = rcu_dereference(
@@ -3228,15 +3228,62 @@ static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
else
queue_index = map->queues[reciprocal_scale(skb_get_hash(skb),
map->len)];
- if (unlikely(queue_index >= dev->real_num_tx_queues))
- queue_index = -1;
+ if (queue_index >= 0 &&
+ likely(queue_index < dev->real_num_tx_queues))
+ return queue_index;
}
}
- rcu_read_unlock();
+#endif
+ return skb_tx_hash(dev, skb);
+}
+
+/* Must be called with RCU read_lock */
+static int get_xps_flows_index(struct net_device *dev, struct sk_buff *skb)
+{
+#ifdef CONFIG_XPS_FLOWS
+ struct xps_dev_flow_table *flow_table;
+ struct xps_dev_flow ent;
+ int queue_index;
+ struct netdev_queue *txq;
+ u32 hash;
+
+ queue_index = get_xps_queue(dev, skb);
+ if (queue_index < 0)
+ return -1;
+
+ flow_table = rcu_dereference(dev->xps_flow_table);
+ if (!flow_table)
+ return queue_index;
+
+ hash = skb_get_hash(skb);
+ if (!hash)
+ return queue_index;
+
+ ent.v64 = flow_table->flows[hash & flow_table->mask].v64;
+
+ if (queue_index != ent.queue_index &&
+ ent.queue_index >= 0 &&
+ ent.queue_index < dev->real_num_tx_queues) {
+ txq = netdev_get_tx_queue(dev, ent.queue_index);
+ if ((int)(txq->dql.num_completed_ops - ent.queue_ptr) < 0) {
+ /* The current queue's tail has not advanced beyond the
+ * last packet that was enqueued using the table entry.
+ * We can't change queues without risking OOO. Stick
+ * with the queue listed in the flow table.
+ */
+ queue_index = ent.queue_index;
+ }
+ }
+
+ /* Save the updated entry */
+ txq = netdev_get_tx_queue(dev, queue_index);
+ ent.queue_index = queue_index;
+ ent.queue_ptr = txq->dql.num_enqueue_ops;
+ flow_table->flows[hash & flow_table->mask].v64 = ent.v64;
return queue_index;
#else
- return -1;
+ return get_xps_queue(dev, skb);
#endif
}
@@ -3244,22 +3291,24 @@ static u16 __netdev_pick_tx(struct net_device *dev, struct sk_buff *skb)
{
struct sock *sk = skb->sk;
int queue_index = sk_tx_queue_get(sk);
-
- if (queue_index < 0 || skb->ooo_okay ||
- queue_index >= dev->real_num_tx_queues) {
- int new_index = get_xps_queue(dev, skb);
- if (new_index < 0)
- new_index = skb_tx_hash(dev, skb);
-
- if (queue_index != new_index && sk &&
- sk_fullsock(sk) &&
- rcu_access_pointer(sk->sk_dst_cache))
- sk_tx_queue_set(sk, new_index);
-
- queue_index = new_index;
+ int new_index;
+
+ if (queue_index < 0) {
+ /* Socket did not provide a queue index, try xps_flows */
+ new_index = get_xps_flows_index(dev, skb);
+ } else if (skb->ooo_okay || queue_index >= dev->real_num_tx_queues) {
+ /* Queue index in socket, see if we can find a better one */
+ new_index = get_xps_queue(dev, skb);
+ } else {
+ /* Valid queue in socket and can't send OOO. Just return it */
+ return queue_index;
}
- return queue_index;
+ if (queue_index != new_index && sk && sk_fullsock(sk) &&
+ rcu_access_pointer(sk->sk_dst_cache))
+ sk_tx_queue_set(sk, new_index);
+
+ return new_index;
}
struct netdev_queue *netdev_pick_tx(struct net_device *dev,
--
2.9.3
^ permalink raw reply related
* [PATCH v2 net-next 5/5] xps: Documentation for transmit socketles flow steering
From: Tom Herbert @ 2016-09-29 3:54 UTC (permalink / raw)
To: davem, netdev; +Cc: kernel-team, rick.jones2, alexander.duyck
In-Reply-To: <20160929035428.204355-1-tom@herbertland.com>
Signed-off-by: Tom Herbert <tom@herbertland.com>
---
Documentation/ABI/testing/sysfs-class-net | 8 ++++++++
Documentation/networking/scaling.txt | 26 ++++++++++++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-class-net b/Documentation/ABI/testing/sysfs-class-net
index 668604f..0d2a6a9 100644
--- a/Documentation/ABI/testing/sysfs-class-net
+++ b/Documentation/ABI/testing/sysfs-class-net
@@ -251,3 +251,11 @@ Contact: netdev@vger.kernel.org
Description:
Indicates the unique physical switch identifier of a switch this
port belongs to, as a string.
+
+What: /sys/class/net/<iface>/xps_dev_flow_table_cnt
+Date: October 2016
+KernelVersion: 4.8
+Contact: netdev@vger.kernel.org
+Description:
+ Indicates the number of entries in the XPS socketless flow
+ table.
diff --git a/Documentation/networking/scaling.txt b/Documentation/networking/scaling.txt
index 59f4db2..7f9590c 100644
--- a/Documentation/networking/scaling.txt
+++ b/Documentation/networking/scaling.txt
@@ -400,6 +400,26 @@ transport layer is responsible for setting ooo_okay appropriately. TCP,
for instance, sets the flag when all data for a connection has been
acknowledged.
+XPS: Transmit Socketless Flow Steering
+======================================
+
+Not all flows have an associated socket in which the transmit queue and
+ooo information can be saved. For instance, packets being transmitted over
+OVS for VMs do not have a socket that is visible to the kernel. To allow
+XPS to be effectively used a flow hash table is employed to save queue and
+ooo information for such socketless flows.
+
+The idea is that transmit queues are saved for socketless flows in a
+flow table that is indexed by skbuff hash. The flow table entries have
+two values: the queue_index and the head cnt of packets from the TX
+queue. When a packet is being sent without an associated socket the
+hash table is consulted which returns the queue_index to use. The
+returned queue is compared to the queue that would be selected by XPS.
+If they are are different the XPS queue is selected only if the tail cnt
+in the TX queue advances beyond the recorded head cnt. This checks if
+sending the packet on the new queue could cause ooo packets for the
+flow.
+
==== XPS Configuration
XPS is only available if the kconfig symbol CONFIG_XPS is enabled (on by
@@ -409,6 +429,12 @@ queue is configured using the sysfs file entry:
/sys/class/net/<dev>/queues/tx-<n>/xps_cpus
+Transmit socketless flow steering is enabled by setting the number of
+entries in the XPS flow table. This is configured in the sysfs file
+entry:
+
+/sys/class/net/<dev>/xps_dev_flow_table_cnt
+
== Suggested Configuration
For a network device with a single transmission queue, XPS configuration
--
2.9.3
^ permalink raw reply related
* Re: Explaining RX-stages for XDP
From: Alexei Starovoitov via iovisor-dev @ 2016-09-29 4:44 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Eric Dumazet, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iovisor-dev-9jONkmmOlFHEE9lA1F8Ukti2O/JbrIOy@public.gmane.org,
John Fastabend, Jamal Hadi Salim, David Miller, Saeed Mahameed,
Edward Cree, Tom Herbert, Daniel Borkmann, Mel Gorman,
Pablo Neira Ayuso
In-Reply-To: <20160928124431.351d7180-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Wed, Sep 28, 2016 at 12:44:31PM +0200, Jesper Dangaard Brouer wrote:
>
> The idea is quite different. It has nothing to do with Edward's
> proposal[3]. The RX packet-vector is simply an array, either of pointers
> or index numbers (into the RX-ring). The needed changes are completely
> contained inside the driver.
>
> > As far as intermixed XDP vs stack traffic, I think for DoS case the
> > traffic patterns are binary. Either all of it is good or under attack
> > most of the traffic is bad, so makes sense to optimize for these two.
> > 50/50 case I think is artificial and not worth optimizing for.
>
> Sorry, but I feel you have completely misunderstood the concept of my
> idea. It does not matter what traffic pattern you believe or don't
> believe in, it is irrelevant. The fact is that intermixed traffic is
> possible with the current solution. The core of my idea is to remove
> the possibility for this intermixed traffic to occur, simply by seeing
> XDP as a RX-stage before the stack.
Is the idea to add two extra 'to_stack' and 'not_to_stack' arrays
after XDP program made a decision ?
Sounds interesting, but I cannot evaluate it properly, since I fail
to see what problem it solves and how it can be benchmarked.
Since you're saying the percentage of good vs bad traffic is irrelevant
then this idea should help all types of traffic in all cases?
That would be truly awesome.
I think the discussion around performance optimizations
should start from:
1. describing the problem: What is the use case?
What is the traffic pattern?
After problem is understood and accepted as a valid problem
and not some research subject then
2. define a benchmark that simulates it
when benchmarking methodology is agreed upon
we can go the next step of
3. come up with multiple ideas/proposals to solve the problem 1.
and finally discuss
4. different implementations of the best proposal from step 3
Without 1 it's easy to come up with a fake benchmark in step 2
that can justify addition of any code in 4
Without 2, the comparison of different proposals will be
based on subjective opinions instead of hard data.
Without 3 the proposed patch will be a hard sell, since
other alternatives were not on the table.
For new features the steps are different, of course.
When we started on XDP among everything else the first two problems
that it's supposed to solve were DoS and loadbalancer/ila_router.
For former the attack traffic is mostly dropped after parsing.
That behavior is simulated by samples/bpf/xdp1_kern.c
For load balancer most of the traffic is parsed and transmitted
out on the same device after packet rewrite. That scenario
is simulated by samples/bpf/xdp2_kern.c
Therefore the benchmarking and performance numbers were
centered around these two programs and I believe XDP itself
is a good solution for DoS and ila_router based on
the performance numbers from these two benchmarks.
I think XDP is _not_ a good solution for many other use cases.
The DoS protection feature when not under attack was
not benchmarked, so this is the area to work on in the future.
(the appropriate benchmark is xdp_pass+tcp+netperf)
Another area that was not benchmarked is how XDP drop/tx
interacts with normal control plane traffic.
We assumed that different hw tx queues should provide
sufficient separation. That is certainly to be tested.
A lot of work to do, no doubt.
> > Optimizing xdp for 'mostly good' traffic is indeed a challange.
> > We'd need all the tricks to make it as good as normal skb-based traffic.
> >
> > I haven't seen any tests yet comparing xdp with 'return XDP_PASS' program
> > vs no xdp at all running netperf tcp/udp in user space. It shouldn't
> > be too far off.
>
> Well, I did post numbers to the list with a 'return XDP_PASS' program[4]:
> https://mid.mail-archive.com/netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg122350.html
>
> Wake-up and smell the coffee, please revise your assumptions:
> * It showed that the performance reduction is 25.98%!!!
> (AB comparison dropping packets in iptables raw)
sure. iptables drop is slow with xdp_pass vs iptables without xdp.
This is not a benchmark I was interested in, since I don't
understand what use case it simulates.
> Conclusion: These measurements confirm that we need a page recycle
> facility for the drivers before switching to order-0 allocations.
...
> page_pool work
> - iptables-raw-drop: driver mlx5
> * 4,487,518 pps - baseline-before => 100.0%
> * 3,624,237 pps - mlx5 order0-patch => - 19.2% (slower)
> * 4,806,142 pps - PoC page_pool patch => +7.1% (faster)
I agree that generic page_pool is a useful facility, but
I don't think it's the right approach to design it based on
iptables_drop+xdp_pass benchmark.
You're saying the protoype gives 7.1% improvement. Ok,
but what is the problem being solved?
If the use case is DoS and fast as possible drop, then XDP_DROP
is a better alternative. Why design/benchmark page_pool
based on iptables_drop ?
If page_pool replaces custom page recycling in mlx4/mlx5
with generic one and maintains the same performance
on xdp1_kern.c and xdp2_kern.c benchmarks then the main
value is not a performance optimization, but making common
piece of infrastructure. Which is great.
^ permalink raw reply
* [PATCH v2 net-next] mlx5: Add ndo_poll_controller() implementation
From: Calvin Owens @ 2016-09-29 4:46 UTC (permalink / raw)
To: Saeed Mahameed, Matan Barak, Leon Romanovsky
Cc: netdev, linux-kernel, kernel-team, davem, Calvin Owens
In-Reply-To: <20160928.045409.1002159444377863778.davem@davemloft.net>
This implements ndo_poll_controller in net_device_ops callbacks for mlx5,
which is necessary to use netconsole with this driver.
Acked-By: Saeed Mahameed <saeedm@mellanox.com>
Signed-off-by: Calvin Owens <calvinowens@fb.com>
---
Changes in v2:
* Only iterate channels to avoid redundant napi_schedule() calls
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index b58cfe3..7eaf380 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3188,6 +3188,20 @@ static int mlx5e_xdp(struct net_device *dev, struct netdev_xdp *xdp)
}
}
+#ifdef CONFIG_NET_POLL_CONTROLLER
+/* Fake "interrupt" called by netpoll (eg netconsole) to send skbs without
+ * reenabling interrupts.
+ */
+static void mlx5e_netpoll(struct net_device *dev)
+{
+ struct mlx5e_priv *priv = netdev_priv(dev);
+ int i;
+
+ for (i = 0; i < priv->params.num_channels; i++)
+ napi_schedule(&priv->channel[i]->napi);
+}
+#endif
+
static const struct net_device_ops mlx5e_netdev_ops_basic = {
.ndo_open = mlx5e_open,
.ndo_stop = mlx5e_close,
@@ -3208,6 +3222,9 @@ static const struct net_device_ops mlx5e_netdev_ops_basic = {
#endif
.ndo_tx_timeout = mlx5e_tx_timeout,
.ndo_xdp = mlx5e_xdp,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ .ndo_poll_controller = mlx5e_netpoll,
+#endif
};
static const struct net_device_ops mlx5e_netdev_ops_sriov = {
@@ -3240,6 +3257,9 @@ static const struct net_device_ops mlx5e_netdev_ops_sriov = {
.ndo_get_vf_stats = mlx5e_get_vf_stats,
.ndo_tx_timeout = mlx5e_tx_timeout,
.ndo_xdp = mlx5e_xdp,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+ .ndo_poll_controller = mlx5e_netpoll,
+#endif
};
static int mlx5e_check_required_hca_cap(struct mlx5_core_dev *mdev)
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2 net-next 4/5] xps_flows: XPS for packets that don't have a socket
From: Eric Dumazet @ 2016-09-29 4:54 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, kernel-team, rick.jones2, alexander.duyck
In-Reply-To: <20160929035428.204355-5-tom@herbertland.com>
On Wed, 2016-09-28 at 20:54 -0700, Tom Herbert wrote:
> xps_flows maintains a per device flow table that is indexed by the
> skbuff hash. The table is only consulted when there is no queue saved in
> a transmit socket for an skbuff.
>
> Each entry in the flow table contains a queue index and a queue
> pointer. The queue pointer is set when a queue is chosen using a
> flow table entry. This pointer is set to the head pointer in the
> transmit queue (which is maintained by BQL).
>
> The new function get_xps_flows_index that looks up flows in the
> xps_flows table. The entry returned gives the last queue a matching flow
> used. The returned queue is compared against the normal XPS queue. If
> they are different, then we only switch if the tail pointer in the TX
> queue has advanced past the pointer saved in the entry. In this
> way OOO should be avoided when XPS wants to use a different queue.
There is something I do not understand with this.
If this OOO avoidance is tied to BQL, it means that packets sitting in a
qdisc wont be part of the detection.
So packets of flow X could possibly be queued on multiple qdiscs.
^ permalink raw reply
* [PATCH net-next 3/8] net/ncsi: Don't probe on the reserved channel ID (0x1f)
From: Gavin Shan @ 2016-09-29 5:03 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
In-Reply-To: <1475125395-8459-1-git-send-email-gwshan@linux.vnet.ibm.com>
We needn't send CIS (Clear Initial State) command to the NCSI
reserved channel (0x1f) in the enumeration. We shouldn't receive
a valid response from CIS on NCSI channel 0x1f.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/ncsi-manage.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 73883ed..3eeb915 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -887,12 +887,12 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
nd->state = ncsi_dev_state_probe_cis;
break;
case ncsi_dev_state_probe_cis:
- ndp->pending_req_num = 32;
+ ndp->pending_req_num = NCSI_RESERVED_CHANNEL;
/* Clear initial state */
nca.type = NCSI_PKT_CMD_CIS;
nca.package = ndp->active_package->id;
- for (index = 0; index < 0x20; index++) {
+ for (index = 0; index < NCSI_RESERVED_CHANNEL; index++) {
nca.channel = index;
ret = ncsi_xmit_cmd(&nca);
if (ret)
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 2/8] net/ncsi: Introduce NCSI_RESERVED_CHANNEL
From: Gavin Shan @ 2016-09-29 5:03 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
In-Reply-To: <1475125395-8459-1-git-send-email-gwshan@linux.vnet.ibm.com>
This defines NCSI_RESERVED_CHANNEL as the reserved NCSI channel
ID (0x1f). No logical changes introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/internal.h | 1 +
net/ncsi/ncsi-manage.c | 14 +++++++-------
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 33738c0..66dc851 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -170,6 +170,7 @@ struct ncsi_package;
#define NCSI_PACKAGE_SHIFT 5
#define NCSI_PACKAGE_INDEX(c) (((c) >> NCSI_PACKAGE_SHIFT) & 0x7)
+#define NCSI_RESERVED_CHANNEL 0x1f
#define NCSI_CHANNEL_INDEX(c) ((c) & ((1 << NCSI_PACKAGE_SHIFT) - 1))
#define NCSI_TO_CHANNEL(p, c) (((p) << NCSI_PACKAGE_SHIFT) | (c))
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index fc0ae54..73883ed 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -529,7 +529,7 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
nca.package = np->id;
if (nd->state == ncsi_dev_state_suspend_select) {
nca.type = NCSI_PKT_CMD_SP;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
if (ndp->flags & NCSI_DEV_HWA)
nca.bytes[0] = 0;
else
@@ -546,7 +546,7 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
nd->state = ncsi_dev_state_suspend_deselect;
} else if (nd->state == ncsi_dev_state_suspend_deselect) {
nca.type = NCSI_PKT_CMD_DP;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
nd->state = ncsi_dev_state_suspend_done;
}
@@ -592,7 +592,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
else
nca.bytes[0] = 1;
nca.package = np->id;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
ret = ncsi_xmit_cmd(&nca);
if (ret)
goto error;
@@ -810,7 +810,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
/* Deselect all possible packages */
nca.type = NCSI_PKT_CMD_DP;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
for (index = 0; index < 8; index++) {
nca.package = index;
ret = ncsi_xmit_cmd(&nca);
@@ -826,7 +826,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
/* Select all possible packages */
nca.type = NCSI_PKT_CMD_SP;
nca.bytes[0] = 1;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
for (index = 0; index < 8; index++) {
nca.package = index;
ret = ncsi_xmit_cmd(&nca);
@@ -879,7 +879,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
nca.type = NCSI_PKT_CMD_SP;
nca.bytes[0] = 1;
nca.package = ndp->active_package->id;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
ret = ncsi_xmit_cmd(&nca);
if (ret)
goto error;
@@ -936,7 +936,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
/* Deselect the active package */
nca.type = NCSI_PKT_CMD_DP;
nca.package = ndp->active_package->id;
- nca.channel = 0x1f;
+ nca.channel = NCSI_RESERVED_CHANNEL;
ret = ncsi_xmit_cmd(&nca);
if (ret)
goto error;
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 0/8] net/ncsi: NCSI Improvment and bug fixes
From: Gavin Shan @ 2016-09-29 5:03 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
This series of patches improves NCSI stack according to the comments
I received after the NCSI code was merged to 4.8.rc1:
* PATCH[1/8] fixes the build warning caused by xchg() with ia64-linux-gcc.
The atomic operations are replaced with {READ, WRITE}_ONCE().
* Channel ID (0x1f) is the reserved one and it cannot be valid channel ID.
So we needn't try to probe channel whose ID is 0x1f. PATCH[2/8] and
PATCH[3/8] are addressing this issue.
* The request IDs are assigned in round-robin fashion, but it's broken.
PATCH[4/8] make it work.
* PATCH[5/8] and PATCH[6/8] reworks the channel monitoring to improve the
code readability and its robustness.
* PATCH[7/8] and PATCH[8/8] introduces ncsi_stop_dev() so that the network
device can be closed and opened afterwards. No error will be seen.
Gavin Shan (8):
net/ncsi: Avoid unused-value build warning from ia64-linux-gcc
net/ncsi: Introduce NCSI_RESERVED_CHANNEL
net/ncsi: Don't probe on the reserved channel ID (0x1f)
net/ncsi: Rework request index allocation
net/ncsi: Allow to extend NCSI request properties
net/ncsi: Rework the channel monitoring
net/ncsi: Introduce ncsi_stop_dev()
net/faraday: Stop NCSI device on shutdown
drivers/net/ethernet/faraday/ftgmac100.c | 2 +
include/net/ncsi.h | 5 +
net/ncsi/internal.h | 22 ++--
net/ncsi/ncsi-aen.c | 19 ++--
net/ncsi/ncsi-cmd.c | 2 +-
net/ncsi/ncsi-manage.c | 166 ++++++++++++++++++-------------
net/ncsi/ncsi-rsp.c | 17 +---
7 files changed, 137 insertions(+), 96 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH net-next 7/8] net/ncsi: Introduce ncsi_stop_dev()
From: Gavin Shan @ 2016-09-29 5:03 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
In-Reply-To: <1475125395-8459-1-git-send-email-gwshan@linux.vnet.ibm.com>
This introduces ncsi_stop_dev(), as counterpart to ncsi_start_dev(),
to stop the NCSI device so that it can be reenabled in future. This
API should be called when the network device driver is going to
shutdown the device. There are 3 things done in the function: Stop
the channel monitoring; Reset channels to inactive state; Report
NCSI link down.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
include/net/ncsi.h | 5 +++++
net/ncsi/ncsi-manage.c | 33 ++++++++++++++++++++++-----------
2 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/include/net/ncsi.h b/include/net/ncsi.h
index 1dbf42f..68680ba 100644
--- a/include/net/ncsi.h
+++ b/include/net/ncsi.h
@@ -31,6 +31,7 @@ struct ncsi_dev {
struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*notifier)(struct ncsi_dev *nd));
int ncsi_start_dev(struct ncsi_dev *nd);
+void ncsi_stop_dev(struct ncsi_dev *nd);
void ncsi_unregister_dev(struct ncsi_dev *nd);
#else /* !CONFIG_NET_NCSI */
static inline struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
@@ -44,6 +45,10 @@ static inline int ncsi_start_dev(struct ncsi_dev *nd)
return -ENOTTY;
}
+static void ncsi_stop_dev(struct ncsi_dev *nd)
+{
+}
+
static inline void ncsi_unregister_dev(struct ncsi_dev *nd)
{
}
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 1b797c9..6a96873 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1161,9 +1161,7 @@ EXPORT_SYMBOL_GPL(ncsi_register_dev);
int ncsi_start_dev(struct ncsi_dev *nd)
{
struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
- struct ncsi_package *np;
- struct ncsi_channel *nc;
- int old_state, ret;
+ int ret;
if (nd->state != ncsi_dev_state_registered &&
nd->state != ncsi_dev_state_functional)
@@ -1175,9 +1173,27 @@ int ncsi_start_dev(struct ncsi_dev *nd)
return 0;
}
- /* Reset channel's state and start over */
+ if (ndp->flags & NCSI_DEV_HWA)
+ ret = ncsi_enable_hwa(ndp);
+ else
+ ret = ncsi_choose_active_channel(ndp);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ncsi_start_dev);
+
+void ncsi_stop_dev(struct ncsi_dev *nd)
+{
+ struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
+ struct ncsi_package *np;
+ struct ncsi_channel *nc;
+ int old_state;
+
+ /* Stop the channel monitor and reset channel's state */
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
+ ncsi_stop_channel_monitor(nc);
+
old_state = READ_ONCE(nc->state);
WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
WARN_ON_ONCE(!list_empty(&nc->link) ||
@@ -1185,14 +1201,9 @@ int ncsi_start_dev(struct ncsi_dev *nd)
}
}
- if (ndp->flags & NCSI_DEV_HWA)
- ret = ncsi_enable_hwa(ndp);
- else
- ret = ncsi_choose_active_channel(ndp);
-
- return ret;
+ ncsi_report_link(ndp, true);
}
-EXPORT_SYMBOL_GPL(ncsi_start_dev);
+EXPORT_SYMBOL_GPL(ncsi_stop_dev);
void ncsi_unregister_dev(struct ncsi_dev *nd)
{
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc
From: Gavin Shan @ 2016-09-29 5:03 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
In-Reply-To: <1475125395-8459-1-git-send-email-gwshan@linux.vnet.ibm.com>
xchg() is used to set NCSI channel's state in order for consistent
access to the state. xchg()'s return value should be used. Otherwise,
one build warning will be raised (with -Wunused-value) as below message
indicates. It is reported by ia64-linux-gcc (GCC) 4.9.0.
net/ncsi/ncsi-manage.c: In function 'ncsi_channel_monitor':
arch/ia64/include/uapi/asm/cmpxchg.h:56:2: warning: value computed is \
not used [-Wunused-value]
((__typeof__(*(ptr))) __xchg((unsigned long) (x), (ptr), sizeof(*(ptr))))
^
net/ncsi/ncsi-manage.c:202:3: note: in expansion of macro 'xchg'
xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
This replaces the atomic access to NCSI channel's state with READ_ONCE()
and WRITE_ONCE() to avoid the above build warning. We needn't hold the
channel's lock when updating its state as well. No logical changes
introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/ncsi-aen.c | 19 +++++++++++++------
net/ncsi/ncsi-manage.c | 37 +++++++++++++++++++++----------------
net/ncsi/ncsi-rsp.c | 13 +++----------
3 files changed, 37 insertions(+), 32 deletions(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index d463468..abc5473 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -53,6 +53,7 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
struct ncsi_aen_lsc_pkt *lsc;
struct ncsi_channel *nc;
struct ncsi_channel_mode *ncm;
+ int old_state;
unsigned long old_data;
unsigned long flags;
@@ -70,12 +71,14 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
if (!((old_data ^ ncm->data[2]) & 0x1) ||
!list_empty(&nc->link))
return 0;
- if (!(nc->state == NCSI_CHANNEL_INACTIVE && (ncm->data[2] & 0x1)) &&
- !(nc->state == NCSI_CHANNEL_ACTIVE && !(ncm->data[2] & 0x1)))
+
+ old_state = READ_ONCE(nc->state);
+ if (!(old_state == NCSI_CHANNEL_INACTIVE && (ncm->data[2] & 0x1)) &&
+ !(old_state == NCSI_CHANNEL_ACTIVE && !(ncm->data[2] & 0x1)))
return 0;
if (!(ndp->flags & NCSI_DEV_HWA) &&
- nc->state == NCSI_CHANNEL_ACTIVE)
+ old_state == NCSI_CHANNEL_ACTIVE)
ndp->flags |= NCSI_DEV_RESHUFFLE;
ncsi_stop_channel_monitor(nc);
@@ -90,6 +93,7 @@ static int ncsi_aen_handler_cr(struct ncsi_dev_priv *ndp,
struct ncsi_aen_pkt_hdr *h)
{
struct ncsi_channel *nc;
+ int old_state;
unsigned long flags;
/* Find the NCSI channel */
@@ -97,13 +101,14 @@ static int ncsi_aen_handler_cr(struct ncsi_dev_priv *ndp,
if (!nc)
return -ENODEV;
+ old_state = READ_ONCE(nc->state);
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE)
+ old_state != NCSI_CHANNEL_ACTIVE)
return 0;
ncsi_stop_channel_monitor(nc);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
spin_lock_irqsave(&ndp->lock, flags);
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
@@ -116,6 +121,7 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
struct ncsi_channel *nc;
struct ncsi_channel_mode *ncm;
struct ncsi_aen_hncdsc_pkt *hncdsc;
+ int old_state;
unsigned long flags;
/* Find the NCSI channel */
@@ -127,8 +133,9 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
ncm = &nc->modes[NCSI_MODE_LINK];
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
+ old_state = READ_ONCE(nc->state);
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE ||
+ old_state != NCSI_CHANNEL_ACTIVE ||
(ncm->data[3] & 0x1))
return 0;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index ef017b8..fc0ae54 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -143,7 +143,7 @@ static void ncsi_report_link(struct ncsi_dev_priv *ndp, bool force_down)
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_ACTIVE)
+ READ_ONCE(nc->state) != NCSI_CHANNEL_ACTIVE)
continue;
if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
@@ -166,7 +166,7 @@ static void ncsi_channel_monitor(unsigned long data)
bool enabled;
unsigned int timeout;
unsigned long flags;
- int ret;
+ int old_state, ret;
spin_lock_irqsave(&nc->lock, flags);
timeout = nc->timeout;
@@ -175,8 +175,10 @@ static void ncsi_channel_monitor(unsigned long data)
if (!enabled || !list_empty(&nc->link))
return;
- if (nc->state != NCSI_CHANNEL_INACTIVE &&
- nc->state != NCSI_CHANNEL_ACTIVE)
+
+ old_state = READ_ONCE(nc->state);
+ if (old_state != NCSI_CHANNEL_INACTIVE &&
+ old_state != NCSI_CHANNEL_ACTIVE)
return;
if (!(timeout % 2)) {
@@ -195,11 +197,11 @@ static void ncsi_channel_monitor(unsigned long data)
if (timeout + 1 >= 3) {
if (!(ndp->flags & NCSI_DEV_HWA) &&
- nc->state == NCSI_CHANNEL_ACTIVE)
+ old_state == NCSI_CHANNEL_ACTIVE)
ncsi_report_link(ndp, true);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
spin_lock_irqsave(&ndp->lock, flags);
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
ncsi_process_next_channel(ndp);
@@ -266,7 +268,7 @@ struct ncsi_channel *ncsi_add_channel(struct ncsi_package *np, unsigned char id)
nc->id = id;
nc->package = np;
- nc->state = NCSI_CHANNEL_INACTIVE;
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
nc->enabled = false;
setup_timer(&nc->timer, ncsi_channel_monitor, (unsigned long)nc);
spin_lock_init(&nc->lock);
@@ -309,7 +311,7 @@ static void ncsi_remove_channel(struct ncsi_channel *nc)
kfree(ncf);
}
- nc->state = NCSI_CHANNEL_INACTIVE;
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
spin_unlock_irqrestore(&nc->lock, flags);
ncsi_stop_channel_monitor(nc);
@@ -556,7 +558,7 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
break;
case ncsi_dev_state_suspend_done:
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
ncsi_process_next_channel(ndp);
break;
@@ -676,9 +678,9 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
break;
case ncsi_dev_state_config_done:
if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1)
- xchg(&nc->state, NCSI_CHANNEL_ACTIVE);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_ACTIVE);
else
- xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
ncsi_start_channel_monitor(nc);
ncsi_process_next_channel(ndp);
@@ -708,7 +710,7 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
if (!list_empty(&nc->link) ||
- nc->state != NCSI_CHANNEL_INACTIVE)
+ READ_ONCE(nc->state) != NCSI_CHANNEL_INACTIVE)
continue;
if (!found)
@@ -770,7 +772,8 @@ static int ncsi_enable_hwa(struct ncsi_dev_priv *ndp)
spin_lock_irqsave(&ndp->lock, flags);
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
- WARN_ON_ONCE(nc->state != NCSI_CHANNEL_INACTIVE ||
+ WARN_ON_ONCE(READ_ONCE(nc->state) !=
+ NCSI_CHANNEL_INACTIVE ||
!list_empty(&nc->link));
ncsi_stop_channel_monitor(nc);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
@@ -987,7 +990,8 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
goto out;
}
- old_state = xchg(&nc->state, NCSI_CHANNEL_INVISIBLE);
+ old_state = READ_ONCE(nc->state);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INVISIBLE);
list_del_init(&nc->link);
spin_unlock_irqrestore(&ndp->lock, flags);
@@ -1006,7 +1010,7 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
break;
default:
netdev_err(ndp->ndev.dev, "Invalid state 0x%x on %d:%d\n",
- nc->state, nc->package->id, nc->id);
+ READ_ONCE(nc->state), nc->package->id, nc->id);
ncsi_report_link(ndp, false);
return -EINVAL;
}
@@ -1166,7 +1170,8 @@ int ncsi_start_dev(struct ncsi_dev *nd)
/* Reset channel's state and start over */
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
- old_state = xchg(&nc->state, NCSI_CHANNEL_INACTIVE);
+ old_state = READ_ONCE(nc->state);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
WARN_ON_ONCE(!list_empty(&nc->link) ||
old_state == NCSI_CHANNEL_INVISIBLE);
}
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index af84389..9ee25ab 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -111,7 +111,6 @@ static int ncsi_rsp_handler_dp(struct ncsi_request *nr)
struct ncsi_dev_priv *ndp = nr->ndp;
struct ncsi_package *np;
struct ncsi_channel *nc;
- unsigned long flags;
/* Find the package */
rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
@@ -121,11 +120,8 @@ static int ncsi_rsp_handler_dp(struct ncsi_request *nr)
return -ENODEV;
/* Change state of all channels attached to the package */
- NCSI_FOR_EACH_CHANNEL(np, nc) {
- spin_lock_irqsave(&nc->lock, flags);
- nc->state = NCSI_CHANNEL_INACTIVE;
- spin_unlock_irqrestore(&nc->lock, flags);
- }
+ NCSI_FOR_EACH_CHANNEL(np, nc)
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
return 0;
}
@@ -184,7 +180,6 @@ static int ncsi_rsp_handler_rc(struct ncsi_request *nr)
struct ncsi_rsp_pkt *rsp;
struct ncsi_dev_priv *ndp = nr->ndp;
struct ncsi_channel *nc;
- unsigned long flags;
/* Find the package and channel */
rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
@@ -194,9 +189,7 @@ static int ncsi_rsp_handler_rc(struct ncsi_request *nr)
return -ENODEV;
/* Update state for the specified channel */
- spin_lock_irqsave(&nc->lock, flags);
- nc->state = NCSI_CHANNEL_INACTIVE;
- spin_unlock_irqrestore(&nc->lock, flags);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
return 0;
}
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 6/8] net/ncsi: Rework the channel monitoring
From: Gavin Shan @ 2016-09-29 5:03 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
In-Reply-To: <1475125395-8459-1-git-send-email-gwshan@linux.vnet.ibm.com>
The original NCSI channel monitoring was implemented based on a
backoff algorithm: the GLS response should be received in the
specified interval. Otherwise, the channel is regarded as dead
and failover should be taken if current channel is an active one.
There are several problems in the implementation: (A) On BCM5718,
we found when the IID (Instance ID) in the GLS command packet
changes from 255 to 1, the response corresponding to IID#1 never
comes in. It means we cannot make the unfair judgement that the
channel is dead when one response is missed. (B) The code's
readability should be improved. (C) We should do failover when
current channel is active one and the channel monitoring should
be marked as disabled before doing failover.
This reworks the channel monitoring to address all above issues.
The fields for channel monitoring is put into separate struct
and the state of channel monitoring is predefined. The channel
is regarded alive if the network controller responses to one of
two GLS commands or both of them in 5 seconds.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/internal.h | 12 +++++++++---
net/ncsi/ncsi-manage.c | 46 ++++++++++++++++++++++++++--------------------
net/ncsi/ncsi-rsp.c | 2 +-
3 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 26e9295..13290a7 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -187,9 +187,15 @@ struct ncsi_channel {
struct ncsi_channel_mode modes[NCSI_MODE_MAX];
struct ncsi_channel_filter *filters[NCSI_FILTER_MAX];
struct ncsi_channel_stats stats;
- struct timer_list timer; /* Link monitor timer */
- bool enabled; /* Timer is enabled */
- unsigned int timeout; /* Times of timeout */
+ struct {
+ struct timer_list timer;
+ bool enabled;
+ unsigned int state;
+#define NCSI_CHANNEL_MONITOR_START 0
+#define NCSI_CHANNEL_MONITOR_RETRY 1
+#define NCSI_CHANNEL_MONITOR_WAIT 2
+#define NCSI_CHANNEL_MONITOR_WAIT_MAX 5
+ } monitor;
struct list_head node;
struct list_head link;
};
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 41d6af9..1b797c9 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -164,13 +164,13 @@ static void ncsi_channel_monitor(unsigned long data)
struct ncsi_dev_priv *ndp = np->ndp;
struct ncsi_cmd_arg nca;
bool enabled;
- unsigned int timeout;
+ unsigned int monitor_state;
unsigned long flags;
int old_state, ret;
spin_lock_irqsave(&nc->lock, flags);
- timeout = nc->timeout;
- enabled = nc->enabled;
+ enabled = nc->monitor.enabled;
+ monitor_state = nc->monitor.state;
spin_unlock_irqrestore(&nc->lock, flags);
if (!enabled || !list_empty(&nc->link))
@@ -181,7 +181,9 @@ static void ncsi_channel_monitor(unsigned long data)
old_state != NCSI_CHANNEL_ACTIVE)
return;
- if (!(timeout % 2)) {
+ switch (monitor_state) {
+ case NCSI_CHANNEL_MONITOR_START:
+ case NCSI_CHANNEL_MONITOR_RETRY:
nca.ndp = ndp;
nca.package = np->id;
nca.channel = nc->id;
@@ -193,14 +195,18 @@ static void ncsi_channel_monitor(unsigned long data)
ret);
return;
}
- }
- if (timeout + 1 >= 3) {
+ break;
+ case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
+ break;
+ default:
if (!(ndp->flags & NCSI_DEV_HWA) &&
- old_state == NCSI_CHANNEL_ACTIVE)
+ old_state == NCSI_CHANNEL_ACTIVE) {
ncsi_report_link(ndp, true);
+ ndp->flags |= NCSI_DEV_RESHUFFLE;
+ }
- WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
+ WRITE_ONCE(nc->state, NCSI_CHANNEL_ACTIVE);
spin_lock_irqsave(&ndp->lock, flags);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
@@ -209,10 +215,9 @@ static void ncsi_channel_monitor(unsigned long data)
}
spin_lock_irqsave(&nc->lock, flags);
- nc->timeout = timeout + 1;
- nc->enabled = true;
+ nc->monitor.state++;
spin_unlock_irqrestore(&nc->lock, flags);
- mod_timer(&nc->timer, jiffies + HZ * (1 << (nc->timeout / 2)));
+ mod_timer(&nc->monitor.timer, jiffies + HZ);
}
void ncsi_start_channel_monitor(struct ncsi_channel *nc)
@@ -220,12 +225,12 @@ void ncsi_start_channel_monitor(struct ncsi_channel *nc)
unsigned long flags;
spin_lock_irqsave(&nc->lock, flags);
- WARN_ON_ONCE(nc->enabled);
- nc->timeout = 0;
- nc->enabled = true;
+ WARN_ON_ONCE(nc->monitor.enabled);
+ nc->monitor.enabled = true;
+ nc->monitor.state = NCSI_CHANNEL_MONITOR_START;
spin_unlock_irqrestore(&nc->lock, flags);
- mod_timer(&nc->timer, jiffies + HZ * (1 << (nc->timeout / 2)));
+ mod_timer(&nc->monitor.timer, jiffies + HZ);
}
void ncsi_stop_channel_monitor(struct ncsi_channel *nc)
@@ -233,14 +238,14 @@ void ncsi_stop_channel_monitor(struct ncsi_channel *nc)
unsigned long flags;
spin_lock_irqsave(&nc->lock, flags);
- if (!nc->enabled) {
+ if (!nc->monitor.enabled) {
spin_unlock_irqrestore(&nc->lock, flags);
return;
}
- nc->enabled = false;
+ nc->monitor.enabled = false;
spin_unlock_irqrestore(&nc->lock, flags);
- del_timer_sync(&nc->timer);
+ del_timer_sync(&nc->monitor.timer);
}
struct ncsi_channel *ncsi_find_channel(struct ncsi_package *np,
@@ -269,8 +274,9 @@ struct ncsi_channel *ncsi_add_channel(struct ncsi_package *np, unsigned char id)
nc->id = id;
nc->package = np;
WRITE_ONCE(nc->state, NCSI_CHANNEL_INACTIVE);
- nc->enabled = false;
- setup_timer(&nc->timer, ncsi_channel_monitor, (unsigned long)nc);
+ nc->monitor.enabled = false;
+ setup_timer(&nc->monitor.timer,
+ ncsi_channel_monitor, (unsigned long)nc);
spin_lock_init(&nc->lock);
INIT_LIST_HEAD(&nc->link);
for (index = 0; index < NCSI_CAP_MAX; index++)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 5143490..1618eda 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -315,7 +315,7 @@ static int ncsi_rsp_handler_gls(struct ncsi_request *nr)
/* Reset the channel monitor if it has been enabled */
spin_lock_irqsave(&nc->lock, flags);
- nc->timeout = 0;
+ nc->monitor.state = NCSI_CHANNEL_MONITOR_START;
spin_unlock_irqrestore(&nc->lock, flags);
return 0;
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 4/8] net/ncsi: Rework request index allocation
From: Gavin Shan @ 2016-09-29 5:03 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
In-Reply-To: <1475125395-8459-1-git-send-email-gwshan@linux.vnet.ibm.com>
The NCSI request index (struct ncsi_request::id) is put into instance
ID (IID) field while sending NCSI command packet. It was designed the
available IDs are given in round-robin fashion. @ndp->request_id was
introduced to represent the next available ID, but it has been used
as number of successively allocated IDs. It breaks the round-robin
design. Besides, we shouldn't put 0 to NCSI command packet's IID
field, meaning ID#0 should be reserved according section 6.3.1.1
in NCSI spec (v1.1.0).
This fixes above two issues. With it applied, the available IDs will
be assigned in round-robin fashion and ID#0 won't be assigned.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/internal.h | 1 +
net/ncsi/ncsi-manage.c | 17 +++++++++--------
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 66dc851..c956fe8 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -259,6 +259,7 @@ struct ncsi_dev_priv {
struct list_head packages; /* List of packages */
struct ncsi_request requests[256]; /* Request table */
unsigned int request_id; /* Last used request ID */
+#define NCSI_REQ_START_IDX 1
unsigned int pending_req_num; /* Number of pending requests */
struct ncsi_package *active_package; /* Currently handled package */
struct ncsi_channel *active_channel; /* Currently handled channel */
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 3eeb915..64aab38 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -415,30 +415,31 @@ struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven)
/* Check if there is one available request until the ceiling */
spin_lock_irqsave(&ndp->lock, flags);
- for (i = ndp->request_id; !nr && i < limit; i++) {
+ for (i = ndp->request_id; i < limit; i++) {
if (ndp->requests[i].used)
continue;
nr = &ndp->requests[i];
nr->used = true;
nr->driven = driven;
- if (++ndp->request_id >= limit)
- ndp->request_id = 0;
+ ndp->request_id = i + 1;
+ goto found;
}
/* Fail back to check from the starting cursor */
- for (i = 0; !nr && i < ndp->request_id; i++) {
+ for (i = NCSI_REQ_START_IDX; i < ndp->request_id; i++) {
if (ndp->requests[i].used)
continue;
nr = &ndp->requests[i];
nr->used = true;
nr->driven = driven;
- if (++ndp->request_id >= limit)
- ndp->request_id = 0;
+ ndp->request_id = i + 1;
+ goto found;
}
- spin_unlock_irqrestore(&ndp->lock, flags);
+found:
+ spin_unlock_irqrestore(&ndp->lock, flags);
return nr;
}
@@ -1122,7 +1123,7 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
/* Initialize private NCSI device */
spin_lock_init(&ndp->lock);
INIT_LIST_HEAD(&ndp->packages);
- ndp->request_id = 0;
+ ndp->request_id = NCSI_REQ_START_IDX;
for (i = 0; i < ARRAY_SIZE(ndp->requests); i++) {
ndp->requests[i].id = i;
ndp->requests[i].ndp = ndp;
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 8/8] net/faraday: Stop NCSI device on shutdown
From: Gavin Shan @ 2016-09-29 5:03 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
In-Reply-To: <1475125395-8459-1-git-send-email-gwshan@linux.vnet.ibm.com>
This stops NCSI device when closing the network device so that the
NCSI device can be reenabled later.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 90f9c54..2625872 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1190,6 +1190,8 @@ static int ftgmac100_stop(struct net_device *netdev)
napi_disable(&priv->napi);
if (netdev->phydev)
phy_stop(netdev->phydev);
+ else if (priv->use_ncsi)
+ ncsi_stop_dev(priv->ndev);
ftgmac100_stop_hw(priv);
free_irq(priv->irq, netdev);
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 5/8] net/ncsi: Allow to extend NCSI request properties
From: Gavin Shan @ 2016-09-29 5:03 UTC (permalink / raw)
To: netdev; +Cc: davem, joel, yuvali, benh, Gavin Shan
In-Reply-To: <1475125395-8459-1-git-send-email-gwshan@linux.vnet.ibm.com>
There is only one NCSI request property for now: the response for
the sent command need drive the workqueue or not. So we had one
field (@driven) for the purpose. We lost the flexibility to extend
NCSI request properties.
This replaces @driven with @flags and @req_flags in NCSI request
and NCSI command argument struct. Each bit of the newly introduced
field can be used for one property. No functional changes introduced.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
net/ncsi/internal.h | 8 +++++---
net/ncsi/ncsi-cmd.c | 2 +-
net/ncsi/ncsi-manage.c | 19 ++++++++++---------
net/ncsi/ncsi-rsp.c | 2 +-
4 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index c956fe8..26e9295 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -207,7 +207,8 @@ struct ncsi_package {
struct ncsi_request {
unsigned char id; /* Request ID - 0 to 255 */
bool used; /* Request that has been assigned */
- bool driven; /* Drive state machine */
+ unsigned int flags; /* NCSI request property */
+#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
struct ncsi_dev_priv *ndp; /* Associated NCSI device */
struct sk_buff *cmd; /* Associated NCSI command packet */
struct sk_buff *rsp; /* Associated NCSI response packet */
@@ -276,7 +277,7 @@ struct ncsi_cmd_arg {
unsigned char package; /* Destination package ID */
unsigned char channel; /* Detination channel ID or 0x1f */
unsigned short payload; /* Command packet payload length */
- bool driven; /* Drive the state machine? */
+ unsigned int req_flags; /* NCSI request properties */
union {
unsigned char bytes[16]; /* Command packet specific data */
unsigned short words[8];
@@ -315,7 +316,8 @@ void ncsi_find_package_and_channel(struct ncsi_dev_priv *ndp,
unsigned char id,
struct ncsi_package **np,
struct ncsi_channel **nc);
-struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven);
+struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp,
+ unsigned int req_flags);
void ncsi_free_request(struct ncsi_request *nr);
struct ncsi_dev *ncsi_find_dev(struct net_device *dev);
int ncsi_process_next_channel(struct ncsi_dev_priv *ndp);
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 21057a8..db7083b 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -272,7 +272,7 @@ static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg *nca)
struct sk_buff *skb;
struct ncsi_request *nr;
- nr = ncsi_alloc_request(ndp, nca->driven);
+ nr = ncsi_alloc_request(ndp, nca->req_flags);
if (!nr)
return NULL;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 64aab38..41d6af9 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -186,7 +186,7 @@ static void ncsi_channel_monitor(unsigned long data)
nca.package = np->id;
nca.channel = nc->id;
nca.type = NCSI_PKT_CMD_GLS;
- nca.driven = false;
+ nca.req_flags = 0;
ret = ncsi_xmit_cmd(&nca);
if (ret) {
netdev_err(ndp->ndev.dev, "Error %d sending GLS\n",
@@ -407,7 +407,8 @@ void ncsi_find_package_and_channel(struct ncsi_dev_priv *ndp,
* be same. Otherwise, the bogus response might be replied. So
* the available IDs are allocated in round-robin fashion.
*/
-struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven)
+struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp,
+ unsigned int req_flags)
{
struct ncsi_request *nr = NULL;
int i, limit = ARRAY_SIZE(ndp->requests);
@@ -421,7 +422,7 @@ struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven)
nr = &ndp->requests[i];
nr->used = true;
- nr->driven = driven;
+ nr->flags = req_flags;
ndp->request_id = i + 1;
goto found;
}
@@ -433,7 +434,7 @@ struct ncsi_request *ncsi_alloc_request(struct ncsi_dev_priv *ndp, bool driven)
nr = &ndp->requests[i];
nr->used = true;
- nr->driven = driven;
+ nr->flags = req_flags;
ndp->request_id = i + 1;
goto found;
}
@@ -461,7 +462,7 @@ void ncsi_free_request(struct ncsi_request *nr)
nr->cmd = NULL;
nr->rsp = NULL;
nr->used = false;
- driven = nr->driven;
+ driven = !!(nr->flags & NCSI_REQ_FLAG_EVENT_DRIVEN);
spin_unlock_irqrestore(&ndp->lock, flags);
if (driven && cmd && --ndp->pending_req_num == 0)
@@ -514,7 +515,7 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
int ret;
nca.ndp = ndp;
- nca.driven = true;
+ nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
switch (nd->state) {
case ncsi_dev_state_suspend:
nd->state = ncsi_dev_state_suspend_select;
@@ -580,7 +581,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
int ret;
nca.ndp = ndp;
- nca.driven = true;
+ nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
switch (nd->state) {
case ncsi_dev_state_config:
case ncsi_dev_state_config_sp:
@@ -801,7 +802,7 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
int ret;
nca.ndp = ndp;
- nca.driven = true;
+ nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
switch (nd->state) {
case ncsi_dev_state_probe:
nd->state = ncsi_dev_state_probe_deselect;
@@ -1075,7 +1076,7 @@ static int ncsi_inet6addr_event(struct notifier_block *this,
return NOTIFY_OK;
nca.ndp = ndp;
- nca.driven = false;
+ nca.req_flags = 0;
nca.package = np->id;
nca.channel = nc->id;
nca.dwords[0] = nc->caps[NCSI_CAP_MC].cap;
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 9ee25ab..5143490 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -310,7 +310,7 @@ static int ncsi_rsp_handler_gls(struct ncsi_request *nr)
ncm->data[3] = ntohl(rsp->other);
ncm->data[4] = ntohl(rsp->oem_status);
- if (nr->driven)
+ if (nr->flags & NCSI_REQ_FLAG_EVENT_DRIVEN)
return 0;
/* Reset the channel monitor if it has been enabled */
--
2.1.0
^ permalink raw reply related
* [PATCH kernel v2] PCI: Enable access to custom VPD for Chelsio devices (cxgb3)
From: Alexey Kardashevskiy @ 2016-09-29 5:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Alexey Kardashevskiy, Netdev, Alexander Duyck, Santosh Raspatur,
linux-kernel, linux-pci
There is at least one Chelsio 10Gb card which uses VPD area to store
some custom blocks (example below). However pci_vpd_size() returns
the length of the first block only assuming that there can be only
one VPD "End Tag" and VFIO blocks access beyond that offset
(since 4e1a63555) which leads to the situation when the guest "cxgb3"
driver fails to probe the device. The host system does not have this
problem as the drives accesses the config space directly without
pci_read_vpd()/...
This adds a quirk to override the VPD size to a bigger value.
The maximum size is taken from EEPROMSIZE in
drivers/net/ethernet/chelsio/cxgb3/common.h. We do not read the tag
as the cxgb3 driver does as the driver supports writing to EEPROM/VPD
and when it writes, it only checks for 8192 bytes boundary. The quirk
is registerted for all devices supported by the cxgb3 driver.
This adds a quirk to the PCI layer (not to the cxgb3 driver) as
the cxgb3 driver itself accesses VPD directly and the problem only exists
with the vfio-pci driver (when cxgb3 is not running on the host and
may not be even loaded) which blocks accesses beyond the first block
of VPD data. However vfio-pci itself does not have quirks mechanism so
we add it to PCI.
Tested on:
Ethernet controller [0200]: Chelsio Communications Inc T310 10GbE Single Port Adapter [1425:0030]
This is its VPD:
0000 Large item 42 bytes; name 0x2 Identifier String
b'10 Gigabit Ethernet-SR PCI Express Adapter'
#00 [EC] len=7: b'D76809 '
#0a [FN] len=7: b'46K7897'
#14 [PN] len=7: b'46K7897'
#1e [MN] len=4: b'1037'
#25 [FC] len=4: b'5769'
#2c [SN] len=12: b'YL102035603V'
#3b [NA] len=12: b'00145E992ED1'
0c00 Large item 16 bytes; name 0x2 Identifier String
b'S310E-SR-X '
0c13 Large item 234 bytes; name 0x10
#00 [PN] len=16: b'TBD '
#13 [EC] len=16: b'110107730D2 '
#26 [SN] len=16: b'97YL102035603V '
#39 [NA] len=12: b'00145E992ED1'
#48 [V0] len=6: b'175000'
#51 [V1] len=6: b'266666'
#5a [V2] len=6: b'266666'
#63 [V3] len=6: b'2000 '
#6c [V4] len=2: b'1 '
#71 [V5] len=6: b'c2 '
#7a [V6] len=6: b'0 '
#83 [V7] len=2: b'1 '
#88 [V8] len=2: b'0 '
#8d [V9] len=2: b'0 '
#92 [VA] len=2: b'0 '
#97 [RV] len=80: b's\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'...
0d00 Large item 252 bytes; name 0x11
#00 [VC] len=16: b'122310_1222 dp '
#13 [VD] len=16: b'610-0001-00 H1\x00\x00'
#26 [VE] len=16: b'122310_1353 fp '
#39 [VF] len=16: b'610-0001-00 H1\x00\x00'
#4c [RW] len=173: b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'...
0dff Small item 0 bytes; name 0xf End Tag
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* used pci_set_vpd_size() helper
* added explicit list of IDs from cxgb3 driver
* added a note in the commit log why the quirk is not in cxgb3
---
drivers/pci/quirks.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 44e0ff3..b22fce5 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3243,6 +3243,28 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PORT_RIDGE,
quirk_thunderbolt_hotplug_msi);
+static void quirk_chelsio_extend_vpd(struct pci_dev *dev)
+{
+ if (!dev->vpd)
+ return;
+
+ pci_set_vpd_size(dev, max_t(unsigned int, dev->vpd->len, 8192));
+}
+
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x20, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x21, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x22, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x23, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x24, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x25, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x26, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x30, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x31, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x32, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x35, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x36, quirk_chelsio_extend_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, 0x37, quirk_chelsio_extend_vpd);
+
#ifdef CONFIG_ACPI
/*
* Apple: Shutdown Cactus Ridge Thunderbolt controller.
--
2.5.0.rc3
^ permalink raw reply related
* Re: [PATCH net-next v5] bpf: allow access into map value arrays
From: David Miller @ 2016-09-29 5:36 UTC (permalink / raw)
To: jbacik; +Cc: daniel, tgraf, netdev, kernel-team, u9012063
In-Reply-To: <1475074472-23538-1-git-send-email-jbacik@fb.com>
From: Josef Bacik <jbacik@fb.com>
Date: Wed, 28 Sep 2016 10:54:32 -0400
> Suppose you have a map array value that is something like this
>
> struct foo {
> unsigned iter;
> int array[SOME_CONSTANT];
> };
>
> You can easily insert this into an array, but you cannot modify the contents of
> foo->array[] after the fact. This is because we have no way to verify we won't
> go off the end of the array at verification time. This patch provides a start
> for this work. We accomplish this by keeping track of a minimum and maximum
> value a register could be while we're checking the code. Then at the time we
> try to do an access into a MAP_VALUE we verify that the maximum offset into that
> region is a valid access into that memory region. So in practice, code such as
> this
>
> unsigned index = 0;
>
> if (foo->iter >= SOME_CONSTANT)
> foo->iter = index;
> else
> index = foo->iter++;
> foo->array[index] = bar;
>
> would be allowed, as we can verify that index will always be between 0 and
> SOME_CONSTANT-1. If you wish to use signed values you'll have to have an extra
> check to make sure the index isn't less than 0, or do something like index %=
> SOME_CONSTANT.
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 0/3] [v2] Add basic ACPI support to the Qualcomm Technologies EMAC driver
From: David Miller @ 2016-09-29 5:50 UTC (permalink / raw)
To: timur; +Cc: netdev, linaro-acpi, linux-arm-kernel, shankerd, vikrams
In-Reply-To: <1475081924-12362-1-git-send-email-timur@codeaurora.org>
From: Timur Tabi <timur@codeaurora.org>
Date: Wed, 28 Sep 2016 11:58:41 -0500
> This patch series adds support to the EMAC driver for extracting addresses,
> interrupts, and some _DSDs (properties) from ACPI. The first two patches
> clean up the code, and the third patch adds ACPI-specific functionality.
>
> The first patch fixes a bug with handling the platform_device for the
> internal PHY. This phy is treated as a separate device in both DT and
> ACPI, but since the platform is not released automatically when the
> driver unloads, managed functions like devm_ioremap_resource cannot be
> used.
>
> The second patch replaces of_get_mac_address with its platform-independent
> equivalent device_get_mac_address.
>
> The third patch parses the ACPI tables to obtain the platform_device for
> the primary EMAC node ("QCOM8070") and the internal phy node ("QCOM8071").
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc
From: David Miller @ 2016-09-29 5:54 UTC (permalink / raw)
To: gwshan; +Cc: netdev, joel, yuvali, benh
In-Reply-To: <1475125395-8459-2-git-send-email-gwshan@linux.vnet.ibm.com>
From: Gavin Shan <gwshan@linux.vnet.ibm.com>
Date: Thu, 29 Sep 2016 15:03:08 +1000
> This replaces the atomic access to NCSI channel's state with READ_ONCE()
> and WRITE_ONCE() to avoid the above build warning. We needn't hold the
> channel's lock when updating its state as well. No logical changes
> introduced.
I don't understand this.
If it's important to take the lock for the list add/del, then it must
be important to make the state change appear atomic wrt. that lock as
well.
Can parallel threads of control enter these functions which change the
state? If so, then you need to make the state changes under the lock.
In fact, you probably have to make the state tests under the locks as
well.
If not, please explain what prevents it from happening.
Thanks.
^ 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