* Re: [PATCH 05/14] net: sched: always take reference to action
From: Jiri Pirko @ 2018-05-15 8:58 UTC (permalink / raw)
To: Vlad Buslov
Cc: netdev, davem, jhs, xiyou.wangcong, pablo, kadlec, fw, ast,
daniel, edumazet, keescook, linux-kernel, netfilter-devel,
coreteam, kliteyn
In-Reply-To: <vbftvradqe4.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
Mon, May 14, 2018 at 08:49:07PM CEST, vladbu@mellanox.com wrote:
>
>On Mon 14 May 2018 at 16:23, Jiri Pirko <jiri@resnulli.us> wrote:
>> Mon, May 14, 2018 at 04:27:06PM CEST, vladbu@mellanox.com wrote:
>>>Without rtnl lock protection it is no longer safe to use pointer to tc
>>>action without holding reference to it. (it can be destroyed concurrently)
>>>
>>>Remove unsafe action idr lookup function. Instead of it, implement safe tcf
>>>idr check function that atomically looks up action in idr and increments
>>>its reference and bind counters.
>>>
>>>Implement both action search and check using new safe function.
>>>
>>>Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
>>>---
>>> net/sched/act_api.c | 38 ++++++++++++++++----------------------
>>> 1 file changed, 16 insertions(+), 22 deletions(-)
>>>
>>>diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>>>index 1331beb..9459cce 100644
>>>--- a/net/sched/act_api.c
>>>+++ b/net/sched/act_api.c
>>>@@ -284,44 +284,38 @@ int tcf_generic_walker(struct tc_action_net *tn, struct sk_buff *skb,
>>> }
>>> EXPORT_SYMBOL(tcf_generic_walker);
>>>
>>>-static struct tc_action *tcf_idr_lookup(u32 index, struct tcf_idrinfo *idrinfo)
>>>+bool __tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
>>>+ int bind)
>>> {
>>>- struct tc_action *p = NULL;
>>>+ struct tcf_idrinfo *idrinfo = tn->idrinfo;
>>>+ struct tc_action *p;
>>>
>>> spin_lock_bh(&idrinfo->lock);
>>
>> Why "_bh" variant is necessary here?
>
>It is not my code.
Yeah, yet still I wonder :)
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 2/3] rdma: print driver resource attributes
From: Leon Romanovsky @ 2018-05-15 8:54 UTC (permalink / raw)
To: Steve Wise; +Cc: Jason Gunthorpe, dsahern, stephen, netdev, linux-rdma
In-Reply-To: <a8493ed9-f16c-eb71-55db-f27bd1331521@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 2436 bytes --]
On Mon, May 14, 2018 at 05:04:26PM -0500, Steve Wise wrote:
>
>
> On 5/14/2018 3:41 PM, Jason Gunthorpe wrote:
> > On Mon, May 07, 2018 at 08:53:16AM -0700, Steve Wise wrote:
> >> This enhancement allows printing rdma device-specific state, if provided
> >> by the kernel. This is done in a generic manner, so rdma tool doesn't
> >> need to know about the details of every type of rdma device.
> >>
> >> Driver attributes for a rdma resource are in the form of <key,
> >> [print_type], value> tuples, where the key is a string and the value can
> >> be any supported driver attribute. The print_type attribute, if present,
> >> provides a print format to use vs the standard print format for the type.
> >> For example, the default print type for a PROVIDER_S32 value is "%d ",
> >> but "0x%x " if the print_type of PRINT_TYPE_HEX is included inthe tuple.
> >>
> >> Driver resources are only printed when the -dd flag is present.
> >> If -p is present, then the output is formatted to not exceed 80 columns,
> >> otherwise it is printed as a single row to be grep/awk friendly.
> >>
> >> Example output:
> >>
> >> # rdma resource show qp lqpn 1028 -dd -p
> >> link cxgb4_0/- lqpn 1028 rqpn 0 type RC state RTS rq-psn 0 sq-psn 0 path-mig-state MIGRATED pid 0 comm [nvme_rdma]
> >> sqid 1028 flushed 0 memsize 123968 cidx 85 pidx 85 wq_pidx 106 flush_cidx 85 in_use 0
> >> size 386 flags 0x0 rqid 1029 memsize 16768 cidx 43 pidx 41 wq_pidx 171 msn 44 rqt_hwaddr 0x2a8a5d00
> >> rqt_size 256 in_use 128 size 130 idx 43 wr_id 0xffff881057c03408 idx 40 wr_id 0xffff881057c033f0
> > Hey some of these look like kernel pointers.. That is a no-no.. What
> > is up there?
>
> Nothing is defined as a kernel pointer. But wr_id is often a pointer to
> the kernel rdma application's context...
>
> > The wr_id often contains a pointer, right? So we cannot just pass it
> > to user space..
>
> Hmm. It is useful for debugging kernel rdma applications. Perhaps
> these attrs can be only be sent up by the kernel if the capabilities
> allow. But previous review comments of the kernel series, which is now
> merged, forced me to remove passing the capabilities information to the
> driver resource fill functions.
>
> So what's the right way to do this?
The reviewer asked do not pass to drivers whole CAP_.. bits, because
they anyway don't need such granularity.
>
> Steve.
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH net-next] sched: manipulate __QDISC_STATE_RUNNING in qdisc_run_* helpers
From: Paolo Abeni @ 2018-05-15 8:50 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
John Fastabend
Currently NOLOCK qdiscs pay a measurable overhead to atomically
manipulate the __QDISC_STATE_RUNNING. Such bit is flipped twice per
packet in the uncontended scenario with packet rate below the
line rate: on packed dequeue and on the next, failing dequeue attempt.
This changeset moves the bit manipulation into the qdisc_run_{begin,end}
helpers, so that the bit is now flipped only once per packet, with
measurable performance improvement in the uncontended scenario.
This also allows simplifying the qdisc teardown code path - since
qdisc_is_running() is now effective for each qdisc type - and avoid a
possible race between qdisc_run() and dev_deactivate_many(), as now
the some_qdisc_is_busy() can properly detect NOLOCK qdiscs being busy
dequeuing packets.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/sch_generic.h | 10 +++++++++-
net/core/dev.c | 2 +-
net/sched/sch_generic.c | 31 +++++++++----------------------
3 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 5154c8300262..4d2b37226e75 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -113,13 +113,19 @@ static inline void qdisc_refcount_inc(struct Qdisc *qdisc)
static inline bool qdisc_is_running(const struct Qdisc *qdisc)
{
+ if (qdisc->flags & TCQ_F_NOLOCK)
+ return test_bit(__QDISC_STATE_RUNNING, &qdisc->state);
return (raw_read_seqcount(&qdisc->running) & 1) ? true : false;
}
static inline bool qdisc_run_begin(struct Qdisc *qdisc)
{
- if (qdisc_is_running(qdisc))
+ if (qdisc->flags & TCQ_F_NOLOCK) {
+ if (test_and_set_bit(__QDISC_STATE_RUNNING, &qdisc->state))
+ return false;
+ } else if (qdisc_is_running(qdisc)) {
return false;
+ }
/* Variant of write_seqcount_begin() telling lockdep a trylock
* was attempted.
*/
@@ -131,6 +137,8 @@ static inline bool qdisc_run_begin(struct Qdisc *qdisc)
static inline void qdisc_run_end(struct Qdisc *qdisc)
{
write_seqcount_end(&qdisc->running);
+ if (qdisc->flags & TCQ_F_NOLOCK)
+ clear_bit(__QDISC_STATE_RUNNING, &qdisc->state);
}
static inline bool qdisc_may_bulk(const struct Qdisc *qdisc)
diff --git a/net/core/dev.c b/net/core/dev.c
index 9f4390182384..7ca19f47a92a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3244,7 +3244,7 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
rc = NET_XMIT_DROP;
} else {
rc = q->enqueue(skb, q, &to_free) & NET_XMIT_MASK;
- __qdisc_run(q);
+ qdisc_run(q);
}
if (unlikely(to_free))
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 39c144b6ff98..ff3ce71aec93 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -373,33 +373,24 @@ bool sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
*/
static inline bool qdisc_restart(struct Qdisc *q, int *packets)
{
- bool more, validate, nolock = q->flags & TCQ_F_NOLOCK;
spinlock_t *root_lock = NULL;
struct netdev_queue *txq;
struct net_device *dev;
struct sk_buff *skb;
+ bool validate;
/* Dequeue packet */
- if (nolock && test_and_set_bit(__QDISC_STATE_RUNNING, &q->state))
- return false;
-
skb = dequeue_skb(q, &validate, packets);
- if (unlikely(!skb)) {
- if (nolock)
- clear_bit(__QDISC_STATE_RUNNING, &q->state);
+ if (unlikely(!skb))
return false;
- }
- if (!nolock)
+ if (!(q->flags & TCQ_F_NOLOCK))
root_lock = qdisc_lock(q);
dev = qdisc_dev(q);
txq = skb_get_tx_queue(dev, skb);
- more = sch_direct_xmit(skb, q, dev, txq, root_lock, validate);
- if (nolock)
- clear_bit(__QDISC_STATE_RUNNING, &q->state);
- return more;
+ return sch_direct_xmit(skb, q, dev, txq, root_lock, validate);
}
void __qdisc_run(struct Qdisc *q)
@@ -1131,17 +1122,13 @@ static bool some_qdisc_is_busy(struct net_device *dev)
dev_queue = netdev_get_tx_queue(dev, i);
q = dev_queue->qdisc_sleeping;
- if (q->flags & TCQ_F_NOLOCK) {
- val = test_bit(__QDISC_STATE_SCHED, &q->state);
- } else {
- root_lock = qdisc_lock(q);
- spin_lock_bh(root_lock);
+ root_lock = qdisc_lock(q);
+ spin_lock_bh(root_lock);
- val = (qdisc_is_running(q) ||
- test_bit(__QDISC_STATE_SCHED, &q->state));
+ val = (qdisc_is_running(q) ||
+ test_bit(__QDISC_STATE_SCHED, &q->state));
- spin_unlock_bh(root_lock);
- }
+ spin_unlock_bh(root_lock);
if (val)
return true;
--
2.14.3
^ permalink raw reply related
* Re: [PATCH] dt-bindings: net: ravb: Add support for r8a77990 SoC
From: Sergei Shtylyov @ 2018-05-15 8:43 UTC (permalink / raw)
To: Simon Horman, David Miller
Cc: yoshihiro.shimoda.uh, netdev, linux-renesas-soc, robh+dt,
mark.rutland, devicetree
In-Reply-To: <20180513075803.plrkbwmzksjnz45u@verge.net.au>
On 5/13/2018 10:58 AM, Simon Horman wrote:
>>> Add documentation for r8a77990 compatible string to renesas ravb device
>>> tree bindings documentation.
>>>
>>> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>>
>> I'm assuming this isn't targetted at one of my trees. Just FYI.
>
> Hi Dave,
>
> I think this is appropriate for net-next but if not I can take it.
There's also Rob, and IIRC he has taken alike patch recently...
[...]
MBR, Sergei
^ permalink raw reply
* [RFC PATCH] net: qualcomm: rmnet: rmnet_ethtool_ops can be static
From: kbuild test robot @ 2018-05-15 8:41 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan
Cc: kbuild-all, davem, netdev, Subash Abhinov Kasiviswanathan
In-Reply-To: <1526328527-20026-3-git-send-email-subashab@codeaurora.org>
Fixes: f85908d4636e ("net: qualcomm: rmnet: Add support for ethtool private stats")
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
rmnet_vnd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 4c04482..cb02e1a 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -196,7 +196,7 @@ static void rmnet_get_ethtool_stats(struct net_device *dev,
memcpy(data, st, ARRAY_SIZE(rmnet_gstrings_stats) * sizeof(u64));
}
-const struct ethtool_ops rmnet_ethtool_ops = {
+static const struct ethtool_ops rmnet_ethtool_ops = {
.get_ethtool_stats = rmnet_get_ethtool_stats,
.get_strings = rmnet_get_strings,
.get_sset_count = rmnet_get_sset_count,
^ permalink raw reply related
* Re: [PATCH net-next 2/3] net: qualcomm: rmnet: Add support for ethtool private stats
From: kbuild test robot @ 2018-05-15 8:41 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan
Cc: kbuild-all, davem, netdev, Subash Abhinov Kasiviswanathan
In-Reply-To: <1526328527-20026-3-git-send-email-subashab@codeaurora.org>
Hi Subash,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Subash-Abhinov-Kasiviswanathan/net-qualcomm-rmnet-Updates-2018-05-14/20180515-115355
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c:199:26: sparse: symbol 'rmnet_ethtool_ops' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [RFC PATCH 5/5] net: macb: Add WOL support with ARP
From: Claudiu Beznea @ 2018-05-15 8:39 UTC (permalink / raw)
To: Harini Katakam
Cc: Nicolas Ferre, David Miller, netdev, linux-kernel, michals,
appanad
In-Reply-To: <CAFcVECKA6J-c4KYPZS05Ooj4x5+AARcxCdJfV2uHOHNOVXZaOQ@mail.gmail.com>
Hi Harini,
On 10.05.2018 13:37, Harini Katakam wrote:
> Hi Claudiu,
>
> On Fri, May 4, 2018 at 5:47 PM, Claudiu Beznea
> <Claudiu.Beznea@microchip.com> wrote:
>>
>>
>> On 22.03.2018 15:51, harinikatakamlinux@gmail.com wrote:
>>> From: Harini Katakam <harinik@xilinx.com>
>>>
>>> This patch enables ARP wake event support in GEM through the following:
>>>
>>> -> WOL capability can be selected based on the SoC/GEM IP version rather
>>> than a devictree property alone. Hence add a new capability property and
>>> set device as "wakeup capable" in probe in this case.
>>> -> Wake source selection can be done via ethtool or by enabling wakeup
>>> in /sys/devices/platform/..../ethx/power/
>>> This patch adds default wake source as ARP and the existing selection of
>>> WOL using magic packet remains unchanged.
>>> -> When GEM is the wake device with ARP as the wake event, the current
>>> IP address to match is written to WOL register along with other
>>> necessary confuguration required for MAC to recognize an ARP event.
>>> -> While RX needs to remain enabled, there is no need to process the
>>> actual wake packet - hence tie off all RX queues to avoid unnecessary
>>> processing by DMA in the background.
>>
>> Why is this different for magic packet vs ARP packet?
>
> This should ideally be the same whether it is a magic packet or ARP on
> the version of the IP we use (more details in my comment below).
> I simply did not alter the magic packet code for now to avoid breaking
> others' flow.
I see your point. But in the end the code should be nice and clean.
>
> <snip>
>>> +#define MACB_CAPS_WOL 0x00000080
>>
>> I think would be better to have this as part of bp->wol and use it properly
>> in suspend/resume hooks.
>
> I think a capability flag as part of config structure is better
> because this is clearly an SoC related feature and there is no need
> to have a devicetree property.
Since there is no difference b/w ARP and magic packet in term of SoC,
I mean, there is no special treatment b/w them, I think we should treat
them both in the same way. This was my point.
>
> <snip>
>> Wouldn't it work if you will change it in something like this:
>>
>> u32 wolmask, arpipmask = 0;
>>
>> if (bp->wol & MACB_WOL_ENABLED) {
>> macb_writel(bp, IER, MACB_BIT(WOL));
>>
>> if (bp->wol & MACB_WOL_HAS_ARP_PACKET) {
>> /* Enable broadcast. */
>> gem_writel(bp, NCFGR, gem_readl(bp, NCFGR) & ~MACB_BIT(NBC));
>> arpipmask = cpu_to_be32p(&bp->dev->ip_ptr->ifa_list->ifa_local) & 0xFFFF;
>> wolmask = arpipmask | MACB_BIT(ARP);
>> } else {
>> wolmask = MACB_BIT(MAG);
>> }
>>
>> macb_writel(bp, WOL, wolmask);
>> enable_irq_wake(bp->queues[0].irq);
>
> The above would work. But I'd still have to add the RX BD changes
> and then stop the phy, disable interrupt etc., for most optimal power
> down state - the idea is to keep only is essential to detect a wake event.
>
Also, with your approach the suspend/resume time will be longer.
>> netif_device_detach(netdev);
>> }
>>
>> I cannot find anything particular for ARP WOL events in datasheet. Also,
>> I cannot find something related to DMA activity while WOL is active
>
> Can you please let me know which version you are referring to?
> ZynqMP uses the IP version r1p06 or 07.
I have user guide revision 15
.
>
> There is a clear set of rules for ARP wake event to be recognized.
>
> About the DMA activity, it is not explicitly mentioned but we have
> observed that the DMA will continue to process incoming packets
> and try to write them to the memory and Cadence has confirmed
> the same. Later versions of the IP may have some provision to
> stop DMA activity on RX channel but unfortunately in this version,
> using a dummy RX buffer descriptor is the only way.>
> I'm looking into your other suggestions.
> Thanks, will get back to you.
>
> Regards,
> Harini
>
^ permalink raw reply
* Re: [PATCH 00/14] Modify action API for implementing lockless actions
From: Jiri Pirko @ 2018-05-15 8:20 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Vlad Buslov, netdev, davem, xiyou.wangcong, pablo, kadlec, fw,
ast, daniel, edumazet, keescook, linux-kernel, netfilter-devel,
coreteam, kliteyn
In-Reply-To: <23f37e5a-dd30-0ad3-a5ab-df22bf0ad7f9@mojatatu.com>
Mon, May 14, 2018 at 08:03:20PM CEST, jhs@mojatatu.com wrote:
>On 14/05/18 10:27 AM, Vlad Buslov wrote:
>> Currently, all netlink protocol handlers for updating rules, actions and
>> qdiscs are protected with single global rtnl lock which removes any
>> possibility for parallelism. This patch set is a first step to remove
>> rtnl lock dependency from TC rules update path. It updates act API to
>> use atomic operations, rcu and spinlocks for fine-grained locking. It
>> also extend API with functions that are needed to update existing
>> actions for parallel execution.
>>
>> Outline of changes:
>> - Change tc action to use atomic reference and bind counters, rcu
>> mechanism for cookie update.
>> - Extend action ops API with 'delete' function and 'unlocked' flag.
>> - Change action API to work with actions in lockless manner based on
>> primitives implemented in previous patches.
>> - Extend action API with new functions necessary to implement unlocked
>> actions.
>
>Please run all the tdc tests with these changes. This area has almost
>good test coverage at this point. If you need help just ping me.
Oh, that reminds me. Vlad, please run also:
tools/testing/selftests/net/forwarding/tc_*
Thanks!
^ permalink raw reply
* [RESEND PATCH v2 1/1] net: phy: micrel: add 125MHz reference clock workaround
From: Marco Felsch @ 2018-05-15 8:18 UTC (permalink / raw)
To: robh+dt, mark.rutland, andrew, f.fainelli
Cc: netdev, devicetree, kernel, niebelm
In-Reply-To: <20180515081856.23322-1-m.felsch@pengutronix.de>
From: Markus Niebel <Markus.Niebel@tqs.de>
The micrel KSZ9031 phy has a optional clock pin (CLK125_NDO) which can be
used as reference clock for the MAC unit. The clock signal must meet the
RGMII requirements to ensure the correct data transmission between the
MAC and the PHY. The KSZ9031 phy does not fulfill the duty cycle
requirement if the phy is configured as slave. For a complete
describtion look at the errata sheets: DS80000691D or DS80000692D.
The errata sheet recommends to force the phy into master mode whenever
there is a 1000Base-T link-up as work around. Only set the
"micrel,force-master" property if you use the phy reference clock provided
by CLK125_NDO pin as MAC reference clock in your application.
Attenation, this workaround is only usable if the link partner can
be configured to slave mode for 1000Base-T.
Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
[m.felsch@pengutronix.de: fix dt-binding documentation]
[m.felsch@pengutronix.de: use already existing result var for read/write]
[m.felsch@pengutronix.de: add error handling]
[m.felsch@pengutronix.de: add more comments]
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
.../bindings/net/micrel-ksz90x1.txt | 7 +++++
drivers/net/phy/micrel.c | 31 +++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
index 42a248301615..e22d8cfea687 100644
--- a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
+++ b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
@@ -57,6 +57,13 @@ KSZ9031:
- txd2-skew-ps : Skew control of TX data 2 pad
- txd3-skew-ps : Skew control of TX data 3 pad
+ - micrel,force-master:
+ Boolean, force phy to master mode. Only set this option if the phy
+ reference clock provided at CLK125_NDO pin is used as MAC reference
+ clock because the clock jitter in slave mode is to high (errata#2).
+ Attention: The link partner must be configurable as slave otherwise
+ no link will be established.
+
Examples:
mdio {
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index f41b224a9cdb..ab195f0916d6 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -573,9 +573,40 @@ static int ksz9031_config_init(struct phy_device *phydev)
ksz9031_of_load_skew_values(phydev, of_node,
MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
tx_data_skews, 4);
+
+ /* Silicon Errata Sheet (DS80000691D or DS80000692D):
+ * When the device links in the 1000BASE-T slave mode only,
+ * the optional 125MHz reference output clock (CLK125_NDO)
+ * has wide duty cycle variation.
+ *
+ * The optional CLK125_NDO clock does not meet the RGMII
+ * 45/55 percent (min/max) duty cycle requirement and therefore
+ * cannot be used directly by the MAC side for clocking
+ * applications that have setup/hold time requirements on
+ * rising and falling clock edges.
+ *
+ * Workaround:
+ * Force the phy to be the master to receive a stable clock
+ * which meets the duty cycle requirement.
+ */
+ if (of_property_read_bool(of_node, "micrel,force-master")) {
+ result = phy_read(phydev, MII_CTRL1000);
+ if (result < 0)
+ goto err_force_master;
+
+ /* enable master mode, config & prefer master */
+ result |= CTL1000_ENABLE_MASTER | CTL1000_AS_MASTER;
+ result = phy_write(phydev, MII_CTRL1000, result);
+ if (result < 0)
+ goto err_force_master;
+ }
}
return ksz9031_center_flp_timing(phydev);
+
+err_force_master:
+ phydev_err(phydev, "failed to force the phy to master mode\n");
+ return result;
}
#define KSZ8873MLL_GLOBAL_CONTROL_4 0x06
--
2.17.0
^ permalink raw reply related
* [RESEND PATCH v2 0/1] net: phy: micrel: add 125MHz reference clock workaround
From: Marco Felsch @ 2018-05-15 8:18 UTC (permalink / raw)
To: robh+dt, mark.rutland, andrew, f.fainelli
Cc: netdev, devicetree, kernel, niebelm
This patch adds the workaround for the errata#2. The KSZ9031 phy
125MHz reference clock jitter is to high if the phy is configured as
slave. This will cause errors if a application uses the clock as
MAC reference clock. The workaround is to configure the phy as master.
v2:
- add more code and dt-binding comments
- delete unnecessary local variables
- add error handling
v1:
- initial commit
Markus Niebel (1):
net: phy: micrel: add 125MHz reference clock workaround
.../bindings/net/micrel-ksz90x1.txt | 7 ++++
drivers/net/phy/micrel.c | 32 +++++++++++++++++++
2 files changed, 39 insertions(+)
--
2.17.0
^ permalink raw reply
* Re: [PATCH bpf-next] selftests/bpf: make sure build-id is on
From: Daniel Borkmann @ 2018-05-15 8:10 UTC (permalink / raw)
To: Alexei Starovoitov, David S . Miller; +Cc: songliubraving, netdev
In-Reply-To: <20180515001129.3557608-1-ast@kernel.org>
On 05/15/2018 02:11 AM, Alexei Starovoitov wrote:
> --build-id may not be a default linker config.
> Make sure it's used when linking urandom_read test program.
> Otherwise test_stacktrace_build_id[_nmi] tests will be failling.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Applied to bpf-next, thanks Alexei!
^ permalink raw reply
* Re: [PATCH v2 0/1] net: phy: micrel: add 125MHz reference clock workaround
From: Marco Felsch @ 2018-05-15 8:06 UTC (permalink / raw)
To: robh+dt, mark.rutland, andrew, f.fainelli
Cc: netdev, niebelm, kernel, devicetree
In-Reply-To: <20180515074527.22546-1-m.felsch@pengutronix.de>
I forgot to set the thread option. I will resend with thread option
enabled.
On 05/15/2018 09:45 AM, Marco Felsch wrote:
> This patch adds the workaround for the errata#2. The KSZ9031 phy
> 125MHz reference clock jitter is to high if the phy is configured as
> slave. This will cause errors if a application uses the clock as
> MAC reference clock. The workaround is to configure the phy as master.
>
> v2:
> - add more code and dt-binding comments
> - delete unnecessary local variables
> - add error handling
>
> v1:
> - initial commit
>
> Markus Niebel (1):
> net: phy: micrel: add 125MHz reference clock workaround
>
> .../bindings/net/micrel-ksz90x1.txt | 7 ++++
> drivers/net/phy/micrel.c | 32 +++++++++++++++++++
> 2 files changed, 39 insertions(+)
>
>
^ permalink raw reply
* [PATCH v2 1/1] net: phy: micrel: add 125MHz reference clock workaround
From: Marco Felsch @ 2018-05-15 7:47 UTC (permalink / raw)
To: robh+dt, mark.rutland, andrew, f.fainelli
Cc: netdev, devicetree, kernel, niebelm
From: Markus Niebel <Markus.Niebel@tqs.de>
The micrel KSZ9031 phy has a optional clock pin (CLK125_NDO) which can be
used as reference clock for the MAC unit. The clock signal must meet the
RGMII requirements to ensure the correct data transmission between the
MAC and the PHY. The KSZ9031 phy does not fulfill the duty cycle
requirement if the phy is configured as slave. For a complete
describtion look at the errata sheets: DS80000691D or DS80000692D.
The errata sheet recommends to force the phy into master mode whenever
there is a 1000Base-T link-up as work around. Only set the
"micrel,force-master" property if you use the phy reference clock provided
by CLK125_NDO pin as MAC reference clock in your application.
Attenation, this workaround is only usable if the link partner can
be configured to slave mode for 1000Base-T.
Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
[m.felsch@pengutronix.de: fix dt-binding documentation]
[m.felsch@pengutronix.de: use already existing result var for read/write]
[m.felsch@pengutronix.de: add error handling]
[m.felsch@pengutronix.de: add more comments]
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
.../bindings/net/micrel-ksz90x1.txt | 7 +++++
drivers/net/phy/micrel.c | 31 +++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
index 42a248301615..e22d8cfea687 100644
--- a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
+++ b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
@@ -57,6 +57,13 @@ KSZ9031:
- txd2-skew-ps : Skew control of TX data 2 pad
- txd3-skew-ps : Skew control of TX data 3 pad
+ - micrel,force-master:
+ Boolean, force phy to master mode. Only set this option if the phy
+ reference clock provided at CLK125_NDO pin is used as MAC reference
+ clock because the clock jitter in slave mode is to high (errata#2).
+ Attention: The link partner must be configurable as slave otherwise
+ no link will be established.
+
Examples:
mdio {
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index f41b224a9cdb..ab195f0916d6 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -573,9 +573,40 @@ static int ksz9031_config_init(struct phy_device *phydev)
ksz9031_of_load_skew_values(phydev, of_node,
MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
tx_data_skews, 4);
+
+ /* Silicon Errata Sheet (DS80000691D or DS80000692D):
+ * When the device links in the 1000BASE-T slave mode only,
+ * the optional 125MHz reference output clock (CLK125_NDO)
+ * has wide duty cycle variation.
+ *
+ * The optional CLK125_NDO clock does not meet the RGMII
+ * 45/55 percent (min/max) duty cycle requirement and therefore
+ * cannot be used directly by the MAC side for clocking
+ * applications that have setup/hold time requirements on
+ * rising and falling clock edges.
+ *
+ * Workaround:
+ * Force the phy to be the master to receive a stable clock
+ * which meets the duty cycle requirement.
+ */
+ if (of_property_read_bool(of_node, "micrel,force-master")) {
+ result = phy_read(phydev, MII_CTRL1000);
+ if (result < 0)
+ goto err_force_master;
+
+ /* enable master mode, config & prefer master */
+ result |= CTL1000_ENABLE_MASTER | CTL1000_AS_MASTER;
+ result = phy_write(phydev, MII_CTRL1000, result);
+ if (result < 0)
+ goto err_force_master;
+ }
}
return ksz9031_center_flp_timing(phydev);
+
+err_force_master:
+ phydev_err(phydev, "failed to force the phy to master mode\n");
+ return result;
}
#define KSZ8873MLL_GLOBAL_CONTROL_4 0x06
--
2.17.0
^ permalink raw reply related
* [PATCH v2 1/1] net: phy: micrel: add 125MHz reference clock workaround
From: Marco Felsch @ 2018-05-15 7:45 UTC (permalink / raw)
To: robh+dt, mark.rutland, andrew, f.fainelli
Cc: netdev, devicetree, kernel, niebelm
From: Markus Niebel <Markus.Niebel@tqs.de>
The micrel KSZ9031 phy has a optional clock pin (CLK125_NDO) which can be
used as reference clock for the MAC unit. The clock signal must meet the
RGMII requirements to ensure the correct data transmission between the
MAC and the PHY. The KSZ9031 phy does not fulfill the duty cycle
requirement if the phy is configured as slave. For a complete
describtion look at the errata sheets: DS80000691D or DS80000692D.
The errata sheet recommends to force the phy into master mode whenever
there is a 1000Base-T link-up as work around. Only set the
"micrel,force-master" property if you use the phy reference clock provided
by CLK125_NDO pin as MAC reference clock in your application.
Attenation, this workaround is only usable if the link partner can
be configured to slave mode for 1000Base-T.
Signed-off-by: Markus Niebel <Markus.Niebel@tqs.de>
[m.felsch@pengutronix.de: fix dt-binding documentation]
[m.felsch@pengutronix.de: use already existing result var for read/write]
[m.felsch@pengutronix.de: add error handling]
[m.felsch@pengutronix.de: add more comments]
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
.../bindings/net/micrel-ksz90x1.txt | 7 +++++
drivers/net/phy/micrel.c | 31 +++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
index 42a248301615..e22d8cfea687 100644
--- a/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
+++ b/Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
@@ -57,6 +57,13 @@ KSZ9031:
- txd2-skew-ps : Skew control of TX data 2 pad
- txd3-skew-ps : Skew control of TX data 3 pad
+ - micrel,force-master:
+ Boolean, force phy to master mode. Only set this option if the phy
+ reference clock provided at CLK125_NDO pin is used as MAC reference
+ clock because the clock jitter in slave mode is to high (errata#2).
+ Attention: The link partner must be configurable as slave otherwise
+ no link will be established.
+
Examples:
mdio {
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index f41b224a9cdb..ab195f0916d6 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -573,9 +573,40 @@ static int ksz9031_config_init(struct phy_device *phydev)
ksz9031_of_load_skew_values(phydev, of_node,
MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
tx_data_skews, 4);
+
+ /* Silicon Errata Sheet (DS80000691D or DS80000692D):
+ * When the device links in the 1000BASE-T slave mode only,
+ * the optional 125MHz reference output clock (CLK125_NDO)
+ * has wide duty cycle variation.
+ *
+ * The optional CLK125_NDO clock does not meet the RGMII
+ * 45/55 percent (min/max) duty cycle requirement and therefore
+ * cannot be used directly by the MAC side for clocking
+ * applications that have setup/hold time requirements on
+ * rising and falling clock edges.
+ *
+ * Workaround:
+ * Force the phy to be the master to receive a stable clock
+ * which meets the duty cycle requirement.
+ */
+ if (of_property_read_bool(of_node, "micrel,force-master")) {
+ result = phy_read(phydev, MII_CTRL1000);
+ if (result < 0)
+ goto err_force_master;
+
+ /* enable master mode, config & prefer master */
+ result |= CTL1000_ENABLE_MASTER | CTL1000_AS_MASTER;
+ result = phy_write(phydev, MII_CTRL1000, result);
+ if (result < 0)
+ goto err_force_master;
+ }
}
return ksz9031_center_flp_timing(phydev);
+
+err_force_master:
+ phydev_err(phydev, "failed to force the phy to master mode\n");
+ return result;
}
#define KSZ8873MLL_GLOBAL_CONTROL_4 0x06
--
2.17.0
^ permalink raw reply related
* [PATCH v2 0/1] net: phy: micrel: add 125MHz reference clock workaround
From: Marco Felsch @ 2018-05-15 7:45 UTC (permalink / raw)
To: robh+dt, mark.rutland, andrew, f.fainelli
Cc: netdev, devicetree, kernel, niebelm
This patch adds the workaround for the errata#2. The KSZ9031 phy
125MHz reference clock jitter is to high if the phy is configured as
slave. This will cause errors if a application uses the clock as
MAC reference clock. The workaround is to configure the phy as master.
v2:
- add more code and dt-binding comments
- delete unnecessary local variables
- add error handling
v1:
- initial commit
Markus Niebel (1):
net: phy: micrel: add 125MHz reference clock workaround
.../bindings/net/micrel-ksz90x1.txt | 7 ++++
drivers/net/phy/micrel.c | 32 +++++++++++++++++++
2 files changed, 39 insertions(+)
--
2.17.0
^ permalink raw reply
* Re: [PATCH v4 1/1] drivers core: multi-threading device shutdown
From: Greg KH @ 2018-05-15 7:37 UTC (permalink / raw)
To: Pavel Tatashin
Cc: steven.sistare, daniel.m.jordan, linux-kernel, jeffrey.t.kirsher,
intel-wired-lan, netdev, alexander.duyck, tobin
In-Reply-To: <20180514194254.14748-2-pasha.tatashin@oracle.com>
On Mon, May 14, 2018 at 03:42:54PM -0400, Pavel Tatashin wrote:
> When system is rebooted, halted or kexeced device_shutdown() is
> called.
>
> This function shuts down every single device by calling either:
>
> dev->bus->shutdown(dev)
> dev->driver->shutdown(dev)
>
> Even on a machine with just a moderate amount of devices, device_shutdown()
> may take multiple seconds to complete. This is because many devices require
> a specific delays to perform this operation.
>
> Here is a sample analysis of time it takes to call device_shutdown() on a
> two socket Intel(R) Xeon(R) CPU E5-2630 v4 @ 2.20GHz machine.
>
> device_shutdown 2.95s
> -----------------------------
> mlx4_shutdown 1.14s
> megasas_shutdown 0.24s
> ixgbe_shutdown 0.37s x 4 (four ixgbe devices on this machine).
> the rest 0.09s
>
> In mlx4 we spent the most time, but that is because there is a 1 second
> sleep, which is defined by hardware specifications:
> mlx4_shutdown
> mlx4_unload_one
> mlx4_free_ownership
> msleep(1000)
>
> With megasas we spend a quarter of a second, but sometimes longer (up-to
> 0.5s) in this path:
>
> megasas_shutdown
> megasas_flush_cache
> megasas_issue_blocked_cmd
> wait_event_timeout
>
> Finally, with ixgbe_shutdown() it takes 0.37 for each device, but that time
> is spread all over the place, with bigger offenders:
>
> ixgbe_shutdown
> __ixgbe_shutdown
> ixgbe_close_suspend
> ixgbe_down
> ixgbe_init_hw_generic
> ixgbe_reset_hw_X540
> msleep(100); 0.104483472
> ixgbe_get_san_mac_addr_generic 0.048414851
> ixgbe_get_wwn_prefix_generic 0.048409893
> ixgbe_start_hw_X540
> ixgbe_start_hw_generic
> ixgbe_clear_hw_cntrs_generic 0.048581502
> ixgbe_setup_fc_generic 0.024225800
>
> All the ixgbe_*generic functions end-up calling:
> ixgbe_read_eerd_X540()
> ixgbe_acquire_swfw_sync_X540
> usleep_range(5000, 6000);
> ixgbe_release_swfw_sync_X540
> usleep_range(5000, 6000);
>
> While these are short sleeps, they end-up calling them over 24 times!
> 24 * 0.0055s = 0.132s. Adding-up to 0.528s for four devices. Also we have
> four msleep(100). Totaling to: 0.928s
>
> While we should keep optimizing the individual device drivers, in some
> cases this is simply a hardware property that forces a specific delay, and
> we must wait.
>
> So, the solution for this problem is to shutdown devices in parallel.
> However, we must shutdown children before shutting down parents, so parent
> device must wait for its children to finish.
>
> With this patch, on the same machine devices_shutdown() takes 1.142s, and
> without mlx4 one second delay only 0.38s
>
> This feature can be optionally disabled via kernel parameter:
> device_shutdown_serial. When booted with this parameter, device_shutdown()
> will shutdown devices one by one.
>
> Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
> ---
> drivers/base/core.c | 292 ++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 242 insertions(+), 50 deletions(-)
Can you refactor this to be at least 2 patches? One that moves code
around to comon functions to make the second patch, that adds the new
functionality, easier to review and understand?
And I echo the "don't use kerneldoc for static functions" review
comment, that's not needed at all.
thanks,
greg k-h
^ permalink raw reply
* Re: possible deadlock in sk_diag_fill
From: Dmitry Vyukov @ 2018-05-15 7:26 UTC (permalink / raw)
To: Andrei Vagin; +Cc: syzbot, avagin, David Miller, LKML, netdev, syzkaller-bugs
In-Reply-To: <20180515061854.GA30523@outlook.office365.com>
On Tue, May 15, 2018 at 8:18 AM, Andrei Vagin <avagin@virtuozzo.com> wrote:
>> >> >> Hello,
>> >> >>
>> >> >> syzbot found the following crash on:
>> >> >>
>> >> >> HEAD commit: c1c07416cdd4 Merge tag 'kbuild-fixes-v4.17' of git://git.k..
>> >> >> git tree: upstream
>> >> >> console output: https://syzkaller.appspot.com/x/log.txt?x=12164c97800000
>> >> >> kernel config: https://syzkaller.appspot.com/x/.config?x=5a1dc06635c10d27
>> >> >> dashboard link: https://syzkaller.appspot.com/bug?extid=c1872be62e587eae9669
>> >> >> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>> >> >> userspace arch: i386
>> >> >>
>> >> >> Unfortunately, I don't have any reproducer for this crash yet.
>> >> >>
>> >> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> >> >> Reported-by: syzbot+c1872be62e587eae9669@syzkaller.appspotmail.com
>> >> >>
>> >> >>
>> >> >> ======================================================
>> >> >> WARNING: possible circular locking dependency detected
>> >> >> 4.17.0-rc3+ #59 Not tainted
>> >> >> ------------------------------------------------------
>> >> >> syz-executor1/25282 is trying to acquire lock:
>> >> >> 000000004fddf743 (&(&u->lock)->rlock/1){+.+.}, at: sk_diag_dump_icons
>> >> >> net/unix/diag.c:82 [inline]
>> >> >> 000000004fddf743 (&(&u->lock)->rlock/1){+.+.}, at:
>> >> >> sk_diag_fill.isra.5+0xa43/0x10d0 net/unix/diag.c:144
>> >> >>
>> >> >> but task is already holding lock:
>> >> >> 00000000b6895645 (rlock-AF_UNIX){+.+.}, at: spin_lock
>> >> >> include/linux/spinlock.h:310 [inline]
>> >> >> 00000000b6895645 (rlock-AF_UNIX){+.+.}, at: sk_diag_dump_icons
>> >> >> net/unix/diag.c:64 [inline]
>> >> >> 00000000b6895645 (rlock-AF_UNIX){+.+.}, at: sk_diag_fill.isra.5+0x94e/0x10d0
>> >> >> net/unix/diag.c:144
>> >> >>
>> >> >> which lock already depends on the new lock.
>> >> >
>> >> > In the code, we have a comment which explains why it is safe to take this lock
>> >> >
>> >> > /*
>> >> > * The state lock is outer for the same sk's
>> >> > * queue lock. With the other's queue locked it's
>> >> > * OK to lock the state.
>> >> > */
>> >> > unix_state_lock_nested(req);
>> >> >
>> >> > It is a question how to explain this to lockdep.
>> >>
>> >> Do I understand it correctly that (&u->lock)->rlock associated with
>> >> AF_UNIX is locked under rlock-AF_UNIX, and then rlock-AF_UNIX is
>> >> locked under (&u->lock)->rlock associated with AF_NETLINK? If so, I
>> >> think we need to split (&u->lock)->rlock by family too, so that we
>> >> have u->lock-AF_UNIX and u->lock-AF_NETLINK.
>> >
>> > I think here is another problem. lockdep woried about
>> > sk->sk_receive_queue vs unix_sk(s)->lock.
>> >
>> > sk_diag_dump_icons() takes sk->sk_receive_queue and then
>> > unix_sk(s)->lock.
>> >
>> > unix_dgram_sendmsg takes unix_sk(sk)->lock and then sk->sk_receive_queue.
>> >
>> > sk_diag_dump_icons() takes locks for two different sockets, but
>> > unix_dgram_sendmsg() takes locks for one socket.
>> >
>> > sk_diag_dump_icons
>> > if (sk->sk_state == TCP_LISTEN) {
>> > spin_lock(&sk->sk_receive_queue.lock);
>> > skb_queue_walk(&sk->sk_receive_queue, skb) {
>> > unix_state_lock_nested(req);
>> > spin_lock_nested(&unix_sk(s)->lock,
>> >
>> >
>> > unix_dgram_sendmsg
>> > unix_state_lock(other)
>> > spin_lock(&unix_sk(s)->lock)
>> > skb_queue_tail(&other->sk_receive_queue, skb);
>> > spin_lock_irqsave(&list->lock, flags);
>>
>>
>> Do you mean the following?
>> There is socket 1 with state lock (S1) and queue lock (Q2), and socket
>> 2 with state lock (S2) and queue lock (Q2). unix_dgram_sendmsg lock
>> S1->Q1. And sk_diag_dump_icons locks Q1->S2.
>> If yes, then this looks pretty much as deadlock. Consider that 2
>> unix_dgram_sendmsg in 2 different threads lock S1 and S2 respectively.
>> Now 2 sk_diag_dump_icons in 2 different threads lock Q1 and Q2
>> respectively. Now sk_diag_dump_icons want to lock S's, and
>> unix_dgram_sendmsg want to lock Q's. Nobody can proceed.
>
> Q1 and S1 belongs to a listen socket, so they can't be taken from
> unix_dgram_sendmsg().
Should we then split Q1/S1 for listening and data sockets? I don't
know it lockdep allows changing lock class on the fly, though. Always
wondered if there was a single reason to mix listening and data
sockets into a single thing on API level...
^ permalink raw reply
* Re: maximal supported bit rate in traffic control
From: Eitan Raviv @ 2018-05-15 7:25 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Dan Kenigsberg
In-Reply-To: <f5797c00-f51f-430c-abf8-db1762f8a993@gmail.com>
Thanks a lot for the fast reply.
However we are using HFSC, and I did not find a 64bit HFSC qdisc. Does
one exits?
If not, are there plans to create one?
Thank you
On Tue, May 15, 2018 at 1:26 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 05/14/2018 03:15 PM, Eitan Raviv wrote:
>> Hi,
>>
>> In RHEVM we use traffic control to enable users to shape their QoS on
>> the networks they use.
>>
>> IIUC from the traffic control man page, the maximal bit rate currently
>> supported (RHEL 7.4) is 2^32 - 1 bytes per sec, which translates to 32
>> Gibit\s if I have done my maths correctly.
>>
>> Does anyone know whether there are any plans to support larger bit
>> rate values upstream or downstream soon or at all?
>>
>
> At least 3 qdisc got 64bit rates :
>
> HTB ( TCA_HTB_RATE64 ), tbf ( TCA_TBF_RATE64), netem ( TCA_NETEM_RATE64 )
>
> No plans yet for others.
>
^ permalink raw reply
* Re: Kernel panic on kernel-3.10.0-693.21.1.el7 in ndisc.h
From: Michal Kubecek @ 2018-05-15 6:55 UTC (permalink / raw)
To: Roman Makhov; +Cc: Stephen Hemminger, Alexander Aring, linux-wpan, netdev
In-Reply-To: <20180514140611.26eec64f@xeon-e3>
On Mon, May 14, 2018 at 02:06:11PM -0700, Stephen Hemminger wrote:
> On Mon, 14 May 2018 21:29:03 +0300 Roman Makhov <roman.makhov@gmail.com> wrote:
> > Thank you for the answer.
> > Unfortunately CentOS goes with these dinosaurs.
> > So we will try to debug the problem in the current one and try to
> > reproduce on the latest kernel.
>
> If you are stuck in old kernels, please bug the CentOs maintainers not
> upstream developers.
Actually, it's not even the dinosaur. Kernels from RHEL (and therefore
CentOS) or SLES differ from the original mainline version they are based
on quite a lot. It's possible that this bug didn't really exist in the
old 3.10 and is related to one of the backports added to CentOS (or
rather RHEL in this case) kernel. People outside of the distribution
have little idea what was backported and why so unless the issue can be
reproduced with mainline kernel they cannot be expected to help.
Michal Kubecek
^ permalink raw reply
* Re: STMMAC driver with TSO enabled issue
From: Bhadram Varka @ 2018-05-15 6:44 UTC (permalink / raw)
To: Jose Abreu, netdev@vger.kernel.org, Joao Pinto
In-Reply-To: <77ff3baf-fec5-f0a2-9fdf-caa63d4944d0@synopsys.com>
Hi Jose,
On 5/10/2018 9:15 PM, Jose Abreu wrote:
>
>
> On 10-05-2018 16:08, Bhadram Varka wrote:
>> Hi Jose,
>>
>> On 5/10/2018 7:59 PM, Jose Abreu wrote:
>>> Hi Bhadram,
>>>
>>> On 10-05-2018 09:55, Jose Abreu wrote:
>>>> ++net-dev
>>>>
>>>> Hi Bhadram,
>>>>
>>>> On 09-05-2018 12:03, Bhadram Varka wrote:
>>>>> Hi,
>>>>>
>>>>> Thanks for responding.
>>>>>
>>>>> Tried below suggested way. Still observing the issue -
>>>> It seems stmmac has a bug in the RX side when using TSO which is
>>>> causing all the RX descriptors to be consumed. The stmmac_rx()
>>>> function will need to be refactored. I will send a fix ASAP.
>>>
>>> Are you using this patch [1] ? Because there is a problem with
>>> the patch. By adding the previously removed call to
>>> stmmac_init_rx_desc() TSO works okay in my setup.
>>>
>>
>> No. I don't have this change in my code base. I am using
>> net-next tree.
>>
>> Can you please post the change for which TSO works ? I can help
>> you with the testing.
>
> It should work with net-next because patch was not merged yet ...
> Please send me the output of "dmesg | grep -i stmmac", since boot
> and your full register values (from 0x0 to 0x12E4).
>
[root@alarm ~]# dmesg | grep -i dwc
[ 6.925005] dwc-eth-dwmac 2490000.ethernet: Cannot get CSR clock
[ 6.933657] dwc-eth-dwmac 2490000.ethernet: no reset control found
[ 6.955325] dwc-eth-dwmac 2490000.ethernet: User ID: 0x10, Synopsys
ID: 0x41
[ 6.962379] dwc-eth-dwmac 2490000.ethernet: DWMAC4/5
[ 6.967434] dwc-eth-dwmac 2490000.ethernet: DMA HW capability
register supported
[ 6.974827] dwc-eth-dwmac 2490000.ethernet: RX Checksum Offload
Engine supported
[ 6.982915] dwc-eth-dwmac 2490000.ethernet: TX Checksum insertion
supported
[ 6.991235] dwc-eth-dwmac 2490000.ethernet: Wake-Up On Lan supported
[ 6.998974] dwc-eth-dwmac 2490000.ethernet: TSO supported
[ 7.006422] dwc-eth-dwmac 2490000.ethernet: TSO feature enabled
[ 7.012581] dwc-eth-dwmac 2490000.ethernet: Enable RX Mitigation via
HW Watchdog Timer
[ 7.236391] dwc-eth-dwmac 2490000.ethernet eth0: device MAC address
4a:d1:e3:58:cb:7a
[ 7.333414] dwc-eth-dwmac 2490000.ethernet eth0: IEEE 1588-2008
Advanced Timestamp supported
[ 7.342441] dwc-eth-dwmac 2490000.ethernet eth0: registered PTP clock
[ 10.157066] dwc-eth-dwmac 2490000.ethernet eth0: Link is Up -
1Gbps/Full - flow control off
[root@alarm ~]# dmesg | grep -i stmma
[ 7.020567] libphy: stmmac: probed
[ 7.316295] Broadcom BCM89610 stmmac-0:00: attached PHY driver
[Broadcom BCM89610] (mii_bus:phy_addr=stmmac-0:00, irq=64)
I will get the register details -
FYI - TSO works fine with single channel. I see the issue only if multi
channel enabled (supports 4 Tx/Rx channels).
--
Thanks,
Bhadram.
^ permalink raw reply
* Re: [PATCH RFC net-next 0/7] net/ipv6: Fix route append and replace use cases
From: Tobin C. Harding @ 2018-05-15 6:38 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Thomas.Winter, idosch, sharpd, roopa
In-Reply-To: <34b6b5df-515f-62ed-7135-142b0895e7d1@gmail.com>
On Tue, May 15, 2018 at 12:25:03AM -0600, David Ahern wrote:
> On 5/15/18 12:12 AM, Tobin C. Harding wrote:
> > Queue questions on how to run your selftests ;)
>
> copy tools/testing/selftests/net/fib_tests.sh to test environment.
>
> Run it with no options to run all tests or to run specific tests:
>
> $ fib_tests.sh -t ipv4_rt
> $ fib_tests.sh -t ipv6_rt
I was joking but thanks!
Tobin
^ permalink raw reply
* Re: [PATCH net-next 2/3] gso: limit udp gso to egress-only virtual devices
From: kbuild test robot @ 2018-05-15 6:34 UTC (permalink / raw)
To: Willem de Bruijn
Cc: kbuild-all, netdev, davem, Willem de Bruijn, Alexander Duyck
In-Reply-To: <20180514230747.118875-3-willemdebruijn.kernel@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2893 bytes --]
Hi Willem,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Willem-de-Bruijn/udp-gso-fixes/20180515-120246
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
drivers/net//team/team.c: In function '__team_compute_features':
>> drivers/net//team/team.c:1030:10: error: 'NETIF_GSO_UDP_L4' undeclared (first use in this function); did you mean 'NETIF_F_GSO_UDP_L4'?
NETIF_GSO_UDP_L4;
^~~~~~~~~~~~~~~~
NETIF_F_GSO_UDP_L4
drivers/net//team/team.c:1030:10: note: each undeclared identifier is reported only once for each function it appears in
vim +1030 drivers/net//team/team.c
996
997 #define TEAM_VLAN_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
998 NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
999 NETIF_F_HIGHDMA | NETIF_F_LRO)
1000
1001 #define TEAM_ENC_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
1002 NETIF_F_RXCSUM | NETIF_F_ALL_TSO)
1003
1004 static void __team_compute_features(struct team *team)
1005 {
1006 struct team_port *port;
1007 u32 vlan_features = TEAM_VLAN_FEATURES & NETIF_F_ALL_FOR_ALL;
1008 netdev_features_t enc_features = TEAM_ENC_FEATURES;
1009 unsigned short max_hard_header_len = ETH_HLEN;
1010 unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE |
1011 IFF_XMIT_DST_RELEASE_PERM;
1012
1013 list_for_each_entry(port, &team->port_list, list) {
1014 vlan_features = netdev_increment_features(vlan_features,
1015 port->dev->vlan_features,
1016 TEAM_VLAN_FEATURES);
1017 enc_features =
1018 netdev_increment_features(enc_features,
1019 port->dev->hw_enc_features,
1020 TEAM_ENC_FEATURES);
1021
1022
1023 dst_release_flag &= port->dev->priv_flags;
1024 if (port->dev->hard_header_len > max_hard_header_len)
1025 max_hard_header_len = port->dev->hard_header_len;
1026 }
1027
1028 team->dev->vlan_features = vlan_features;
1029 team->dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
> 1030 NETIF_GSO_UDP_L4;
1031 team->dev->hard_header_len = max_hard_header_len;
1032
1033 team->dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
1034 if (dst_release_flag == (IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM))
1035 team->dev->priv_flags |= IFF_XMIT_DST_RELEASE;
1036 }
1037
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52976 bytes --]
^ permalink raw reply
* Re: [PATCH RFC net-next 0/7] net/ipv6: Fix route append and replace use cases
From: David Ahern @ 2018-05-15 6:25 UTC (permalink / raw)
To: Tobin C. Harding; +Cc: netdev, Thomas.Winter, idosch, sharpd, roopa
In-Reply-To: <20180515061231.GC10152@eros>
On 5/15/18 12:12 AM, Tobin C. Harding wrote:
> Queue questions on how to run your selftests ;)
copy tools/testing/selftests/net/fib_tests.sh to test environment.
Run it with no options to run all tests or to run specific tests:
$ fib_tests.sh -t ipv4_rt
$ fib_tests.sh -t ipv6_rt
^ permalink raw reply
* Re: possible deadlock in sk_diag_fill
From: Andrei Vagin @ 2018-05-15 6:18 UTC (permalink / raw)
To: Dmitry Vyukov; +Cc: syzbot, avagin, David Miller, LKML, netdev, syzkaller-bugs
In-Reply-To: <CACT4Y+a9v+38=DUpTGu0doaf3xWW02PUzoC7idgwOg+rV2eFTQ@mail.gmail.com>
On Tue, May 15, 2018 at 07:19:39AM +0200, Dmitry Vyukov wrote:
> On Mon, May 14, 2018 at 8:00 PM, Andrei Vagin <avagin@virtuozzo.com> wrote:
> >> >> Hello,
> >> >>
> >> >> syzbot found the following crash on:
> >> >>
> >> >> HEAD commit: c1c07416cdd4 Merge tag 'kbuild-fixes-v4.17' of git://git.k..
> >> >> git tree: upstream
> >> >> console output: https://syzkaller.appspot.com/x/log.txt?x=12164c97800000
> >> >> kernel config: https://syzkaller.appspot.com/x/.config?x=5a1dc06635c10d27
> >> >> dashboard link: https://syzkaller.appspot.com/bug?extid=c1872be62e587eae9669
> >> >> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> >> >> userspace arch: i386
> >> >>
> >> >> Unfortunately, I don't have any reproducer for this crash yet.
> >> >>
> >> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >> >> Reported-by: syzbot+c1872be62e587eae9669@syzkaller.appspotmail.com
> >> >>
> >> >>
> >> >> ======================================================
> >> >> WARNING: possible circular locking dependency detected
> >> >> 4.17.0-rc3+ #59 Not tainted
> >> >> ------------------------------------------------------
> >> >> syz-executor1/25282 is trying to acquire lock:
> >> >> 000000004fddf743 (&(&u->lock)->rlock/1){+.+.}, at: sk_diag_dump_icons
> >> >> net/unix/diag.c:82 [inline]
> >> >> 000000004fddf743 (&(&u->lock)->rlock/1){+.+.}, at:
> >> >> sk_diag_fill.isra.5+0xa43/0x10d0 net/unix/diag.c:144
> >> >>
> >> >> but task is already holding lock:
> >> >> 00000000b6895645 (rlock-AF_UNIX){+.+.}, at: spin_lock
> >> >> include/linux/spinlock.h:310 [inline]
> >> >> 00000000b6895645 (rlock-AF_UNIX){+.+.}, at: sk_diag_dump_icons
> >> >> net/unix/diag.c:64 [inline]
> >> >> 00000000b6895645 (rlock-AF_UNIX){+.+.}, at: sk_diag_fill.isra.5+0x94e/0x10d0
> >> >> net/unix/diag.c:144
> >> >>
> >> >> which lock already depends on the new lock.
> >> >
> >> > In the code, we have a comment which explains why it is safe to take this lock
> >> >
> >> > /*
> >> > * The state lock is outer for the same sk's
> >> > * queue lock. With the other's queue locked it's
> >> > * OK to lock the state.
> >> > */
> >> > unix_state_lock_nested(req);
> >> >
> >> > It is a question how to explain this to lockdep.
> >>
> >> Do I understand it correctly that (&u->lock)->rlock associated with
> >> AF_UNIX is locked under rlock-AF_UNIX, and then rlock-AF_UNIX is
> >> locked under (&u->lock)->rlock associated with AF_NETLINK? If so, I
> >> think we need to split (&u->lock)->rlock by family too, so that we
> >> have u->lock-AF_UNIX and u->lock-AF_NETLINK.
> >
> > I think here is another problem. lockdep woried about
> > sk->sk_receive_queue vs unix_sk(s)->lock.
> >
> > sk_diag_dump_icons() takes sk->sk_receive_queue and then
> > unix_sk(s)->lock.
> >
> > unix_dgram_sendmsg takes unix_sk(sk)->lock and then sk->sk_receive_queue.
> >
> > sk_diag_dump_icons() takes locks for two different sockets, but
> > unix_dgram_sendmsg() takes locks for one socket.
> >
> > sk_diag_dump_icons
> > if (sk->sk_state == TCP_LISTEN) {
> > spin_lock(&sk->sk_receive_queue.lock);
> > skb_queue_walk(&sk->sk_receive_queue, skb) {
> > unix_state_lock_nested(req);
> > spin_lock_nested(&unix_sk(s)->lock,
> >
> >
> > unix_dgram_sendmsg
> > unix_state_lock(other)
> > spin_lock(&unix_sk(s)->lock)
> > skb_queue_tail(&other->sk_receive_queue, skb);
> > spin_lock_irqsave(&list->lock, flags);
>
>
> Do you mean the following?
> There is socket 1 with state lock (S1) and queue lock (Q2), and socket
> 2 with state lock (S2) and queue lock (Q2). unix_dgram_sendmsg lock
> S1->Q1. And sk_diag_dump_icons locks Q1->S2.
> If yes, then this looks pretty much as deadlock. Consider that 2
> unix_dgram_sendmsg in 2 different threads lock S1 and S2 respectively.
> Now 2 sk_diag_dump_icons in 2 different threads lock Q1 and Q2
> respectively. Now sk_diag_dump_icons want to lock S's, and
> unix_dgram_sendmsg want to lock Q's. Nobody can proceed.
Q1 and S1 belongs to a listen socket, so they can't be taken from
unix_dgram_sendmsg().
^ permalink raw reply
* Re: [PATCH RFC net-next 0/7] net/ipv6: Fix route append and replace use cases
From: Tobin C. Harding @ 2018-05-15 6:12 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Thomas.Winter, idosch, sharpd, roopa
In-Reply-To: <458fe36b-355a-f11e-32b4-8a139c26a298@gmail.com>
On Mon, May 14, 2018 at 11:54:08PM -0600, David Ahern wrote:
> On 5/14/18 11:21 PM, Tobin C. Harding wrote:
> > Hi David,
> >
> > On Mon, May 14, 2018 at 07:51:05PM -0700, David Ahern wrote:
> >> This patch set fixes a few append and replace uses cases for IPv6 and
> >> adds test cases that codifies the expectations of how append and replace
> >> are expected to work.
> >
> > Nood question: what commit does this apply on top of please. I
> > attempted to apply the set on top of net-next
> >
> > commit (961423f9fcbc Merge branch 'sctp-Introduce-sctp_flush_ctx')
> >
> > patch 4 and 5 have merge conflicts (I stopped at 5).
>
> Base commit is:
>
> commit 289e1f4e9e4a09c73a1c0152bb93855ea351ccda
> Author: Anders Roxell <anders.roxell@linaro.org>
> Date: Sun May 13 21:48:30 2018 +0200
>
> net: ipv4: ipconfig: fix unused variable
>
>
>
> I don't see 961423f9fcbc in any tree.
Got it, thanks.
FTR
$ date
Tuesday 15 May 16:05:51 AEST 2018
$ git status
On branch net-next
Your branch is up-to-date with 'net-next/master'.
nothing to commit, working directory clean
$ git log -49 --pretty=oneline --abbrev-commit --reverse | grep 'ipv4: ipconfig: fix unused variable'
289e1f4e9e4a net: ipv4: ipconfig: fix unused variable
$ git log -48 --pretty=oneline --abbrev-commit --reverse | grep 'ipv4: ipconfig: fix unused variable'
Queue questions on how to run your selftests ;)
thanks,
Tobin.
^ 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