* Re: [PATCH net-next v2 0/3] ethtool: support for forward error correction mode setting on a link
From: Jakub Kicinski @ 2017-07-28 16:46 UTC (permalink / raw)
To: Roopa Prabhu
Cc: davem@davemloft.net, John W. Linville, netdev@vger.kernel.org,
Vidya Sagar Ravipati, Dustin Byford, Dave Olson, Casey Leedom,
Gal Pressman, Andrew Lunn, Manoj Malviya, Santosh Rastapur,
yuval.mintz, odedw, Ariel Almog, Jeff Kirsher
In-Reply-To: <CAJieiUiXuN9s0ndJ4K6UVE2pomRi0aD9PH6uYHe12qCzL0+wSg@mail.gmail.com>
On Fri, 28 Jul 2017 07:53:01 -0700, Roopa Prabhu wrote:
> On Thu, Jul 27, 2017 at 7:33 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> > On Thu, 27 Jul 2017 16:47:25 -0700, Roopa Prabhu wrote:
> >> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>
> >> Forward Error Correction (FEC) modes i.e Base-R
> >> and Reed-Solomon modes are introduced in 25G/40G/100G standards
> >> for providing good BER at high speeds. Various networking devices
> >> which support 25G/40G/100G provides ability to manage supported FEC
> >> modes and the lack of FEC encoding control and reporting today is a
> >> source for interoperability issues for many vendors.
> >> FEC capability as well as specific FEC mode i.e. Base-R
> >> or RS modes can be requested or advertised through bits D44:47 of base link
> >> codeword.
> >>
> >> This patch set intends to provide option under ethtool to manage and
> >> report FEC encoding settings for networking devices as per IEEE 802.3
> >> bj, bm and by specs.
> >>
> >> v2 :
> >> - minor patch format fixes and typos pointed out by Andrew
> >> - there was a pending discussion on the use of 'auto' vs
> >> 'automatic' for fec settings. I have left it as 'auto'
> >> because in most cases today auto is used in place of
> >> automatic to represent automatically generated values.
> >> We use it in other networking config too. I would prefer
> >> leaving it as auto.
> >
> > On the subject of resetting the values when module is replugged I
> > assume what was previously described remains:
> > - we always allow users to set the FEC regardless of the module type;
> > - if user set an incorrect FEC for the module type (or module gets
> > swapped) the link will be administratively taken down by either
> > the driver or FW.
> >
> > Is that correct? Am I misremembering?
>
> yes, correct. And possible future sfp hotplug events can give user-space
> more info to react to module type changes etc.
OK, if nobody else objects and we go with that - lets make sure we
document clearly those are expected :) My concern is that if there is
ever 10G + RS FEC standard we don't want to end up in a situation where
some drivers silently ignore FEC settings in 10G and other apply it.
So let's make it clear what the intended Linux behaviour is. It could
be in the ethtool man page, or the kernel somewhere.
^ permalink raw reply
* Long stalls creating a new netns after a netns with a SMB client exits
From: Rolf Neugebauer @ 2017-07-28 16:47 UTC (permalink / raw)
To: Linux Kernel Network Developers, Rolf Neugebauer
Hi
several docker users reported long stalls when they used containers
with network filesystem mounts. For example, this was reported here:
https://github.com/moby/moby/issues/5618#issuecomment-314515980.
Another user, Pierre Carru (@piec on GH), then managed to create a
simpler reproduction here: https://github.com/piec/docker-samba-loop
which I analysed initially here:
https://github.com/moby/moby/issues/5618#issuecomment-318432218
I managed to condense a repro down to a simple script below, which
does not rely on docker.
- Configure and start a SMB server on the host
- Create a veth pair and configure one peer in the root namespace
- Create a network namespace and move and configure the other veth peer there
- Execute a (mount.cifs; ls; unmount) inside the network namespace
(and in its own mount namespace, though the mount namespace is not
strictly required)
- Direclty after the 'unmount', delete the network namespace and try
to create a new network namespace
Creating the new namespace is stalling for around 200 seconds and
there 20 odd messages on the console, like:
[ 67.372603] unregister_netdevice: waiting for lo to become free.
Usage count = 1
Adding a 'sleep 1' before deleting the original network namespace
"solves" the issue, but that doesn't sound like a good fix. Not using
unmount also does not help (understandable).
While the creation of the new namespace is stalled, I used 'sysrq' a
few times to dump the work queues. There is an example below. Also,
the hung task detection kicks in after 120 seconds (also below)
I can readily reproduce this on 4.9.39, 4.11.12 and another user
repro-ed it on 4.12.3. It seems to happen every time. At least one
user reported issues with NFS mounts as well, but we were not able to
reproduce it. It's not clear to me if this is directly related to
'mount.cifs' or if that just happens to reliably repro it.
It would be great if someone more familiar with the code could take a
look. I'm happy to provide additional info (perf traces etc) or test
patches if needed.
Thanks
Rolf
Work queue dump:
----------------
[ 67.372603] unregister_netdevice: waiting for lo to become free.
Usage count = 1
[ 76.821394] sysrq: SysRq : Show Blocked State
[ 76.821820] task PC stack pid father
[ 76.822394] kworker/u2:0 D 0 6 2 0x00000000
[ 76.822896] Workqueue: netns cleanup_net
[ 76.823216] 0000000000018980 0000000000000000 ffff99797a80f080
ffffffff89c10500
[ 76.824007] ffff99797c9980c0 ffff99797cc18980 ffffffff897cfc83
0000000000000002
[ 76.824809] ffff99797c9980c0 ffffb3580002fd00 ffffb3580002fd28
0000000000000001
[ 76.825551] Call Trace:
[ 76.826001] [<ffffffff897cfc83>] ? __schedule+0x364/0x465
[ 76.826468] [<ffffffff897cfe02>] ? schedule+0x7e/0x87
[ 76.826913] [<ffffffff897d1b0a>] ? schedule_timeout+0xc1/0x101
[ 76.827431] [<ffffffff89127ba6>] ? del_timer_sync+0x42/0x42
[ 76.827875] [<ffffffff89127f62>] ? msleep+0x1a/0x1d
[ 76.828328] [<ffffffff89127f62>] ? msleep+0x1a/0x1d
[ 76.828783] [<ffffffff8963ba0b>] ? netdev_run_todo+0x158/0x296
[ 76.829311] [<ffffffff89636cf4>] ? default_device_exit_batch+0x138/0x158
[ 76.829907] [<ffffffff8910ea06>] ? __wake_up_sync+0x9/0x9
[ 76.830411] [<ffffffff896308e1>] ? cleanup_net+0x1a1/0x252
[ 76.830973] [<ffffffff890f2adb>] ? process_one_work+0x185/0x287
[ 76.832052] [<ffffffff890f30a5>] ? worker_thread+0x1d8/0x2ab
[ 76.833063] [<ffffffff890f2ecd>] ? rescuer_thread+0x2c4/0x2c4
[ 76.833769] [<ffffffff890f739c>] ? kthread+0xb4/0xbc
[ 76.834350] [<ffffffff890f72e8>] ? init_completion+0x1d/0x1d
[ 76.834859] [<ffffffff897d2a55>] ? ret_from_fork+0x25/0x30
[ 76.835644] ip D 0 656 653 0x00000000
[ 76.836260] 0000000000018980 0000000000000000 ffff99796ca68840
ffffffff89c10500
[ 76.836960] ffff99796cb9ce80 ffff99797cc18980 ffffffff897cfc83
0000000000000002
[ 76.837665] ffff99796cb9ce80 ffffb35800433e60 ffffffff89d006e4
ffff99796cb9ce80
[ 76.838369] Call Trace:
[ 76.838604] [<ffffffff897cfc83>] ? __schedule+0x364/0x465
[ 76.839126] [<ffffffff897cfe02>] ? schedule+0x7e/0x87
[ 76.839525] [<ffffffff897cffcd>] ? schedule_preempt_disabled+0xa/0xb
[ 76.840139] [<ffffffff897d10f1>] ? __mutex_lock_slowpath+0xb6/0x13b
[ 76.840751] [<ffffffff897d1191>] ? mutex_lock+0x1b/0x2a
[ 76.841234] [<ffffffff897d1191>] ? mutex_lock+0x1b/0x2a
[ 76.841829] [<ffffffff89630a36>] ? copy_net_ns+0xa4/0x12c
[ 76.842335] [<ffffffff890f848d>] ? create_new_namespaces+0x125/0x191
[ 76.842859] [<ffffffff890f8675>] ? unshare_nsproxy_namespaces+0x87/0xa4
[ 76.843788] [<ffffffff890dd418>] ? SyS_unshare+0x17b/0x306
[ 76.844263] [<ffffffff897d27f7>] ? entry_SYSCALL_64_fastpath+0x1a/0xa9
[ 77.648626] unregister_netdevice: waiting for lo to become free.
Usage count = 1
Hung task detection
-------------------
[ 241.612198] unregister_netdevice: waiting for lo to become free.
Usage count = 1
[ 243.955712] INFO: task ip:656 blocked for more than 120 seconds.
[ 243.956292] Not tainted 4.9.39-linuxkit #1
[ 243.956703] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[ 243.957394] ip D 0 656 653 0x00000000
[ 243.957963] 0000000000018980 0000000000000000 ffff99796ca68840
ffffffff89c10500
[ 243.958701] ffff99796cb9ce80 ffff99797cc18980 ffffffff897cfc83
0000000000000002
[ 243.959438] ffff99796cb9ce80 ffffb35800433e60 ffffffff89d006e4
ffff99796cb9ce80
[ 243.960175] Call Trace:
[ 243.960482] [<ffffffff897cfc83>] ? __schedule+0x364/0x465
[ 243.961063] [<ffffffff897cfe02>] ? schedule+0x7e/0x87
[ 243.961538] [<ffffffff897cffcd>] ? schedule_preempt_disabled+0xa/0xb
[ 243.962052] [<ffffffff897d10f1>] ? __mutex_lock_slowpath+0xb6/0x13b
[ 243.962642] [<ffffffff897d1191>] ? mutex_lock+0x1b/0x2a
[ 243.963156] [<ffffffff897d1191>] ? mutex_lock+0x1b/0x2a
[ 243.963649] [<ffffffff89630a36>] ? copy_net_ns+0xa4/0x12c
[ 243.964166] [<ffffffff890f848d>] ? create_new_namespaces+0x125/0x191
[ 243.964757] [<ffffffff890f8675>] ? unshare_nsproxy_namespaces+0x87/0xa4
[ 243.965381] [<ffffffff890dd418>] ? SyS_unshare+0x17b/0x306
[ 243.965898] [<ffffffff897d27f7>] ? entry_SYSCALL_64_fastpath+0x1a/0xa9
[ 251.877100] unregister_netdevice: waiting for lo to become free.
Usage count = 1
[ 262.139630] unregister_netdevice: waiting for lo to become free.
Usage count = 1
Script to repro:
----------------
apk add --no-cache iproute2 samba samba-common-tools cifs-utils
# For debian/ubuntu
# apt-get install -y samba cifs-utils
# SMB server setup
cat <<EOF > /etc/samba/smb.conf
[global]
workgroup = WORKGROUP
netbios name = FOO
passdb backend = tdbsam
security = user
guest account = nobody
strict locking = no
min protocol = SMB2
[public]
path = /share
browsable = yes
read only = no
guest ok = yes
browseable = yes
create mask = 777
EOF
adduser -D -G nobody nobody && smbpasswd -a -n nobody
mkdir /share && chmod ugo+rwx /share && touch /share/foo
chown -R nobody.nobody /share
# Start SMB server and sleep for it to serve
smbd -D
# Bring up a veth pair
ip link add hdev type veth peer name nsdev
ip addr add 10.0.0.1/24 dev hdev
ip link set hdev up
# Create namespace and configure veth peer
ip netns add client-ns
ip link set nsdev netns client-ns
ip netns exec client-ns ip addr add 10.0.0.2/24 dev nsdev
ip netns exec client-ns ip link set lo up
ip netns exec client-ns ip link set nsdev up
sleep 1 # Wait for device to be up
# Execute (mount, ls, unmount) in the network namespace and a new
mount namespace
ip netns exec client-ns unshare --mount \
/bin/sh -c 'mount.cifs //10.0.0.1/public /mnt -o vers=3.0,guest;
ls /mnt; umount /mnt'
# Delete the client network namespace.
ip netns del client-ns
# create a new namespace. This stalls
ip netns add client-ns2
^ permalink raw reply
* Re: rt2x00: make const array glrt_table static
From: Kalle Valo @ 2017-07-28 16:51 UTC (permalink / raw)
To: Colin Ian King
Cc: Stanislaw Gruszka, Helmut Schaa, linux-wireless, kernel-janitors,
netdev
In-Reply-To: <20170728150130.336D06095E@smtp.codeaurora.org>
Kalle Valo <kvalo@codeaurora.org> writes:
> Colin Ian King <colin.king@canonical.com> wrote:
>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Don't populate array glrt_table on the stack but make it static.
>> Makes the object code a smaller by over 670 bytes:
>>
>> Before:
>> text data bss dec hex filename
>> 131772 4733 0 136505 21539 rt2800lib.o
>>
>> After:
>> text data bss dec hex filename
>> 131043 4789 0 135832 21298 rt2800lib.o
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>
>
> Patch applied to wireless-drivers-next.git, thanks.
>
> 17830147c40a mwifiex: fix spelling mistake: "Insuffient" -> "Insufficient"
Dave had applied this earlier and my script failed to detect that. Hence
the commit above is wrong.
commit bf98bd0be1ba509c5e6d77524ffac192f1edb2dd
Author: Colin Ian King <colin.king@canonical.com>
AuthorDate: Tue Jul 11 12:47:33 2017 +0100
Commit: David S. Miller <davem@davemloft.net>
CommitDate: Thu Jul 13 09:23:56 2017 -0700
rt2x00: make const array glrt_table static
--
Kalle Valo
^ permalink raw reply
* Re: [v3,04/10] ipw2200: constify attribute_group structures
From: Kalle Valo @ 2017-07-28 16:54 UTC (permalink / raw)
To: Arvind Yadav
Cc: davem, stas.yakovlev, sgruszka, netdev, linux-kernel,
linux-wireless
In-Reply-To: <87inicwqe2.fsf@purkki.adurom.net>
Kalle Valo <kvalo@codeaurora.org> writes:
> Kalle Valo <kvalo@codeaurora.org> writes:
>
>> Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
>>
>>> attribute_group are not supposed to change at runtime. All functions
>>> working with attribute_group provided by <linux/sysfs.h> work
>>> with const attribute_group. So mark the non-const structs as const.
>>>
>>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>>
>> 4 patches applied to wireless-drivers-next.git, thanks.
>>
>> 17830147c40a mwifiex: fix spelling mistake: "Insuffient" -> "Insufficient"
>> 185ffc194800 ipw2100: don't return positive values to PCI probe on error
>> af643fe9bbe0 zd1211rw: fix spelling mistake 'hybernate' -> 'hibernate'
>> e66d70b789d1 brcmfmac: constify pci_device_id
>
> Please ignore this mail, I have a bug in my patchwork script.
The reason was that Dave had already applied these patches but my script
failed to detect that. I need to fix my script.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next 4/4] net: dsa: lan9303: MDIO access phy registers directly
From: Vivien Didelot @ 2017-07-28 16:55 UTC (permalink / raw)
To: Egil Hjelmeland, andrew, f.fainelli, netdev, linux-kernel, kernel
In-Reply-To: <50d2d34c-c1a5-7f0a-90f1-44b1be1775b7@egil-hjelmeland.no>
Hi Egil,
Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
>>> +const struct lan9303_phy_ops lan9303_indirect_phy_ops = {
>>> + .phy_read = lan9303_indirect_phy_read,
>>> + .phy_write = lan9303_indirect_phy_write,
>>> +};
>>> +EXPORT_SYMBOL(lan9303_indirect_phy_ops);
>>
>> Isn't EXPORT_SYMBOL_GPL prefered over EXPORT_SYMBOL?
>
> I have no opinion. I just used the same variant as the other EXPORTS
> in the file.
If there is no concern from others about this, LGTM too:
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: dsa: lan9303: Fix lan9303_detect_phy_setup() for MDIO
From: Florian Fainelli @ 2017-07-28 17:02 UTC (permalink / raw)
To: Egil Hjelmeland, andrew, vivien.didelot, netdev, linux-kernel,
kernel
In-Reply-To: <20170728151157.28983-2-privat@egil-hjelmeland.no>
On 07/28/2017 08:11 AM, Egil Hjelmeland wrote:
> Handle that MDIO read with no response return 0xffff.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v3 3/3] net: ethernet: ti: cpts: fix tx timestamping timeout
From: Ivan Khoronzhuk @ 2017-07-28 17:02 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Richard Cochran, Sekhar Nori,
linux-kernel, linux-omap, Wingman Kwok, John Stultz,
Thomas Gleixner
In-Reply-To: <20170726221138.12986-4-grygorii.strashko@ti.com>
On Wed, Jul 26, 2017 at 05:11:38PM -0500, Grygorii Strashko wrote:
> With the low speed Ethernet connection CPDMA notification about packet
> processing can be received before CPTS TX timestamp event, which is set
> when packet actually left CPSW while cpdma notification is sent when packet
> pushed in CPSW fifo. As result, when connection is slow and CPU is fast
> enough TX timestamping is not working properly.
>
> Fix it, by introducing TX SKB queue to store PTP SKBs for which Ethernet
> Transmit Event hasn't been received yet and then re-check this queue
> with new Ethernet Transmit Events by scheduling CPTS overflow
> work more often (every 1 jiffies) until TX SKB queue is not empty.
>
> Side effect of this change is:
> - User space tools require to take into account possible delay in TX
> timestamp processing (for example ptp4l works with tx_timestamp_timeout=400
> under net traffic and tx_timestamp_timeout=25 in idle).
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> drivers/net/ethernet/ti/cpts.c | 86 ++++++++++++++++++++++++++++++++++++++++--
> drivers/net/ethernet/ti/cpts.h | 1 +
> 2 files changed, 84 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
> index 3ed438e..c2121d2 100644
> --- a/drivers/net/ethernet/ti/cpts.c
> +++ b/drivers/net/ethernet/ti/cpts.c
> @@ -31,9 +31,18 @@
>
> #include "cpts.h"
>
> +#define CPTS_SKB_TX_WORK_TIMEOUT 1 /* jiffies */
> +
> +struct cpts_skb_cb_data {
> + unsigned long tmo;
> +};
> +
> #define cpts_read32(c, r) readl_relaxed(&c->reg->r)
> #define cpts_write32(c, v, r) writel_relaxed(v, &c->reg->r)
>
> +static int cpts_match(struct sk_buff *skb, unsigned int ptp_class,
> + u16 ts_seqid, u8 ts_msgtype);
> +
> static int event_expired(struct cpts_event *event)
> {
> return time_after(jiffies, event->tmo);
> @@ -77,6 +86,47 @@ static int cpts_purge_events(struct cpts *cpts)
> return removed ? 0 : -1;
> }
>
> +static bool cpts_match_tx_ts(struct cpts *cpts, struct cpts_event *event)
> +{
> + struct sk_buff *skb, *tmp;
> + u16 seqid;
> + u8 mtype;
> + bool found = false;
> +
> + mtype = (event->high >> MESSAGE_TYPE_SHIFT) & MESSAGE_TYPE_MASK;
> + seqid = (event->high >> SEQUENCE_ID_SHIFT) & SEQUENCE_ID_MASK;
> +
> + /* no need to grab txq.lock as access is always done under cpts->lock */
> + skb_queue_walk_safe(&cpts->txq, skb, tmp) {
> + struct skb_shared_hwtstamps ssh;
> + unsigned int class = ptp_classify_raw(skb);
> + struct cpts_skb_cb_data *skb_cb =
> + (struct cpts_skb_cb_data *)skb->cb;
> +
> + if (cpts_match(skb, class, seqid, mtype)) {
> + u64 ns = timecounter_cyc2time(&cpts->tc, event->low);
> +
> + memset(&ssh, 0, sizeof(ssh));
> + ssh.hwtstamp = ns_to_ktime(ns);
> + skb_tstamp_tx(skb, &ssh);
> + found = true;
> + __skb_unlink(skb, &cpts->txq);
> + dev_consume_skb_any(skb);
> + dev_dbg(cpts->dev, "match tx timestamp mtype %u seqid %04x\n",
> + mtype, seqid);
> + } else if (time_after(jiffies, skb_cb->tmo)) {
> + /* timeout any expired skbs over 1s */
> + dev_dbg(cpts->dev,
> + "expiring tx timestamp mtype %u seqid %04x\n",
> + mtype, seqid);
> + __skb_unlink(skb, &cpts->txq);
> + dev_consume_skb_any(skb);
> + }
> + }
> +
> + return found;
> +}
> +
> /*
> * Returns zero if matching event type was found.
> */
> @@ -101,9 +151,15 @@ static int cpts_fifo_read(struct cpts *cpts, int match)
> event->low = lo;
> type = event_type(event);
> switch (type) {
> + case CPTS_EV_TX:
> + if (cpts_match_tx_ts(cpts, event)) {
> + /* if the new event matches an existing skb,
> + * then don't queue it
> + */
> + break;
> + }
> case CPTS_EV_PUSH:
> case CPTS_EV_RX:
> - case CPTS_EV_TX:
> list_del_init(&event->list);
> list_add_tail(&event->list, &cpts->events);
> break;
> @@ -229,8 +285,15 @@ static long cpts_overflow_check(struct ptp_clock_info *ptp)
> struct cpts *cpts = container_of(ptp, struct cpts, info);
> unsigned long delay = cpts->ov_check_period;
> struct timespec64 ts;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&cpts->lock, flags);
> + ts = ns_to_timespec64(timecounter_read(&cpts->tc));
> +
> + if (!skb_queue_empty(&cpts->txq))
> + delay = CPTS_SKB_TX_WORK_TIMEOUT;
> + spin_unlock_irqrestore(&cpts->lock, flags);
>
> - cpts_ptp_gettime(&cpts->info, &ts);
> pr_debug("cpts overflow check at %lld.%09lu\n", ts.tv_sec, ts.tv_nsec);
> return (long)delay;
> }
> @@ -301,7 +364,7 @@ static u64 cpts_find_ts(struct cpts *cpts, struct sk_buff *skb, int ev_type)
> return 0;
>
> spin_lock_irqsave(&cpts->lock, flags);
> - cpts_fifo_read(cpts, CPTS_EV_PUSH);
> + cpts_fifo_read(cpts, -1);
Seems it should be explained or send as separate patch before this one.
Not sure, but smth like "Don't stop read fifo if PUSH event is found",
but how it can happen that push event is present w/o TS_PUSH request..,
anyway, if there is some valuable reason for that - should be explained.
> list_for_each_safe(this, next, &cpts->events) {
> event = list_entry(this, struct cpts_event, list);
> if (event_expired(event)) {
> @@ -319,6 +382,19 @@ static u64 cpts_find_ts(struct cpts *cpts, struct sk_buff *skb, int ev_type)
> break;
> }
> }
> +
> + if (ev_type == CPTS_EV_TX && !ns) {
> + struct cpts_skb_cb_data *skb_cb =
> + (struct cpts_skb_cb_data *)skb->cb;
> + /* Not found, add frame to queue for processing later.
> + * The periodic FIFO check will handle this.
> + */
> + skb_get(skb);
> + /* get the timestamp for timeouts */
> + skb_cb->tmo = jiffies + msecs_to_jiffies(100);
> + __skb_queue_tail(&cpts->txq, skb);
> + ptp_schedule_worker(cpts->clock, 0);
> + }
As I see no need in separate irq handler for cpts events after this.
Is that caused by h/w limitations on keystone2?
And what about rx path introduced in RFC, is it not needed any more or are
you going to send additional series?
> spin_unlock_irqrestore(&cpts->lock, flags);
>
> return ns;
> @@ -360,6 +436,7 @@ int cpts_register(struct cpts *cpts)
> {
> int err, i;
>
> + skb_queue_head_init(&cpts->txq);
> INIT_LIST_HEAD(&cpts->events);
> INIT_LIST_HEAD(&cpts->pool);
> for (i = 0; i < CPTS_MAX_EVENTS; i++)
> @@ -400,6 +477,9 @@ void cpts_unregister(struct cpts *cpts)
> cpts_write32(cpts, 0, int_enable);
> cpts_write32(cpts, 0, control);
>
> + /* Drop all packet */
> + skb_queue_purge(&cpts->txq);
> +
> clk_disable(cpts->refclk);
> }
> EXPORT_SYMBOL_GPL(cpts_unregister);
> diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
> index 586edd9..73d73fa 100644
> --- a/drivers/net/ethernet/ti/cpts.h
> +++ b/drivers/net/ethernet/ti/cpts.h
> @@ -125,6 +125,7 @@ struct cpts {
> struct list_head pool;
> struct cpts_event pool_data[CPTS_MAX_EVENTS];
> unsigned long ov_check_period;
> + struct sk_buff_head txq;
> };
>
> void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);
> --
> 2.10.1
>
^ permalink raw reply
* Re: [PATCH net-next 2/4] net: dsa: lan9303: Multiply by 4 to get MDIO register
From: Florian Fainelli @ 2017-07-28 17:02 UTC (permalink / raw)
To: Egil Hjelmeland, andrew, vivien.didelot, netdev, linux-kernel,
kernel
In-Reply-To: <20170728151157.28983-3-privat@egil-hjelmeland.no>
On 07/28/2017 08:11 AM, Egil Hjelmeland wrote:
> lan9303_mdio_write()/_read() must multiply register number by 4 to get
> offset.
>
> Added some commments to the register definitions.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: dsa: lan9303: Renamed indirect phy access functions
From: Florian Fainelli @ 2017-07-28 17:03 UTC (permalink / raw)
To: Egil Hjelmeland, andrew, vivien.didelot, netdev, linux-kernel,
kernel
In-Reply-To: <20170728151157.28983-4-privat@egil-hjelmeland.no>
On 07/28/2017 08:11 AM, Egil Hjelmeland wrote:
> Preparing for the following fix of MDIO phy access:
>
> Renamed functions that access PHY 1 and 2 indirectly through PMI
> registers.
>
> lan9303_port_phy_reg_wait_for_completion() to
> lan9303_indirect_phy_wait_for_completion()
>
> lan9303_port_phy_reg_read() to
> lan9303_indirect_phy_read()
>
> lan9303_port_phy_reg_write() to
> lan9303_indirect_phy_write()
>
> Also changed "val" parameter of lan9303_indirect_phy_write() to u16,
> for clarity.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 4/4] net: dsa: lan9303: MDIO access phy registers directly
From: Florian Fainelli @ 2017-07-28 17:05 UTC (permalink / raw)
To: Vivien Didelot, Egil Hjelmeland, andrew, netdev, linux-kernel,
kernel
In-Reply-To: <87fudgqz4w.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On 07/28/2017 09:55 AM, Vivien Didelot wrote:
> Hi Egil,
>
> Egil Hjelmeland <privat@egil-hjelmeland.no> writes:
>
>>>> +const struct lan9303_phy_ops lan9303_indirect_phy_ops = {
>>>> + .phy_read = lan9303_indirect_phy_read,
>>>> + .phy_write = lan9303_indirect_phy_write,
>>>> +};
>>>> +EXPORT_SYMBOL(lan9303_indirect_phy_ops);
>>>
>>> Isn't EXPORT_SYMBOL_GPL prefered over EXPORT_SYMBOL?
>>
>> I have no opinion. I just used the same variant as the other EXPORTS
>> in the file.
>
> If there is no concern from others about this, LGTM too:
Since the kernel module license is GPL, EXPORT_SYMBOL_GPL() would seem
to be appropriate, which can be done as a subsequent patch.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: Roopa Prabhu @ 2017-07-28 17:13 UTC (permalink / raw)
To: David Ahern; +Cc: Cong Wang, Hangbin Liu, network dev
In-Reply-To: <7f404f61-0b9a-b25b-3c15-83395d30641d@gmail.com>
On Fri, Jul 28, 2017 at 8:10 AM, David Ahern <dsahern@gmail.com> wrote:
> On 7/27/17 10:56 PM, Cong Wang wrote:
>> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> wrote:
>>> On 7/26/17 12:27 PM, Roopa Prabhu wrote:
>>>> agreed...so looks like the check in v3 should be
>>>>
>>>>
>>>> + if ( rt == net->ipv6.ip6_null_entry ||
>>>> + (rt->dst.error &&
>>>> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES
>>>> + rt != net->ipv6.ip6_prohibit_entry &&
>>>> + rt != net->ipv6.ip6_blk_hole_entry &&
>>>> +#endif
>>>> + )) {
>>>> err = rt->dst.error;
>>>> ip6_rt_put(rt);
>>>> goto errout;
>>>>
>>>
>>> I don't think so. If I add a prohibit route and use the fibmatch
>>> attribute, I want to see the route from the FIB that was matched.
>>
>> But net->ipv6.ip6_prohibit_entry is not the prohibit route you can
>> add in user-space, it is only used by rule actions. So do you really
>> want to dump it?? My gut feeling is no, but I am definitely not sure.
>>
>> When you add a prohibit route, a new rt is allocated dynamically,
>> net->ipv6.ip6_prohibit_entry is relatively static, internal and is the
>> only one per netns. (Same for net->ipv6.ip6_blk_hole_entry)
>>
>> I think Hangbin's example doesn't have ip rules, so this case
>> is not shown up.
>>
>
> Understood. The v4 patch returns getroute to the original behavior. The
> original behavior returned a route entry not just an error code. The
> following is at 5dafc87f40d7 which the commit before roopa's patch set:
>
> # ip -6 ru add to 6000::/120 prohibit
> # ip -6 ro get 6000::1
> prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
> 4294967295 error -13 pref medium
>
> # ip -6 ro add vrf red prohibit 5000::1/120
> # ip -6 ro get vrf red 5000::1
> prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric 1024
> error -13 pref medium
>
> Generically, the only time you get just an error response is when the
> lookup fails to find a match and returns the null_entry which has
> dst.error = -ENETUNREACH.
>
>
> Now to your point about the new fibmatch option I have gone back and
> forth but in the end I think returning the route associated with the FIB
> rule is better than just failing with an error code.
>
> Roopa?
for fibmatch, my original intent was to return with an error code.
This is similar
to the ipv4 behavior. One option is to keep the check in there and put
the 'fibmatch'
condition around it. But, i do want to make sure that for the fibmatch case,
it does not return an error directly on an existing prohibit route
entry in the fib.
This is probably doable by checking for appropriate
net->ipv6.ip6_prohibit_entry entries.
^ permalink raw reply
* refcount_t + (resend to wider audience)
From: Mark Salyzyn @ 2017-07-28 17:15 UTC (permalink / raw)
To: davem, netdev; +Cc: stable
In-Reply-To: <cc109bec-d4bb-8965-3169-fe36d85e6e19@android.com>
(Resend to wider audience to comply with
Documentation/networking/netdev-FAQ.txt)
Please apply/backport the following upstream feature and followup
grouped fixes patches to the stable trees (expect included in at least
3.10.y, 3.18.y, 4.4.y and 4.9.y):
GROUP 1: refcount_t feature:
f405df5de3170c00e5c54f8b7cf4766044a032ba refcount_t: Introduce a special
purpose refcount type
29dee3c03abce04cd527878ef5f9e5f91b7b83f4 locking/refcounts: Out-of-line
everything
bd174169c7a12a37b3b4aa2221f084ade010b182 locking/refcount: Add
refcount_t API kernel-doc comments
fd25d19f6b8da315332bb75936605fb45d3ea981 locking/refcount: Create
unchecked atomic_t implementation
They should merge cleanly as they are orthogonal, except for the last
one which will have a trivial merge conflict to arch/Kconfig.
Following these that add the refcount_t upstream API, the following
groups of
upsteam patches are requested because we are experiencing KASAN errors:
GROUP 2: sk_buff.users:
3889a803e1da9bd7cd10d6504bf281ee7e55dfd6 net: factor out a helper to
decrement the skb refcount
7608894e43d071ef2322a01c79522954c070ac6c net: use skb_unref() in
napi_consume_skb()
9899886d5e8ec5b343b1efe44f185a0e68dc6454 net: core: Prevent from
dereferencing null pointer when releasing SKB
633547973ffc32fd2c815639d4675e1531f0896f net: convert sk_buff.users from
atomic_t to refcount_t
They appear to merge trivially (4.9), but the last one will likely need
#include
<linux/refcount.h> added after the #include <lunux/atomic.h> in
include/linux/skbuff.h to facilitate compilation.
GROUP 3: sec_path.refcnt:
b0fcee825c0ad05057a97d1f4685e1b9e9d00c53 xfrm: Add a secpath_set helper.
55eabed60a68e918abc44f6beb64f38cc008b29d net, xfrm: convert
sec_path.refcnt from atomic_t to refcount_t
They merged with zero conflict on 4.9.
GROUP 4: sock.sk_wmem_alloc
14afee4b6092fde451ee17604e5f5c89da33e71e net: convert sock.sk_wmem_alloc
from atomic_t to refcount_t
This did not merge entirely cleanly (4.9), appears fragments for
net/ipv4/esp4.c and net/ipv6/esp6.c need to be dropped as well as minor
conflicts in two other files.
There may be more related to follow as our investigations continue, but
this is already turning into a tall order so we stop here.
Sincerely -- Mark Salyzyn
^ permalink raw reply
* Re: [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info
From: David Ahern @ 2017-07-28 17:23 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Cong Wang, Roopa Prabhu, David Miller
In-Reply-To: <9e5d3137-c84d-91f7-6c82-813519964513@gmail.com>
On 7/27/17 12:03 PM, David Ahern wrote:
> On 7/27/17 10:25 AM, Hangbin Liu wrote:
>> After commit 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib
>> result when requested"). When we get a prohibit ertry, we will return
>> -EACCES directly instead of dump route info.
>>
>> Fix it by remove the rt->dst.error check.
>>
>> Before fix:
>> \# ip -6 route add prohibit 2003::/64 dev eth1
>> \# ip -6 route get fibmatch 2003::1
>> RTNETLINK answers: Permission denied
>> \# ip -6 route add unreachable 2004::/64 dev eth1
>> \# ip -6 route get fibmatch 2004::1
>> RTNETLINK answers: No route to host
>>
>> After fix:
>> \# ip -6 route add prohibit 2003::/64 dev eth1
>> \# ip -6 route get fibmatch 2003::1
>> prohibit 2003::/64 dev lo metric 1024 error -13 pref medium
>> \# ip -6 route add unreachable 2004::/64 dev eth1
>> \# ip -6 route get fibmatch 2004::1
>> unreachable 2004::/64 dev lo metric 1024 error -113 pref medium
>>
>> Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
>> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>> ---
>> net/ipv6/route.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>
> Acked-by: David Ahern <dsahern@gmail.com>
>
Dave: please hold off on applying this patch.
^ permalink raw reply
* Re: [v4.12 regression] netns: NULL deref in fib_sync_down_dev()
From: Cong Wang @ 2017-07-28 17:28 UTC (permalink / raw)
To: Ido Schimmel; +Cc: Michał Mirosław, Linux Kernel Network Developers
In-Reply-To: <20170728164331.GA5268@splinter>
On Fri, Jul 28, 2017 at 9:43 AM, Ido Schimmel <idosch@idosch.org> wrote:
> On Fri, Jul 28, 2017 at 06:00:47PM +0200, Michał Mirosław wrote:
>> Dear NetDevs,
>>
>> Before I go to bisecting, have you seen a following NULL dereference,
>> yet? Where should I start looking? It is triggered by deleting netns
>> (cut-down script attached - triggers every time). This was working
>> correctly under v4.11.x.
>
> Thanks for the report. I just reproduced this on my system. I believe
> the problem is a missing NULL check for 'in_dev' in
> call_fib_nh_notifiers(). I'll test a fix.
But your commit 982acb97560c8118c2109504a22b0d78a580547d
is merged in v4.11-rc1. How could 4.11.x work correctly?
^ permalink raw reply
* [PATCH net-next 1/2] tcp: extract the function to compute delivery rate
From: Wei Wang @ 2017-07-28 17:28 UTC (permalink / raw)
To: David Miller, netdev; +Cc: Yuchung Cheng, Soheil Hassas Yeganeh, Wei Wang
From: Wei Wang <weiwan@google.com>
Refactor the code to extract the function to compute delivery rate.
This function will be used in later commit.
Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
---
net/ipv4/tcp.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 71ce33decd97..65d210d0394c 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -388,6 +388,19 @@ static int retrans_to_secs(u8 retrans, int timeout, int rto_max)
return period;
}
+static u64 tcp_compute_delivery_rate(const struct tcp_sock *tp)
+{
+ u32 rate = READ_ONCE(tp->rate_delivered);
+ u32 intv = READ_ONCE(tp->rate_interval_us);
+ u64 rate64 = 0;
+
+ if (rate && intv) {
+ rate64 = (u64)rate * tp->mss_cache * USEC_PER_SEC;
+ do_div(rate64, intv);
+ }
+ return rate64;
+}
+
/* Address-family independent initialization for a tcp_sock.
*
* NOTE: A lot of things set to zero explicitly by call to
@@ -2823,7 +2836,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)
{
const struct tcp_sock *tp = tcp_sk(sk); /* iff sk_type == SOCK_STREAM */
const struct inet_connection_sock *icsk = inet_csk(sk);
- u32 now, intv;
+ u32 now;
u64 rate64;
bool slow;
u32 rate;
@@ -2922,13 +2935,9 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)
info->tcpi_data_segs_out = tp->data_segs_out;
info->tcpi_delivery_rate_app_limited = tp->rate_app_limited ? 1 : 0;
- rate = READ_ONCE(tp->rate_delivered);
- intv = READ_ONCE(tp->rate_interval_us);
- if (rate && intv) {
- rate64 = (u64)rate * tp->mss_cache * USEC_PER_SEC;
- do_div(rate64, intv);
+ rate64 = tcp_compute_delivery_rate(tp);
+ if (rate64)
info->tcpi_delivery_rate = rate64;
- }
unlock_sock_fast(sk, slow);
}
EXPORT_SYMBOL_GPL(tcp_get_info);
--
2.14.0.rc0.400.g1c36432dff-goog
^ permalink raw reply related
* [PATCH net-next 2/2] tcp: add related fields into SCM_TIMESTAMPING_OPT_STATS
From: Wei Wang @ 2017-07-28 17:28 UTC (permalink / raw)
To: David Miller, netdev; +Cc: Yuchung Cheng, Soheil Hassas Yeganeh, Wei Wang
In-Reply-To: <20170728172821.102562-1-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Add the following stats into SCM_TIMESTAMPING_OPT_STATS control msg:
TCP_NLA_PACING_RATE
TCP_NLA_DELIVERY_RATE
TCP_NLA_SND_CWND
TCP_NLA_REORDERING
TCP_NLA_MIN_RTT
TCP_NLA_RECUR_RETRANS
TCP_NLA_DELIVERY_RATE_APP_LMT
Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
---
include/uapi/linux/tcp.h | 8 ++++++++
net/ipv4/tcp.c | 20 +++++++++++++++++++-
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index a5507c977497..030e594bab45 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -231,6 +231,14 @@ enum {
TCP_NLA_SNDBUF_LIMITED, /* Time (usec) limited by send buffer */
TCP_NLA_DATA_SEGS_OUT, /* Data pkts sent including retransmission */
TCP_NLA_TOTAL_RETRANS, /* Data pkts retransmitted */
+ TCP_NLA_PACING_RATE, /* Pacing rate in bytes per second */
+ TCP_NLA_DELIVERY_RATE, /* Delivery rate in bytes per second */
+ TCP_NLA_SND_CWND, /* Sending congestion window */
+ TCP_NLA_REORDERING, /* Reordering metric */
+ TCP_NLA_MIN_RTT, /* minimum RTT */
+ TCP_NLA_RECUR_RETRANS, /* Recurring retransmits for the current pkt */
+ TCP_NLA_DELIVERY_RATE_APP_LMT, /* delivery rate application limited ? */
+
};
/* for TCP_MD5SIG socket option */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 65d210d0394c..be78662faa1a 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2947,8 +2947,12 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
const struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *stats;
struct tcp_info info;
+ u64 rate64;
+ u32 rate;
- stats = alloc_skb(5 * nla_total_size_64bit(sizeof(u64)), GFP_ATOMIC);
+ stats = alloc_skb(7 * nla_total_size_64bit(sizeof(u64)) +
+ 3 * nla_total_size(sizeof(u32)) +
+ 2 * nla_total_size(sizeof(u8)), GFP_ATOMIC);
if (!stats)
return NULL;
@@ -2963,6 +2967,20 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
tp->data_segs_out, TCP_NLA_PAD);
nla_put_u64_64bit(stats, TCP_NLA_TOTAL_RETRANS,
tp->total_retrans, TCP_NLA_PAD);
+
+ rate = READ_ONCE(sk->sk_pacing_rate);
+ rate64 = rate != ~0U ? rate : ~0ULL;
+ nla_put_u64_64bit(stats, TCP_NLA_PACING_RATE, rate64, TCP_NLA_PAD);
+
+ rate64 = tcp_compute_delivery_rate(tp);
+ nla_put_u64_64bit(stats, TCP_NLA_DELIVERY_RATE, rate64, TCP_NLA_PAD);
+
+ nla_put_u32(stats, TCP_NLA_SND_CWND, tp->snd_cwnd);
+ nla_put_u32(stats, TCP_NLA_REORDERING, tp->reordering);
+ nla_put_u32(stats, TCP_NLA_MIN_RTT, tcp_min_rtt(tp));
+
+ nla_put_u8(stats, TCP_NLA_RECUR_RETRANS, inet_csk(sk)->icsk_retransmits);
+ nla_put_u8(stats, TCP_NLA_DELIVERY_RATE_APP_LMT, !!tp->rate_app_limited);
return stats;
}
--
2.14.0.rc0.400.g1c36432dff-goog
^ permalink raw reply related
* STABLE: net: reduce skb_warn_bad_offload() noise
From: Mark Salyzyn @ 2017-07-28 17:29 UTC (permalink / raw)
To: stable, netdev, David S. Miller
Please backport the upstream patch to the stable trees (including
3.10.y, 3.18.y, 4.4.y and 4.9.y):
b2504a5dbef3305ef41988ad270b0e8ec289331c net: reduce
skb_warn_bad_offload() noise
Impacting performance or creating unnecessary alarm, and will result in
kernel panic for panic_on_warn configuration.
Sincerely -- Mark Salyzyn
^ permalink raw reply
* Re: [PATCH net-next v2 0/3] ethtool: support for forward error correction mode setting on a link
From: Andrew Lunn @ 2017-07-28 17:29 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Roopa Prabhu, davem@davemloft.net, John W. Linville,
netdev@vger.kernel.org, Vidya Sagar Ravipati, Dustin Byford,
Dave Olson, Casey Leedom, Gal Pressman, Manoj Malviya,
Santosh Rastapur, yuval.mintz, odedw, Ariel Almog, Jeff Kirsher
In-Reply-To: <20170728094620.525ba156@cakuba.netronome.com>
On Fri, Jul 28, 2017 at 09:46:20AM -0700, Jakub Kicinski wrote:
> On Fri, 28 Jul 2017 07:53:01 -0700, Roopa Prabhu wrote:
> > On Thu, Jul 27, 2017 at 7:33 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> > > On Thu, 27 Jul 2017 16:47:25 -0700, Roopa Prabhu wrote:
> > >> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> > >>
> > >> Forward Error Correction (FEC) modes i.e Base-R
> > >> and Reed-Solomon modes are introduced in 25G/40G/100G standards
> > >> for providing good BER at high speeds. Various networking devices
> > >> which support 25G/40G/100G provides ability to manage supported FEC
> > >> modes and the lack of FEC encoding control and reporting today is a
> > >> source for interoperability issues for many vendors.
> > >> FEC capability as well as specific FEC mode i.e. Base-R
> > >> or RS modes can be requested or advertised through bits D44:47 of base link
> > >> codeword.
> > >>
> > >> This patch set intends to provide option under ethtool to manage and
> > >> report FEC encoding settings for networking devices as per IEEE 802.3
> > >> bj, bm and by specs.
> > >>
> > >> v2 :
> > >> - minor patch format fixes and typos pointed out by Andrew
> > >> - there was a pending discussion on the use of 'auto' vs
> > >> 'automatic' for fec settings. I have left it as 'auto'
> > >> because in most cases today auto is used in place of
> > >> automatic to represent automatically generated values.
> > >> We use it in other networking config too. I would prefer
> > >> leaving it as auto.
> > >
> > > On the subject of resetting the values when module is replugged I
> > > assume what was previously described remains:
> > > - we always allow users to set the FEC regardless of the module type;
> > > - if user set an incorrect FEC for the module type (or module gets
> > > swapped) the link will be administratively taken down by either
> > > the driver or FW.
> > >
> > > Is that correct? Am I misremembering?
> >
> > yes, correct. And possible future sfp hotplug events can give user-space
> > more info to react to module type changes etc.
>
> OK, if nobody else objects and we go with that - lets make sure we
> document clearly those are expected :) My concern is that if there is
> ever 10G + RS FEC standard we don't want to end up in a situation where
> some drivers silently ignore FEC settings in 10G and other apply it.
> So let's make it clear what the intended Linux behaviour is. It could
> be in the ethtool man page, or the kernel somewhere.
You might also find this interesting:
https://py3.patchwork.dja.id.au/patch/42846/
Most of the rest of the series has been reviewed, so i don't think it
will be too long before it is in the kernel.
Andrew
^ permalink raw reply
* Re: [PATCH v3 3/3] net: ethernet: ti: cpts: fix tx timestamping timeout
From: Grygorii Strashko @ 2017-07-28 17:30 UTC (permalink / raw)
To: David S. Miller, netdev, Richard Cochran, Sekhar Nori,
linux-kernel, linux-omap, Wingman Kwok, John Stultz,
Thomas Gleixner
In-Reply-To: <20170728170233.GA19760@khorivan>
On 07/28/2017 12:02 PM, Ivan Khoronzhuk wrote:
> On Wed, Jul 26, 2017 at 05:11:38PM -0500, Grygorii Strashko wrote:
>> With the low speed Ethernet connection CPDMA notification about packet
>> processing can be received before CPTS TX timestamp event, which is set
>> when packet actually left CPSW while cpdma notification is sent when packet
>> pushed in CPSW fifo. As result, when connection is slow and CPU is fast
>> enough TX timestamping is not working properly.
>>
>> Fix it, by introducing TX SKB queue to store PTP SKBs for which Ethernet
>> Transmit Event hasn't been received yet and then re-check this queue
>> with new Ethernet Transmit Events by scheduling CPTS overflow
>> work more often (every 1 jiffies) until TX SKB queue is not empty.
>>
>> Side effect of this change is:
>> - User space tools require to take into account possible delay in TX
>> timestamp processing (for example ptp4l works with tx_timestamp_timeout=400
>> under net traffic and tx_timestamp_timeout=25 in idle).
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>
>
>> ---
>> drivers/net/ethernet/ti/cpts.c | 86 ++++++++++++++++++++++++++++++++++++++++--
>> drivers/net/ethernet/ti/cpts.h | 1 +
>> 2 files changed, 84 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
>> index 3ed438e..c2121d2 100644
>> --- a/drivers/net/ethernet/ti/cpts.c
>> +++ b/drivers/net/ethernet/ti/cpts.c
>> @@ -31,9 +31,18 @@
>>
>> #include "cpts.h"
>>
>> +#define CPTS_SKB_TX_WORK_TIMEOUT 1 /* jiffies */
>> +
>> +struct cpts_skb_cb_data {
>> + unsigned long tmo;
>> +};
>> +
>> #define cpts_read32(c, r) readl_relaxed(&c->reg->r)
>> #define cpts_write32(c, v, r) writel_relaxed(v, &c->reg->r)
>>
>> +static int cpts_match(struct sk_buff *skb, unsigned int ptp_class,
>> + u16 ts_seqid, u8 ts_msgtype);
>> +
>> static int event_expired(struct cpts_event *event)
>> {
>> return time_after(jiffies, event->tmo);
>> @@ -77,6 +86,47 @@ static int cpts_purge_events(struct cpts *cpts)
>> return removed ? 0 : -1;
>> }
>>
>> +static bool cpts_match_tx_ts(struct cpts *cpts, struct cpts_event *event)
>> +{
>> + struct sk_buff *skb, *tmp;
>> + u16 seqid;
>> + u8 mtype;
>> + bool found = false;
>> +
>> + mtype = (event->high >> MESSAGE_TYPE_SHIFT) & MESSAGE_TYPE_MASK;
>> + seqid = (event->high >> SEQUENCE_ID_SHIFT) & SEQUENCE_ID_MASK;
>> +
>> + /* no need to grab txq.lock as access is always done under cpts->lock */
>> + skb_queue_walk_safe(&cpts->txq, skb, tmp) {
>> + struct skb_shared_hwtstamps ssh;
>> + unsigned int class = ptp_classify_raw(skb);
>> + struct cpts_skb_cb_data *skb_cb =
>> + (struct cpts_skb_cb_data *)skb->cb;
>> +
>> + if (cpts_match(skb, class, seqid, mtype)) {
>> + u64 ns = timecounter_cyc2time(&cpts->tc, event->low);
>> +
>> + memset(&ssh, 0, sizeof(ssh));
>> + ssh.hwtstamp = ns_to_ktime(ns);
>> + skb_tstamp_tx(skb, &ssh);
>> + found = true;
>> + __skb_unlink(skb, &cpts->txq);
>> + dev_consume_skb_any(skb);
>> + dev_dbg(cpts->dev, "match tx timestamp mtype %u seqid %04x\n",
>> + mtype, seqid);
>> + } else if (time_after(jiffies, skb_cb->tmo)) {
>> + /* timeout any expired skbs over 1s */
>> + dev_dbg(cpts->dev,
>> + "expiring tx timestamp mtype %u seqid %04x\n",
>> + mtype, seqid);
>> + __skb_unlink(skb, &cpts->txq);
>> + dev_consume_skb_any(skb);
>> + }
>> + }
>> +
>> + return found;
>> +}
>> +
>> /*
>> * Returns zero if matching event type was found.
>> */
>> @@ -101,9 +151,15 @@ static int cpts_fifo_read(struct cpts *cpts, int match)
>> event->low = lo;
>> type = event_type(event);
>> switch (type) {
>> + case CPTS_EV_TX:
>> + if (cpts_match_tx_ts(cpts, event)) {
>> + /* if the new event matches an existing skb,
>> + * then don't queue it
>> + */
>> + break;
>> + }
>> case CPTS_EV_PUSH:
>> case CPTS_EV_RX:
>> - case CPTS_EV_TX:
>> list_del_init(&event->list);
>> list_add_tail(&event->list, &cpts->events);
>> break;
>> @@ -229,8 +285,15 @@ static long cpts_overflow_check(struct ptp_clock_info *ptp)
>> struct cpts *cpts = container_of(ptp, struct cpts, info);
>> unsigned long delay = cpts->ov_check_period;
>> struct timespec64 ts;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&cpts->lock, flags);
>> + ts = ns_to_timespec64(timecounter_read(&cpts->tc));
>> +
>> + if (!skb_queue_empty(&cpts->txq))
>> + delay = CPTS_SKB_TX_WORK_TIMEOUT;
>> + spin_unlock_irqrestore(&cpts->lock, flags);
>>
>> - cpts_ptp_gettime(&cpts->info, &ts);
>> pr_debug("cpts overflow check at %lld.%09lu\n", ts.tv_sec, ts.tv_nsec);
>> return (long)delay;
>> }
>> @@ -301,7 +364,7 @@ static u64 cpts_find_ts(struct cpts *cpts, struct sk_buff *skb, int ev_type)
>> return 0;
>>
>> spin_lock_irqsave(&cpts->lock, flags);
>> - cpts_fifo_read(cpts, CPTS_EV_PUSH);
>> + cpts_fifo_read(cpts, -1);
> Seems it should be explained or send as separate patch before this one.
> Not sure, but smth like "Don't stop read fifo if PUSH event is found",
> but how it can happen that push event is present w/o TS_PUSH request..,
> anyway, if there is some valuable reason for that - should be explained.
ok. I'll drop it from this series.
>
>> list_for_each_safe(this, next, &cpts->events) {
>> event = list_entry(this, struct cpts_event, list);
>> if (event_expired(event)) {
>> @@ -319,6 +382,19 @@ static u64 cpts_find_ts(struct cpts *cpts, struct sk_buff *skb, int ev_type)
>> break;
>> }
>> }
>> +
>> + if (ev_type == CPTS_EV_TX && !ns) {
>> + struct cpts_skb_cb_data *skb_cb =
>> + (struct cpts_skb_cb_data *)skb->cb;
>> + /* Not found, add frame to queue for processing later.
>> + * The periodic FIFO check will handle this.
>> + */
>> + skb_get(skb);
>> + /* get the timestamp for timeouts */
>> + skb_cb->tmo = jiffies + msecs_to_jiffies(100);
>> + __skb_queue_tail(&cpts->txq, skb);
>> + ptp_schedule_worker(cpts->clock, 0);
>> + }
> As I see no need in separate irq handler for cpts events after this.
> Is that caused by h/w limitations on keystone2?
It's still required for proper support of HW_TS_PUSH events.
This fixes current issue which blocks CPTS usage.
>
> And what about rx path introduced in RFC, is it not needed any more or are
> you going to send additional series?
And I'm still trying to work on enabling CPTS IRQs and if it will be
enabled - the RX path will also need to be modified.
--
regards,
-grygorii
^ permalink raw reply
* Re: [v4.12 regression] netns: NULL deref in fib_sync_down_dev()
From: Ido Schimmel @ 2017-07-28 17:36 UTC (permalink / raw)
To: Cong Wang; +Cc: Michał Mirosław, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpXmUfdUzsu7Xpc2EjkePQCEjcP+irMkpc4bTDaWfAOVLw@mail.gmail.com>
On Fri, Jul 28, 2017 at 10:28:16AM -0700, Cong Wang wrote:
> On Fri, Jul 28, 2017 at 9:43 AM, Ido Schimmel <idosch@idosch.org> wrote:
> > On Fri, Jul 28, 2017 at 06:00:47PM +0200, Michał Mirosław wrote:
> >> Dear NetDevs,
> >>
> >> Before I go to bisecting, have you seen a following NULL dereference,
> >> yet? Where should I start looking? It is triggered by deleting netns
> >> (cut-down script attached - triggers every time). This was working
> >> correctly under v4.11.x.
> >
> > Thanks for the report. I just reproduced this on my system. I believe
> > the problem is a missing NULL check for 'in_dev' in
> > call_fib_nh_notifiers(). I'll test a fix.
>
> But your commit 982acb97560c8118c2109504a22b0d78a580547d
> is merged in v4.11-rc1. How could 4.11.x work correctly?
It doesn't. I just reproduced this on v4.11.
^ permalink raw reply
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: David Ahern @ 2017-07-28 17:39 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: Cong Wang, Hangbin Liu, network dev
In-Reply-To: <CAJieiUgxQ+okYAffbb6dqA-=U9qn1QA7A3-ZJ8=HC=o1bkF3ow@mail.gmail.com>
On 7/28/17 11:13 AM, Roopa Prabhu wrote:
> for fibmatch, my original intent was to return with an error code.
> This is similar
> to the ipv4 behavior. One option is to keep the check in there and put
> the 'fibmatch'
> condition around it. But, i do want to make sure that for the fibmatch case,
> it does not return an error directly on an existing prohibit route
> entry in the fib.
> This is probably doable by checking for appropriate
> net->ipv6.ip6_prohibit_entry entries.
>
IPv4 does not have the notion of null_entry or prohibit route entries
which makes IPv4 and IPv6 inconsistent - something we really need to be
avoiding from a user experience.
We have the following cases:
# ip -4 rule add to 172.16.60.0/24 prohibit
# ip -4 route add prohibit 172.16.50.0/24
# ip -6 rule add to 6000::/120 prohibit
# ip -6 route add prohibit 5000::/120
Behavior before Roopa's patch set:
Rule match:
# ip ro get 172.16.60.1
RTNETLINK answers: Permission denied
# ip -6 ro get 6000::1
prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
4294967295 error -13 pref medium
Route match:
# ip ro get 172.16.50.1
RTNETLINK answers: Permission denied
# ip -6 ro get 5000::1
prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric
1024 error -13 pref medium
Behavior after Roopa's patch set:
Rule match:
# ip ro get 172.16.60.1
RTNETLINK answers: Permission denied
# ip -6 ro get 6000::1
RTNETLINK answers: Permission denied
Route match:
# ip ro get 172.16.50.1
RTNETLINK answers: Permission denied
# ip -6 ro get 5000::1
RTNETLINK answers: Permission denied
So Roopa's fibmatch patches brings consistency between IPv4 and IPv6 at
the cost of breaking backwards compatibility for IPv6 when the prohibit
or blackhole routes are hit.
If that is not acceptable, then let's wrap the change in 'if (fibmatch)'
so that when fibmatch is requested we have consistency between IPv4 and
IPv6 when it is set.
^ permalink raw reply
* Re: refcount_t + (resend to wider audience)
From: Andrew Lunn @ 2017-07-28 17:41 UTC (permalink / raw)
To: Mark Salyzyn; +Cc: davem, netdev, stable
In-Reply-To: <adaee9d9-1d35-921c-36b3-e32e6c8ce9e3@android.com>
On Fri, Jul 28, 2017 at 10:15:23AM -0700, Mark Salyzyn wrote:
> (Resend to wider audience to comply with
> Documentation/networking/netdev-FAQ.txt)
>
> Please apply/backport the following upstream feature and followup
> grouped fixes patches to the stable trees (expect included in at least
> 3.10.y, 3.18.y, 4.4.y and 4.9.y):
.. _stable_kernel_rules:
Hi Mark
Everything you ever wanted to know about Linux -stable releases
===============================================================
Rules on what kind of patches are accepted, and which ones are not, into the
"-stable" tree:
- It must be obviously correct and tested.
- It cannot be bigger than 100 lines, with context.
The first patch you list is 342 lines. The second one is 634.
Please could you read the rules and then provide some justification
for ignoring many of the rules.
Andrew
^ permalink raw reply
* Re: Long stalls creating a new netns after a netns with a SMB client exits
From: Cong Wang @ 2017-07-28 17:49 UTC (permalink / raw)
To: Rolf Neugebauer; +Cc: Linux Kernel Network Developers
In-Reply-To: <CA+pO-2cm7bz861KvPMpugP8rVwf6rU8QYrVq7PVO-ZXx2kh1fw@mail.gmail.com>
Hello,
On Fri, Jul 28, 2017 at 9:47 AM, Rolf Neugebauer
<rolf.neugebauer@docker.com> wrote:
> Creating the new namespace is stalling for around 200 seconds and
> there 20 odd messages on the console, like:
>
> [ 67.372603] unregister_netdevice: waiting for lo to become free.
> Usage count = 1
>
Sounds like another netdev refcnt leak.
> Adding a 'sleep 1' before deleting the original network namespace
> "solves" the issue, but that doesn't sound like a good fix. Not using
> unmount also does not help (understandable).
Interesting, if sleeping for 1sec help, why did you see the stall for
200sec? The "leak" should go away eventually without 'sleep 1',
right?
>
> While the creation of the new namespace is stalled, I used 'sysrq' a
> few times to dump the work queues. There is an example below. Also,
> the hung task detection kicks in after 120 seconds (also below)
Yeah, the net_mutex is held by cleanup_net().
>
> I can readily reproduce this on 4.9.39, 4.11.12 and another user
> repro-ed it on 4.12.3. It seems to happen every time. At least one
> user reported issues with NFS mounts as well, but we were not able to
> reproduce it. It's not clear to me if this is directly related to
> 'mount.cifs' or if that just happens to reliably repro it.
OK, so commit d747a7a51b00984127a88113c does not help this case
either.
>
> It would be great if someone more familiar with the code could take a
> look. I'm happy to provide additional info (perf traces etc) or test
> patches if needed.
>
The last time I debugged this kind of netdev refcnt leak problem,
I added a few trace_printk() to dev_hold() and dev_put(),
so you can try it too. I will see if I can use your reproducer
here.
Thanks.
^ permalink raw reply
* Re: [PATCH 0/3] net-next: stmmac: support future possible different internal phy mode
From: Florian Fainelli @ 2017-07-28 17:54 UTC (permalink / raw)
To: Corentin Labbe, Andrew Lunn
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, peppe.cavallaro-qxv4g6HH51o,
alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
icenowy-h8G6r0blFSE
In-Reply-To: <20170728144424.GB7221@Red>
On 07/28/2017 07:44 AM, Corentin Labbe wrote:
> On Fri, Jul 28, 2017 at 04:36:00PM +0200, Andrew Lunn wrote:
>>>> I've probably asked this before: Does the internal PHY use a different
>>>> PHY ID in registers 2 and 3?
>>>>
>>>
>>> yes
>>>
>>> reg2: 0x0044
>>> reg3: 0X1500
>
> Copy/paste error, its 1400
>
>>
>> So this is not about loading the correct PHY driver. You can already
>> do this based on the PHY IDs...
>>
>> This is about selecting which PHY to use. Internal or External?
>>
>> Andrew
>
> It is too late when we know the PHY ID.
> We need to set a syscon for choosing external/internal PHY.
> So we can rely only on DT.
Since the Device Tree needs to be correct to identify which PHY to use
(internal or external), if you use the standard compatible string for
the PHY that contains its OUI, e.g:
compatible = "ethernet-phy-id0044.1400", "ethernet-phy-ieee802.3-c22"
then you can have your Ethernet MAC identify whether this is an internal
PHY by having a list of compatible strings to match against.
Corentin, can you make sure you copy netdev, Andrew and myself on the
next submissions so we don't have to keep track of seemingly identical
threads (this one + the rockchip dwmac changes) and we can work towards
one solution?
Thanks
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [v4.12 regression] netns: NULL deref in fib_sync_down_dev()
From: Michał Mirosław @ 2017-07-28 18:04 UTC (permalink / raw)
To: Ido Schimmel; +Cc: Cong Wang, Linux Kernel Network Developers
In-Reply-To: <20170728173602.GA30482@splinter>
On Fri, Jul 28, 2017 at 08:36:02PM +0300, Ido Schimmel wrote:
> On Fri, Jul 28, 2017 at 10:28:16AM -0700, Cong Wang wrote:
> > On Fri, Jul 28, 2017 at 9:43 AM, Ido Schimmel <idosch@idosch.org> wrote:
> > > On Fri, Jul 28, 2017 at 06:00:47PM +0200, Michał Mirosław wrote:
> > >> Dear NetDevs,
> > >>
> > >> Before I go to bisecting, have you seen a following NULL dereference,
> > >> yet? Where should I start looking? It is triggered by deleting netns
> > >> (cut-down script attached - triggers every time). This was working
> > >> correctly under v4.11.x.
> > > Thanks for the report. I just reproduced this on my system. I believe
> > > the problem is a missing NULL check for 'in_dev' in
> > > call_fib_nh_notifiers(). I'll test a fix.
> > But your commit 982acb97560c8118c2109504a22b0d78a580547d
> > is merged in v4.11-rc1. How could 4.11.x work correctly?
> It doesn't. I just reproduced this on v4.11.
Thanks for looking into this. I was sure that I ran v4.11.7 last time,
but it turns out I worked on this earlier than that. I'll be glad to
test patches for this issue when you have it.
Best Regards,
Michał Mirosław
^ 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