* Re: [PATCH net] net/ncsi: Don't assume last available channel exists
From: David Miller @ 2017-09-20 23:05 UTC (permalink / raw)
To: sam; +Cc: netdev, linux-kernel
In-Reply-To: <20170920041251.14635-1-sam@mendozajonas.com>
From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Wed, 20 Sep 2017 14:12:51 +1000
> When handling new VLAN tags in NCSI we check the maximum allowed number
> of filters on the last active ("hot") channel. However if the 'add'
> callback is called before NCSI has configured a channel, this causes a
> NULL dereference.
>
> Check that we actually have a hot channel, and warn if it is missing.
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Well, a few things.
We should not allow this driver method to be invoked in the first
place if the device is not in a state where the operation is legal
yet.
Second of all, if !ndp is true, you should not return 0 to indicate
success.
But more fundamentally, we should block this method from being
callable unless the device is in the proper state and can complete the
operation.
Seriously, these checks should be completely unnecessary if those
issues are handled properly.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: add new T5 pci device id's
From: David Miller @ 2017-09-20 23:06 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh
In-Reply-To: <1505887327-8687-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Wed, 20 Sep 2017 11:32:07 +0530
> Add 0x50a5, 0x50a6, 0x50a7, 0x50a8 and 0x50a9 T5 device
> id's.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v5 1/4] bpf: add helper bpf_perf_event_read_value for perf event array map
From: David Miller @ 2017-09-20 23:07 UTC (permalink / raw)
To: peterz; +Cc: yhs, rostedt, ast, daniel, netdev, kernel-team
In-Reply-To: <20170920172651.zx4nfmj2lpgxzubq@hirez.programming.kicks-ass.net>
From: Peter Zijlstra <peterz@infradead.org>
Date: Wed, 20 Sep 2017 19:26:51 +0200
> Dave, could we have this in a topic tree of sorts, because I have a
> pending series to rework all the timekeeping and it might be nice to not
> have sfr run into all sorts of conflicts.
If you want to merge it into your tree that's fine.
But it means that any further development done on top of this
work will need to go via you as well.
^ permalink raw reply
* Re: [PATCH 0/2] Netfilter fixes for net
From: David Miller @ 2017-09-20 23:08 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1505904543-10004-1-git-send-email-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed, 20 Sep 2017 12:49:01 +0200
> The following patchset contains two Netfilter fixes for your net tree,
> they are:
>
> 1) Fix NAt compilation with UP, from Geert Uytterhoeven.
>
> 2) Fix incorrect number of entries when dumping a set, from
> Vishwanath Pai.
Pulled, thanks Pablo.
^ permalink raw reply
* Re: [PATCH net 0/9] TM related bugfixes for the HNS3 Ethernet Driver
From: David Miller @ 2017-09-20 23:15 UTC (permalink / raw)
To: linyunsheng
Cc: huangdaode, xuwei5, liguozhu, Yisen.Zhuang, gabriele.paoloni,
john.garry, linuxarm, salil.mehta, lipeng321, netdev,
linux-kernel
In-Reply-To: <1505904778-53217-1-git-send-email-linyunsheng@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Wed, 20 Sep 2017 18:52:49 +0800
> This patch set contains a few bugfixes related to hclge_tm module.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
From: Stephen Hemminger @ 2017-09-20 23:21 UTC (permalink / raw)
To: David Ahern; +Cc: David Miller, vincent, bridge, netdev
In-Reply-To: <16e5566a-909d-ba83-7637-1fb6c93126bc@gmail.com>
On Wed, 20 Sep 2017 15:57:16 -0600
David Ahern <dsahern@gmail.com> wrote:
> On 9/20/17 3:09 PM, David Miller wrote:
> > From: Vincent Bernat <vincent@bernat.im>
> > Date: Sat, 16 Sep 2017 16:18:33 +0200
> >
> > David, I am CC:'ing you because you've done work in this area over the
> > past year. I'm applying this patch, it's been sitting since the 16th
> > and likes entirely correct to me. But if you have objections just let
> > me know.
> >
> >> Currently, when an interface is released from a bridge via
> >> ioctl(), we get a RTM_DELLINK event through netlink:
> >>
> >> Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> >> link/ether 6e:23:c2:54:3a:b3
> >>
> >> Userspace has to interpret that as a removal from the bridge, not as a
> >> complete removal of the interface. When an bridged interface is
> >> completely removed, we get two events:
> >>
> >> Deleted 2: dummy0: <BROADCAST,NOARP> mtu 1500 master bridge0 state DOWN
> >> link/ether 6e:23:c2:54:3a:b3
> >> Deleted 2: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default
> >> link/ether 6e:23:c2:54:3a:b3 brd ff:ff:ff:ff:ff:ff
> >>
> >> In constrast, when an interface is released from a bond, we get a
> >> RTM_NEWLINK with only the new characteristics (no master):
> >>
> >> 3: dummy1: <BROADCAST,NOARP,SLAVE,UP,LOWER_UP> mtu 1500 qdisc noqueue master bond0 state UNKNOWN group default
> >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
> >> 3: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
> >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
> >> 4: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default
> >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
> >> 3: dummy1: <BROADCAST,NOARP> mtu 1500 qdisc noqueue state DOWN group default
> >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
> >> 3: dummy1: <BROADCAST,NOARP> mtu 1500 qdisc noqueue state DOWN group default
> >> link/ether ca:c8:7b:66:f8:25 brd ff:ff:ff:ff:ff:ff
> >> 4: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default
> >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
> >>
> >> Userland may be confused by the fact we say a link is deleted while
> >> its characteristics are only modified. A first solution would have
> >> been to turn the RTM_DELLINK event in del_nbp() into a RTM_NEWLINK
> >> event. However, maybe some piece of userland is relying on this
> >> RTM_DELLINK to detect when a bridged interface is released. Instead,
> >> we also emit a RTM_NEWLINK event once the interface is
> >> released (without master info).
> >>
> >> Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> >> link/ether 8a:bb:e7:94:b1:f8
> >> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
> >> link/ether 8a:bb:e7:94:b1:f8 brd ff:ff:ff:ff:ff:ff
> >>
> >> This is done only when using ioctl(). When using Netlink, such an
> >> event is already automatically emitted in do_setlink().
>
> The DELLINK is for AF_BRIDGE family (ifi_family). Adding family to
> print_linkinfo and running the monitor I get:
>
>
> [LINK]family 0: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc
> noqueue master br0 state UNKNOWN group default
> link/ether d6:c3:73:86:3c:73 brd ff:ff:ff:ff:ff:ff
>
> [LINK]family 7: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500
> master br0 state UNKNOWN
> link/ether d6:c3:73:86:3c:73
>
> [LINK]Deleted family 7: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu
> 1500 master br0 state UNKNOWN
> link/ether d6:c3:73:86:3c:73
>
> [LINK]family 0: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc
> noqueue state UNKNOWN group default
> link/ether d6:c3:73:86:3c:73 brd ff:ff:ff:ff:ff:ff
>
> And that seems correct. So I think the RTM_NEWLINK added by this patch
> should not be needed.
Agreed, thanks for tracing this.
The one concern is that ports added or removed through ioctl should
cause same events as doing the same thing via netlink. Some users use
brctl (ioctl) and others use newer bridge (netlink) API.
^ permalink raw reply
* Re: [PATCH v3 14/31] vxfs: Define usercopy region in vxfs_inode slab cache
From: Christoph Hellwig @ 2017-09-20 23:22 UTC (permalink / raw)
To: Kees Cook
Cc: Christoph Hellwig, LKML, David Windsor,
linux-fsdevel@vger.kernel.org, Network Development, Linux-MM,
kernel-hardening@lists.openwall.com
In-Reply-To: <CAGXu5j+hr0UwB5NsvPSKVVfM6NFHHhnNeUZbuwyTRppSOx9Ucw@mail.gmail.com>
On Wed, Sep 20, 2017 at 02:21:45PM -0700, Kees Cook wrote:
> This is why I included several other lists on the full CC (am I
> unlucky enough to have you not subscribed to any of them?). Adding a
> CC for everyone can result in a huge CC list, especially for the
> forth-coming 300-patch timer_list series. ;)
If you think the lists are enough to review changes include only
the lists, but don't add CCs for individual patches, that's what
I usually do for cleanups that touch a lot of drivers, but don't
really change actual logic in ever little driver touched.
> Do you want me to resend the full series to you, or would you prefer
> something else like a patchwork bundle? (I'll explicitly add you to CC
> for any future versions, though.)
I'm fine with not being Cced at all if there isn't anything requiring
my urgent personal attention. It's up to you whom you want to Cc,
but my preference is generally for rather less than more people, and
rather more than less mailing lists.
But the important bit is to Cc a person or mailinglist either on
all patches or on none, otherwise a good review isn't possible.
^ permalink raw reply
* Re: [PATCH] net_sched: always reset qdisc backlog in qdisc_reset()
From: Cong Wang @ 2017-09-20 23:26 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Linux Kernel Network Developers, David S. Miller, Jiri Pirko,
Jamal Hadi Salim
In-Reply-To: <150591153693.113604.8604505743746410801.stgit@buzz>
On Wed, Sep 20, 2017 at 5:45 AM, Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
> SKB stored in qdisc->gso_skb also counted into backlog.
>
> Some qdiscs don't reset backlog to zero in ->reset(),
> for example sfq just dequeue and free all queued skb.
>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> Fixes: 2ccccf5fb43f ("net_sched: update hierarchical backlog too")
Looks good.
sch->qstats.backlog = 0 can be removed from each ->reset()
after this patch.
^ permalink raw reply
* [PATCH v2 13/31] timer: Remove meaningless .data/.function assignments
From: Kees Cook @ 2017-09-20 23:27 UTC (permalink / raw)
To: Thomas Gleixner
Cc: devel, Kees Cook, Greg Kroah-Hartman, linux-wireless,
linux-kernel, Jens Axboe, Ganesh Krishna, netdev, Aditya Shankar,
Krzysztof Halasa
In-Reply-To: <1505950075-50223-1-git-send-email-keescook@chromium.org>
Several timer users needlessly reset their .function/.data fields during
their timer callback, but nothing else changes them. Some users do not
use their .data field at all. Each instance is removed here.
Cc: Krzysztof Halasa <khc@pm.waw.pl>
Cc: Aditya Shankar <aditya.shankar@microchip.com>
Cc: Ganesh Krishna <ganesh.krishna@microchip.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jens Axboe <axboe@fb.com>
Cc: netdev@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: devel@driverdev.osuosl.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> # for staging
Acked-by: Krzysztof Halasa <khc@pm.waw.pl> # for wan/hdlc*
Acked-by: Jens Axboe <axboe@kernel.dk> # for amiflop
---
drivers/block/amiflop.c | 3 +--
drivers/net/wan/hdlc_cisco.c | 2 --
drivers/net/wan/hdlc_fr.c | 2 --
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 +---
4 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index c4b1cba27178..6680d75bc857 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -323,7 +323,7 @@ static void fd_deselect (int drive)
}
-static void motor_on_callback(unsigned long nr)
+static void motor_on_callback(unsigned long ignored)
{
if (!(ciaa.pra & DSKRDY) || --on_attempts == 0) {
complete_all(&motor_on_completion);
@@ -344,7 +344,6 @@ static int fd_motor_on(int nr)
fd_select(nr);
reinit_completion(&motor_on_completion);
- motor_on_timer.data = nr;
mod_timer(&motor_on_timer, jiffies + HZ/2);
on_attempts = 10;
diff --git a/drivers/net/wan/hdlc_cisco.c b/drivers/net/wan/hdlc_cisco.c
index c696d42f4502..6c98d85f2773 100644
--- a/drivers/net/wan/hdlc_cisco.c
+++ b/drivers/net/wan/hdlc_cisco.c
@@ -276,8 +276,6 @@ static void cisco_timer(unsigned long arg)
spin_unlock(&st->lock);
st->timer.expires = jiffies + st->settings.interval * HZ;
- st->timer.function = cisco_timer;
- st->timer.data = arg;
add_timer(&st->timer);
}
diff --git a/drivers/net/wan/hdlc_fr.c b/drivers/net/wan/hdlc_fr.c
index de42faca076a..7da2424c28a4 100644
--- a/drivers/net/wan/hdlc_fr.c
+++ b/drivers/net/wan/hdlc_fr.c
@@ -644,8 +644,6 @@ static void fr_timer(unsigned long arg)
state(hdlc)->settings.t391 * HZ;
}
- state(hdlc)->timer.function = fr_timer;
- state(hdlc)->timer.data = arg;
add_timer(&state(hdlc)->timer);
}
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index ac5aaafa461c..60f088babf27 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -266,7 +266,7 @@ static void update_scan_time(void)
last_scanned_shadow[i].time_scan = jiffies;
}
-static void remove_network_from_shadow(unsigned long arg)
+static void remove_network_from_shadow(unsigned long unused)
{
unsigned long now = jiffies;
int i, j;
@@ -287,7 +287,6 @@ static void remove_network_from_shadow(unsigned long arg)
}
if (last_scanned_cnt != 0) {
- hAgingTimer.data = arg;
mod_timer(&hAgingTimer, jiffies + msecs_to_jiffies(AGING_TIME));
}
}
@@ -304,7 +303,6 @@ static int is_network_in_shadow(struct network_info *pstrNetworkInfo,
int i;
if (last_scanned_cnt == 0) {
- hAgingTimer.data = (unsigned long)user_void;
mod_timer(&hAgingTimer, jiffies + msecs_to_jiffies(AGING_TIME));
state = -1;
} else {
--
2.7.4
^ permalink raw reply related
* [PATCH v2 20/31] net/core: Collapse redundant sk_timer callback data assignments
From: Kees Cook @ 2017-09-20 23:27 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, David S. Miller, Ralf Baechle, Andrew Hendry,
Eric Dumazet, Paolo Abeni, David Howells, Colin Ian King,
Ingo Molnar, linzhang, netdev, linux-hams, linux-x25,
linux-kernel
In-Reply-To: <1505950075-50223-1-git-send-email-keescook@chromium.org>
The core sk_timer initializer can provide the common .data assignment
instead of it being set separately in users.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Colin Ian King <colin.king@canonical.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: linzhang <xiaolou4617@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-hams@vger.kernel.org
Cc: linux-x25@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
net/core/sock.c | 2 +-
net/netrom/nr_timer.c | 1 -
net/rose/rose_timer.c | 1 -
net/x25/af_x25.c | 1 -
net/x25/x25_timer.c | 1 -
5 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index 9b7b6bbb2a23..3a0233106f62 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2678,7 +2678,7 @@ void sock_init_data(struct socket *sock, struct sock *sk)
sk_init_common(sk);
sk->sk_send_head = NULL;
- init_timer(&sk->sk_timer);
+ setup_timer(&sk->sk_timer, NULL, (unsigned long)sk);
sk->sk_allocation = GFP_KERNEL;
sk->sk_rcvbuf = sysctl_rmem_default;
diff --git a/net/netrom/nr_timer.c b/net/netrom/nr_timer.c
index 94d05806a9a2..f84ce71f1f5f 100644
--- a/net/netrom/nr_timer.c
+++ b/net/netrom/nr_timer.c
@@ -45,7 +45,6 @@ void nr_init_timers(struct sock *sk)
setup_timer(&nr->idletimer, nr_idletimer_expiry, (unsigned long)sk);
/* initialized by sock_init_data */
- sk->sk_timer.data = (unsigned long)sk;
sk->sk_timer.function = &nr_heartbeat_expiry;
}
diff --git a/net/rose/rose_timer.c b/net/rose/rose_timer.c
index bc5469d6d9cb..6baa415b199a 100644
--- a/net/rose/rose_timer.c
+++ b/net/rose/rose_timer.c
@@ -36,7 +36,6 @@ void rose_start_heartbeat(struct sock *sk)
{
del_timer(&sk->sk_timer);
- sk->sk_timer.data = (unsigned long)sk;
sk->sk_timer.function = &rose_heartbeat_expiry;
sk->sk_timer.expires = jiffies + 5 * HZ;
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index ac095936552d..c590c0bd1393 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -414,7 +414,6 @@ static void __x25_destroy_socket(struct sock *sk)
/* Defer: outstanding buffers */
sk->sk_timer.expires = jiffies + 10 * HZ;
sk->sk_timer.function = x25_destroy_timer;
- sk->sk_timer.data = (unsigned long)sk;
add_timer(&sk->sk_timer);
} else {
/* drop last reference so sock_put will free */
diff --git a/net/x25/x25_timer.c b/net/x25/x25_timer.c
index 5c5db1a36399..de5cec41d100 100644
--- a/net/x25/x25_timer.c
+++ b/net/x25/x25_timer.c
@@ -36,7 +36,6 @@ void x25_init_timers(struct sock *sk)
setup_timer(&x25->timer, x25_timer_expiry, (unsigned long)sk);
/* initialized by sock_init_data */
- sk->sk_timer.data = (unsigned long)sk;
sk->sk_timer.function = &x25_heartbeat_expiry;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v2 25/31] net/atm/mpc: Use separate static data field with with static timer
From: Kees Cook @ 2017-09-20 23:27 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, David S. Miller, Andrew Morton, Alexey Dobriyan,
Reshetova, Elena, netdev, linux-kernel
In-Reply-To: <1505950075-50223-1-git-send-email-keescook@chromium.org>
In preparation for changing the timer callback argument to the timer
pointer, move to a separate static data variable.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: "Reshetova, Elena" <elena.reshetova@intel.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
net/atm/mpc.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index 63138c8c2269..3b59a053b7cb 100644
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -95,7 +95,7 @@ static netdev_tx_t mpc_send_packet(struct sk_buff *skb,
static int mpoa_event_listener(struct notifier_block *mpoa_notifier,
unsigned long event, void *dev);
static void mpc_timer_refresh(void);
-static void mpc_cache_check(unsigned long checking_time);
+static void mpc_cache_check(unsigned long unused);
static struct llc_snap_hdr llc_snap_mpoa_ctrl = {
0xaa, 0xaa, 0x03,
@@ -121,7 +121,8 @@ static struct notifier_block mpoa_notifier = {
struct mpoa_client *mpcs = NULL; /* FIXME */
static struct atm_mpoa_qos *qos_head = NULL;
-static DEFINE_TIMER(mpc_timer, NULL);
+static DEFINE_TIMER(mpc_timer, mpc_cache_check);
+static unsigned long checking_time;
static struct mpoa_client *find_mpc_by_itfnum(int itf)
@@ -1411,12 +1412,11 @@ static void clean_up(struct k_message *msg, struct mpoa_client *mpc, int action)
static void mpc_timer_refresh(void)
{
mpc_timer.expires = jiffies + (MPC_P2 * HZ);
- mpc_timer.data = mpc_timer.expires;
- mpc_timer.function = mpc_cache_check;
+ checking_time = mpc_timer.expires;
add_timer(&mpc_timer);
}
-static void mpc_cache_check(unsigned long checking_time)
+static void mpc_cache_check(unsigned long unused)
{
struct mpoa_client *mpc = mpcs;
static unsigned long previous_resolving_check_time;
--
2.7.4
^ permalink raw reply related
* [PATCH v2 30/31] appletalk: Remove unneeded synchronization
From: Kees Cook @ 2017-09-20 23:27 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: Kees Cook, David Howells, netdev, linux-kernel
In-Reply-To: <1505950075-50223-1-git-send-email-keescook@chromium.org>
The use of del_timer_sync() will make sure a timer is not rescheduled.
As such, there is no need to add external signals to kill timers. In
preparation for switching the timer callback argument to the timer
pointer, this drops the .data argument since it doesn't serve a meaningful
purpose here.
Cc: David Howells <dhowells@redhat.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/appletalk/ltpc.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/net/appletalk/ltpc.c b/drivers/net/appletalk/ltpc.c
index e4aa374caa4d..cc3dc9337eae 100644
--- a/drivers/net/appletalk/ltpc.c
+++ b/drivers/net/appletalk/ltpc.c
@@ -880,14 +880,10 @@ static void ltpc_poll(unsigned long l)
}
ltpc_poll_counter--;
}
-
- if (!dev)
- return; /* we've been downed */
/* poll 20 times per second */
idle(dev);
ltpc_timer.expires = jiffies + HZ/20;
-
add_timer(<pc_timer);
}
@@ -1252,8 +1248,6 @@ static void __exit ltpc_cleanup(void)
if(debug & DEBUG_VERBOSE) printk("unregister_netdev\n");
unregister_netdev(dev_ltpc);
- ltpc_timer.data = 0; /* signal the poll routine that we're done */
-
del_timer_sync(<pc_timer);
if(debug & DEBUG_VERBOSE) printk("freeing irq\n");
--
2.7.4
^ permalink raw reply related
* [PATCH v2 31/31] timer: Switch to testing for .function instead of .data
From: Kees Cook @ 2017-09-20 23:27 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, Rafael J. Wysocki, Pavel Machek, Len Brown,
Greg Kroah-Hartman, Mike Marciniszyn, Dennis Dalessandro,
Doug Ledford, Sean Hefty, Hal Rosenstock, Dmitry Torokhov,
Jeff Kirsher, linux-pm, linux-rdma, linux-input, intel-wired-lan,
netdev, linux-kernel
In-Reply-To: <1505950075-50223-1-git-send-email-keescook@chromium.org>
This is part of the work to support switching all struct timer_list
callbacks to get the timer pointer as the argument (like other modern
callback interfaces in the kernel) instead of from the .data field.
This patch is one of several steps in removing open-coded users of the
.data field:
In several places, .data is checked for initialization to gate early
calls to del_timer_sync(). Checking for .function is equally valid, so
switch to this in all callers.
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Mike Marciniszyn <mike.marciniszyn@intel.com>
Cc: Dennis Dalessandro <dennis.dalessandro@intel.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> # for input part
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> # for i40e part
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> # for power
---
drivers/base/power/wakeup.c | 3 +--
drivers/infiniband/hw/hfi1/chip.c | 6 ++----
drivers/infiniband/hw/hfi1/init.c | 2 +-
drivers/infiniband/hw/qib/qib_iba7220.c | 2 +-
drivers/infiniband/hw/qib/qib_iba7322.c | 2 +-
drivers/infiniband/hw/qib/qib_init.c | 14 +++++---------
drivers/infiniband/hw/qib/qib_mad.c | 2 +-
drivers/input/input.c | 5 ++---
drivers/net/ethernet/intel/i40e/i40e_main.c | 2 +-
9 files changed, 15 insertions(+), 23 deletions(-)
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index cdd6f256da59..7f030d79a438 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -481,8 +481,7 @@ static bool wakeup_source_not_registered(struct wakeup_source *ws)
* Use timer struct to check if the given source is initialized
* by wakeup_source_add.
*/
- return ws->timer.function != pm_wakeup_timer_fn ||
- ws->timer.data != (unsigned long)ws;
+ return ws->timer.function != pm_wakeup_timer_fn;
}
/*
diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index b2ed4b9cda6e..efd93521d7e4 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -5565,9 +5565,8 @@ static int init_rcverr(struct hfi1_devdata *dd)
static void free_rcverr(struct hfi1_devdata *dd)
{
- if (dd->rcverr_timer.data)
+ if (dd->rcverr_timer.function)
del_timer_sync(&dd->rcverr_timer);
- dd->rcverr_timer.data = 0;
}
static void handle_rxe_err(struct hfi1_devdata *dd, u32 unused, u64 reg)
@@ -12067,9 +12066,8 @@ static void free_cntrs(struct hfi1_devdata *dd)
struct hfi1_pportdata *ppd;
int i;
- if (dd->synth_stats_timer.data)
+ if (dd->synth_stats_timer.function)
del_timer_sync(&dd->synth_stats_timer);
- dd->synth_stats_timer.data = 0;
ppd = (struct hfi1_pportdata *)(dd + 1);
for (i = 0; i < dd->num_pports; i++, ppd++) {
kfree(ppd->cntrs);
diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c
index fba77001c3a7..e7042e8b5e5a 100644
--- a/drivers/infiniband/hw/hfi1/init.c
+++ b/drivers/infiniband/hw/hfi1/init.c
@@ -1006,7 +1006,7 @@ static void stop_timers(struct hfi1_devdata *dd)
for (pidx = 0; pidx < dd->num_pports; ++pidx) {
ppd = dd->pport + pidx;
- if (ppd->led_override_timer.data) {
+ if (ppd->led_override_timer.function) {
del_timer_sync(&ppd->led_override_timer);
atomic_set(&ppd->led_override_timer_active, 0);
}
diff --git a/drivers/infiniband/hw/qib/qib_iba7220.c b/drivers/infiniband/hw/qib/qib_iba7220.c
index 51a9266eea78..6fd83b91a1db 100644
--- a/drivers/infiniband/hw/qib/qib_iba7220.c
+++ b/drivers/infiniband/hw/qib/qib_iba7220.c
@@ -1663,7 +1663,7 @@ static void qib_7220_quiet_serdes(struct qib_pportdata *ppd)
dd->control | QLOGIC_IB_C_FREEZEMODE);
ppd->cpspec->chase_end = 0;
- if (ppd->cpspec->chase_timer.data) /* if initted */
+ if (ppd->cpspec->chase_timer.function) /* if initted */
del_timer_sync(&ppd->cpspec->chase_timer);
if (ppd->cpspec->ibsymdelta || ppd->cpspec->iblnkerrdelta ||
diff --git a/drivers/infiniband/hw/qib/qib_iba7322.c b/drivers/infiniband/hw/qib/qib_iba7322.c
index 14cadf6d6214..1ac7afe32897 100644
--- a/drivers/infiniband/hw/qib/qib_iba7322.c
+++ b/drivers/infiniband/hw/qib/qib_iba7322.c
@@ -2531,7 +2531,7 @@ static void qib_7322_mini_quiet_serdes(struct qib_pportdata *ppd)
cancel_delayed_work_sync(&ppd->cpspec->ipg_work);
ppd->cpspec->chase_end = 0;
- if (ppd->cpspec->chase_timer.data) /* if initted */
+ if (ppd->cpspec->chase_timer.function) /* if initted */
del_timer_sync(&ppd->cpspec->chase_timer);
/*
diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
index c5a4c65636d6..1e72522d5691 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -815,23 +815,19 @@ static void qib_stop_timers(struct qib_devdata *dd)
struct qib_pportdata *ppd;
int pidx;
- if (dd->stats_timer.data) {
+ if (dd->stats_timer.function)
del_timer_sync(&dd->stats_timer);
- dd->stats_timer.data = 0;
- }
- if (dd->intrchk_timer.data) {
+ if (dd->intrchk_timer.function)
del_timer_sync(&dd->intrchk_timer);
- dd->intrchk_timer.data = 0;
- }
for (pidx = 0; pidx < dd->num_pports; ++pidx) {
ppd = dd->pport + pidx;
- if (ppd->hol_timer.data)
+ if (ppd->hol_timer.function)
del_timer_sync(&ppd->hol_timer);
- if (ppd->led_override_timer.data) {
+ if (ppd->led_override_timer.function) {
del_timer_sync(&ppd->led_override_timer);
atomic_set(&ppd->led_override_timer_active, 0);
}
- if (ppd->symerr_clear_timer.data)
+ if (ppd->symerr_clear_timer.function)
del_timer_sync(&ppd->symerr_clear_timer);
}
}
diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
index f3d451136248..b72185360d34 100644
--- a/drivers/infiniband/hw/qib/qib_mad.c
+++ b/drivers/infiniband/hw/qib/qib_mad.c
@@ -2491,7 +2491,7 @@ void qib_notify_free_mad_agent(struct rvt_dev_info *rdi, int port_idx)
struct qib_devdata *dd = container_of(ibdev,
struct qib_devdata, verbs_dev);
- if (dd->pport[port_idx].cong_stats.timer.data)
+ if (dd->pport[port_idx].cong_stats.timer.function)
del_timer_sync(&dd->pport[port_idx].cong_stats.timer);
if (dd->pport[port_idx].ibport_data.smi_ah)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index d268fdc23c64..15153886253b 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -76,7 +76,7 @@ static void input_start_autorepeat(struct input_dev *dev, int code)
{
if (test_bit(EV_REP, dev->evbit) &&
dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
- dev->timer.data) {
+ dev->timer.function) {
dev->repeat_key = code;
mod_timer(&dev->timer,
jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
@@ -1790,7 +1790,7 @@ struct input_dev *input_allocate_device(void)
device_initialize(&dev->dev);
mutex_init(&dev->mutex);
spin_lock_init(&dev->event_lock);
- init_timer(&dev->timer);
+ setup_timer(&dev->timer, NULL, (unsigned long)dev);
INIT_LIST_HEAD(&dev->h_list);
INIT_LIST_HEAD(&dev->node);
@@ -2053,7 +2053,6 @@ static void devm_input_device_unregister(struct device *dev, void *res)
*/
void input_enable_softrepeat(struct input_dev *dev, int delay, int period)
{
- dev->timer.data = (unsigned long) dev;
dev->timer.function = input_repeat_key;
dev->rep[REP_DELAY] = delay;
dev->rep[REP_PERIOD] = period;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 6498da8806cb..74d550704013 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11777,7 +11777,7 @@ static void i40e_remove(struct pci_dev *pdev)
/* no more scheduling of any task */
set_bit(__I40E_SUSPENDED, pf->state);
set_bit(__I40E_DOWN, pf->state);
- if (pf->service_timer.data)
+ if (pf->service_timer.function)
del_timer_sync(&pf->service_timer);
if (pf->service_task.func)
cancel_work_sync(&pf->service_task);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 00/10] net/smc: updates 2017-09-20
From: David Miller @ 2017-09-20 23:29 UTC (permalink / raw)
To: ubraun
Cc: netdev, linux-rdma, linux-s390, jwi, schwidefsky, heiko.carstens,
raspl
In-Reply-To: <20170920115813.63745-1-ubraun@linux.vnet.ibm.com>
From: Ursula Braun <ubraun@linux.vnet.ibm.com>
Date: Wed, 20 Sep 2017 13:58:03 +0200
> here is a collection of small smc-patches built for net-next
> improving the smc code in different areas.
There are bug fixes in here, which should be targetted at 'net'.
^ permalink raw reply
* Re: [PATCH v2 1/3] can: m_can: Make hclk optional
From: Franklin S Cooper Jr @ 2017-09-20 23:29 UTC (permalink / raw)
To: Mario Hüttel, linux-kernel, devicetree, netdev, linux-can,
wg, mkl, robh+dt, quentin.schulz
In-Reply-To: <096c73b1-eaa5-8c40-4486-e08a784c0897@gmx.net>
On 09/20/2017 05:00 PM, Mario Hüttel wrote:
>> Hclk is the MCAN's interface clock. However, for OMAP based devices such as
>> DRA7 SoC family the interface clock is handled by hwmod. Therefore, this
>> interface clock is managed by hwmod driver via pm_runtime_get and
>> pm_runtime_put calls. Therefore, this interface clock isn't defined in DT
>> and thus the driver shouldn't fail if this clock isn't found.
> I also agree in this point.
> However, there's a problem I want to point out:
>
> The M_CAN can only function correctly, if the condition
> 'hclk >= cclk' holds true.
>
> The internal clock domain crossing can fail if this condition
> is violated.
>
> I thought about adding the condition to the driver to ensure
> correct operation. But I had some problems:
>
> 1. Determine the clock rates:
> The devices you've mentioned above don't have an assigned
> hclk. Is there still a possibility to get the clock frequency?
I believe interface clocks via hwmod aren't exposed to drivers. So the
only way to get access to the clock frequency is to add this interface
clock to dt.
>
> 2. What to do if 'hclk < cclk'?
> Is there a general way to process such an error? - I think not.
> Is a simple warning in case of an error enough?
>
> Is there a way of ensuring that the condition is always met at all?
>
> I think it is quite unlikely that the condition is violated, because
> devices that actually run Linux usually have (bus) clock rates that
> are high enough to ensure the correct operation.
>
> Would it be safe to just ignore this possible fault?
I think alot of peripherals have similar constraints when there is an
interface and functional clock. However, I haven't seen drivers verify
that this kind of constraint is properly met. Personally I think its a
valid fault but one that can be ignored from the driver perspective.
>
> Regards
>
> Mario
>
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>> ---
>> Version 2 changes:
>> Used NULL instead of 0 for unused hclk handle
>>
>> drivers/net/can/m_can/m_can.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>> index f4947a7..ea48e59 100644
>> --- a/drivers/net/can/m_can/m_can.c
>> +++ b/drivers/net/can/m_can/m_can.c
>> @@ -1568,8 +1568,13 @@ static int m_can_plat_probe(struct platform_device *pdev)
>> hclk = devm_clk_get(&pdev->dev, "hclk");
>> cclk = devm_clk_get(&pdev->dev, "cclk");
>>
>> - if (IS_ERR(hclk) || IS_ERR(cclk)) {
>> - dev_err(&pdev->dev, "no clock found\n");
>> + if (IS_ERR(hclk)) {
>> + dev_warn(&pdev->dev, "hclk could not be found\n");
>> + hclk = NULL;
>> + }
>> +
>> + if (IS_ERR(cclk)) {
>> + dev_err(&pdev->dev, "cclk could not be found\n");
>> ret = -ENODEV;
>> goto failed_ret;
>> }
>
>
^ permalink raw reply
* [PATCH net-next] net: dsa: better scoping of slave functions
From: Vivien Didelot @ 2017-09-20 23:31 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
A few DSA slave functions take a dsa_slave_priv pointer as first
argument, whereas the scope of the slave.c functions is the slave
net_device structure. Fix this and rename dsa_netpoll_send_skb to
dsa_slave_netpoll_send_skb.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/slave.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index d51b10450e1b..bc376fc1bbe7 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -385,10 +385,12 @@ static int dsa_slave_port_attr_get(struct net_device *dev,
return 0;
}
-static inline netdev_tx_t dsa_netpoll_send_skb(struct dsa_slave_priv *p,
- struct sk_buff *skb)
+static inline netdev_tx_t dsa_slave_netpoll_send_skb(struct net_device *dev,
+ struct sk_buff *skb)
{
#ifdef CONFIG_NET_POLL_CONTROLLER
+ struct dsa_slave_priv *p = netdev_priv(dev);
+
if (p->netpoll)
netpoll_send_skb(p->netpoll, skb);
#else
@@ -422,7 +424,7 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)
* tag to be successfully transmitted
*/
if (unlikely(netpoll_tx_running(dev)))
- return dsa_netpoll_send_skb(p, nskb);
+ return dsa_slave_netpoll_send_skb(dev, nskb);
/* Queue the SKB for transmission on the parent interface, but
* do not modify its EtherType
@@ -736,9 +738,9 @@ static int dsa_slave_get_phys_port_name(struct net_device *dev,
}
static struct dsa_mall_tc_entry *
-dsa_slave_mall_tc_entry_find(struct dsa_slave_priv *p,
- unsigned long cookie)
+dsa_slave_mall_tc_entry_find(struct net_device *dev, unsigned long cookie)
{
+ struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_mall_tc_entry *mall_tc_entry;
list_for_each_entry(mall_tc_entry, &p->mall_tc_list, list)
@@ -820,7 +822,7 @@ static void dsa_slave_del_cls_matchall(struct net_device *dev,
if (!ds->ops->port_mirror_del)
return;
- mall_tc_entry = dsa_slave_mall_tc_entry_find(p, cls->cookie);
+ mall_tc_entry = dsa_slave_mall_tc_entry_find(dev, cls->cookie);
if (!mall_tc_entry)
return;
@@ -1027,10 +1029,9 @@ static int dsa_slave_fixed_link_update(struct net_device *dev,
}
/* slave device setup *******************************************************/
-static int dsa_slave_phy_connect(struct dsa_slave_priv *p,
- struct net_device *slave_dev,
- int addr)
+static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr)
{
+ struct dsa_slave_priv *p = netdev_priv(slave_dev);
struct dsa_switch *ds = p->dp->ds;
p->phy = mdiobus_get_phy(ds->slave_mii_bus, addr);
@@ -1046,9 +1047,9 @@ static int dsa_slave_phy_connect(struct dsa_slave_priv *p,
p->phy_interface);
}
-static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
- struct net_device *slave_dev)
+static int dsa_slave_phy_setup(struct net_device *slave_dev)
{
+ struct dsa_slave_priv *p = netdev_priv(slave_dev);
struct dsa_switch *ds = p->dp->ds;
struct device_node *phy_dn, *port_dn;
bool phy_is_fixed = false;
@@ -1088,7 +1089,7 @@ static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
*/
if (!phy_is_fixed && phy_id >= 0 &&
(ds->phys_mii_mask & (1 << phy_id))) {
- ret = dsa_slave_phy_connect(p, slave_dev, phy_id);
+ ret = dsa_slave_phy_connect(slave_dev, phy_id);
if (ret) {
netdev_err(slave_dev, "failed to connect to phy%d: %d\n", phy_id, ret);
of_node_put(phy_dn);
@@ -1111,7 +1112,7 @@ static int dsa_slave_phy_setup(struct dsa_slave_priv *p,
* MDIO bus instead
*/
if (!p->phy) {
- ret = dsa_slave_phy_connect(p, slave_dev, p->dp->index);
+ ret = dsa_slave_phy_connect(slave_dev, p->dp->index);
if (ret) {
netdev_err(slave_dev, "failed to connect to port %d: %d\n",
p->dp->index, ret);
@@ -1233,7 +1234,7 @@ int dsa_slave_create(struct dsa_port *port, const char *name)
netif_carrier_off(slave_dev);
- ret = dsa_slave_phy_setup(p, slave_dev);
+ ret = dsa_slave_phy_setup(slave_dev);
if (ret) {
netdev_err(master, "error %d setting up slave phy\n", ret);
unregister_netdev(slave_dev);
--
2.14.1
^ permalink raw reply related
* [PATCH net-next] net: dsa: add port fdb dump
From: Vivien Didelot @ 2017-09-20 23:32 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
Dumping a DSA port's FDB entries is not specific to a DSA slave, so add
a dsa_port_fdb_dump function, similarly to dsa_port_fdb_add and
dsa_port_fdb_del.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa_priv.h | 1 +
net/dsa/port.c | 11 +++++++++++
net/dsa/slave.c | 9 ++-------
3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index f616b3444418..9803952a5b40 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -128,6 +128,7 @@ int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
u16 vid);
int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
u16 vid);
+int dsa_port_fdb_dump(struct dsa_port *dp, dsa_fdb_dump_cb_t *cb, void *data);
int dsa_port_mdb_add(struct dsa_port *dp,
const struct switchdev_obj_port_mdb *mdb,
struct switchdev_trans *trans);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 659676ba3f8b..76d43a82d397 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -173,6 +173,17 @@ int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
return dsa_port_notify(dp, DSA_NOTIFIER_FDB_DEL, &info);
}
+int dsa_port_fdb_dump(struct dsa_port *dp, dsa_fdb_dump_cb_t *cb, void *data)
+{
+ struct dsa_switch *ds = dp->ds;
+ int port = dp->index;
+
+ if (!ds->ops->port_fdb_dump)
+ return -EOPNOTSUPP;
+
+ return ds->ops->port_fdb_dump(ds, port, cb, data);
+}
+
int dsa_port_mdb_add(struct dsa_port *dp,
const struct switchdev_obj_port_mdb *mdb,
struct switchdev_trans *trans)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index bc376fc1bbe7..b87ed29b5b55 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -263,16 +263,11 @@ dsa_slave_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
};
struct dsa_slave_priv *p = netdev_priv(dev);
struct dsa_port *dp = p->dp;
- struct dsa_switch *ds = dp->ds;
int err;
- if (!ds->ops->port_fdb_dump)
- return -EOPNOTSUPP;
-
- err = ds->ops->port_fdb_dump(ds, dp->index,
- dsa_slave_port_fdb_do_dump,
- &dump);
+ err = dsa_port_fdb_dump(dp, dsa_slave_port_fdb_do_dump, &dump);
*idx = dump.idx;
+
return err;
}
--
2.14.1
^ permalink raw reply related
* [PATCH v2 19/31] timer: Remove open-coded casts for .data and .function
From: Kees Cook @ 2017-09-20 23:27 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, Samuel Ortiz, Tyrel Datwyler, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, James E.J. Bottomley,
Martin K. Petersen, netdev, linux-scsi, linuxppc-dev,
linux-kernel
In-Reply-To: <1505950075-50223-1-git-send-email-keescook@chromium.org>
This standardizes the callback and data prototypes in several places that
perform casting, in an effort to remove more open-coded .data and
.function uses in favor of setup_timer().
Cc: Samuel Ortiz <samuel@sortiz.org>
Cc: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "James E.J. Bottomley" <jejb@linux.vnet.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: netdev@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com> # for ibmvscsi
---
drivers/scsi/ibmvscsi/ibmvfc.c | 14 ++++++--------
drivers/scsi/ibmvscsi/ibmvscsi.c | 8 ++++----
drivers/staging/irda/drivers/bfin_sir.c | 5 +++--
3 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index b491af31a5f8..66d51052caa1 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -1393,8 +1393,9 @@ static int ibmvfc_map_sg_data(struct scsi_cmnd *scmd,
*
* Called when an internally generated command times out
**/
-static void ibmvfc_timeout(struct ibmvfc_event *evt)
+static void ibmvfc_timeout(unsigned long data)
{
+ struct ibmvfc_event *evt = (struct ibmvfc_event *)data;
struct ibmvfc_host *vhost = evt->vhost;
dev_err(vhost->dev, "Command timed out (%p). Resetting connection\n", evt);
ibmvfc_reset_host(vhost);
@@ -1424,12 +1425,10 @@ static int ibmvfc_send_event(struct ibmvfc_event *evt,
BUG();
list_add_tail(&evt->queue, &vhost->sent);
- init_timer(&evt->timer);
+ setup_timer(&evt->timer, ibmvfc_timeout, (unsigned long)evt);
if (timeout) {
- evt->timer.data = (unsigned long) evt;
evt->timer.expires = jiffies + (timeout * HZ);
- evt->timer.function = (void (*)(unsigned long))ibmvfc_timeout;
add_timer(&evt->timer);
}
@@ -3692,8 +3691,9 @@ static void ibmvfc_tgt_adisc_cancel_done(struct ibmvfc_event *evt)
* out, reset the CRQ. When the ADISC comes back as cancelled,
* log back into the target.
**/
-static void ibmvfc_adisc_timeout(struct ibmvfc_target *tgt)
+static void ibmvfc_adisc_timeout(unsigned long data)
{
+ struct ibmvfc_target *tgt = (struct ibmvfc_target *)data;
struct ibmvfc_host *vhost = tgt->vhost;
struct ibmvfc_event *evt;
struct ibmvfc_tmf *tmf;
@@ -3778,9 +3778,7 @@ static void ibmvfc_tgt_adisc(struct ibmvfc_target *tgt)
if (timer_pending(&tgt->timer))
mod_timer(&tgt->timer, jiffies + (IBMVFC_ADISC_TIMEOUT * HZ));
else {
- tgt->timer.data = (unsigned long) tgt;
tgt->timer.expires = jiffies + (IBMVFC_ADISC_TIMEOUT * HZ);
- tgt->timer.function = (void (*)(unsigned long))ibmvfc_adisc_timeout;
add_timer(&tgt->timer);
}
@@ -3912,7 +3910,7 @@ static int ibmvfc_alloc_target(struct ibmvfc_host *vhost, u64 scsi_id)
tgt->vhost = vhost;
tgt->need_login = 1;
tgt->cancel_key = vhost->task_set++;
- init_timer(&tgt->timer);
+ setup_timer(&tgt->timer, ibmvfc_adisc_timeout, (unsigned long)tgt);
kref_init(&tgt->kref);
ibmvfc_init_tgt(tgt, ibmvfc_tgt_implicit_logout);
spin_lock_irqsave(vhost->host->host_lock, flags);
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 7d156b161482..ffe917f02695 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -837,8 +837,9 @@ static void ibmvscsi_reset_host(struct ibmvscsi_host_data *hostdata)
*
* Called when an internally generated command times out
*/
-static void ibmvscsi_timeout(struct srp_event_struct *evt_struct)
+static void ibmvscsi_timeout(unsigned long data)
{
+ struct srp_event_struct *evt_struct = (struct srp_event_struct *)data;
struct ibmvscsi_host_data *hostdata = evt_struct->hostdata;
dev_err(hostdata->dev, "Command timed out (%x). Resetting connection\n",
@@ -927,11 +928,10 @@ static int ibmvscsi_send_srp_event(struct srp_event_struct *evt_struct,
*/
list_add_tail(&evt_struct->list, &hostdata->sent);
- init_timer(&evt_struct->timer);
+ setup_timer(&evt_struct->timer, ibmvscsi_timeout,
+ (unsigned long)evt_struct);
if (timeout) {
- evt_struct->timer.data = (unsigned long) evt_struct;
evt_struct->timer.expires = jiffies + (timeout * HZ);
- evt_struct->timer.function = (void (*)(unsigned long))ibmvscsi_timeout;
add_timer(&evt_struct->timer);
}
diff --git a/drivers/staging/irda/drivers/bfin_sir.c b/drivers/staging/irda/drivers/bfin_sir.c
index 3151b580dbd6..c9413bd580a7 100644
--- a/drivers/staging/irda/drivers/bfin_sir.c
+++ b/drivers/staging/irda/drivers/bfin_sir.c
@@ -317,8 +317,9 @@ static void bfin_sir_dma_rx_chars(struct net_device *dev)
async_unwrap_char(dev, &self->stats, &self->rx_buff, port->rx_dma_buf.buf[i]);
}
-void bfin_sir_rx_dma_timeout(struct net_device *dev)
+void bfin_sir_rx_dma_timeout(unsigned long data)
{
+ struct net_device *dev = (struct net_device *)data;
struct bfin_sir_self *self = netdev_priv(dev);
struct bfin_sir_port *port = self->sir_port;
int x_pos, pos;
@@ -406,7 +407,7 @@ static int bfin_sir_startup(struct bfin_sir_port *port, struct net_device *dev)
enable_dma(port->rx_dma_channel);
port->rx_dma_timer.data = (unsigned long)(dev);
- port->rx_dma_timer.function = (void *)bfin_sir_rx_dma_timeout;
+ port->rx_dma_timer.function = bfin_sir_rx_dma_timeout;
#else
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 09/14] gtp: Allow configuring GTP interface as standalone
From: Harald Welte @ 2017-09-21 0:13 UTC (permalink / raw)
To: Tom Herbert
Cc: Andreas Schultz, Tom Herbert, David S. Miller,
Linux Kernel Network Developers, Pablo Neira Ayuso, Rohit Seth
In-Reply-To: <CAPDqMeq1tLi=NkLyqma2OEHQi-fYEYeTU-8th9WKoZaeWiM0Rw@mail.gmail.com>
Hi Tom,
On Wed, Sep 20, 2017 at 09:24:07AM -0700, Tom Herbert wrote:
> On Wed, Sep 20, 2017 at 9:07 AM, Andreas Schultz <aschultz@tpip.net> wrote:
> > GTP isn't special, I just don't like to have testing only features in there
> > when the same goal can be reached without having to add extra stuff. Adding
> > code that is not going to be useful in real production setups (or in this
> > case would even break production setups when enabled accidentally) makes the
> > implementation more complex than it needs to be.
>
> Well, you could make the same argument that allowing GTP to configured
> as standalone interface is a problem since GTP is only allowed to be
> with used with GTP-C. But, then we have something in the kernel that
> the community is expected to support, but requires jumping through a
> whole bunch of hoops just to run a simple netperf.
"A whole bunch of hoops" without your new interface would consist of
running a single command-line program that is supplied with libgtpnl.
This is not a complete 3GPP network, but a simple libmnl-based helper
library with no other depenencies.
I'm not neccessarily against introducing features like the 'standalone
interface configuration'. However, we must make sure that any
significant new feature contributions like IPv6 are tested in a
"realistic setup" and not just using those 'interfaces added for easy
development'. Also, I would argue those 'interfaces added for easy
deveopment/benchmarking' should probably be clearly marked as such to
avoid raising the impression that this is what leads to a
standard-conforming / production-type setup.
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6
From: Harald Welte @ 2017-09-21 0:04 UTC (permalink / raw)
To: Tom Herbert
Cc: David Miller, Tom Herbert, Linux Kernel Network Developers,
Pablo Neira Ayuso, Rohit Seth
In-Reply-To: <CALx6S36zz1CFq75S5-=UK8wtuK-pxaQdWU7tVuRGhtb_Y9QkDg@mail.gmail.com>
Hi Tom,
On Wed, Sep 20, 2017 at 01:40:54PM -0700, Tom Herbert wrote:
> On Wed, Sep 20, 2017 at 12:45 PM, David Miller <davem@davemloft.net> wrote:
> > There is a socket associated with the tunnel to do the encapsulation
> > and it has an address family, right?
>
> If fd's are set from userspace for the sockets then we could derive
> the address family from them. I'll change that. Although, looking at
> now I am wondering why were passing fds into GTP instead of just
> having the kernel create the UDP port like is done for other encaps.
because the userspace process has to take care of those bits of GTP-U
that the kernel doesn't, such as responding to GTP ECHO requests with
GTP echo responses. Only the "GTP Message type G-PDU" is handled in the
kernel, as only those frames contain user plane. See table 1 of Section
7.1 of 3GPP TS 29.060.
If you create the socket in the kernel, how would you hand the socket to
the userspace process later on?
IMHO, it feels more natural to simply create it in userspace (like you
would do in the non-kernel-accelerated case) and then simply handle the
G-PDU messages in the kernel while doing the rest in userspace.
But if there's another method that feels more usual to the kernel
community, I'm not against any changes - but given kernel policies, we'd
have to keep userspace compatbility, right?
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6
From: Tom Herbert @ 2017-09-21 0:16 UTC (permalink / raw)
To: Harald Welte
Cc: Tom Herbert, David Miller, Linux Kernel Network Developers,
Pablo Neira Ayuso, Rohit Seth
In-Reply-To: <20170921000437.rg2h6vyxsrbc3bel@nataraja>
On Wed, Sep 20, 2017 at 5:04 PM, Harald Welte <laforge@gnumonks.org> wrote:
> Hi Tom,
>
> On Wed, Sep 20, 2017 at 01:40:54PM -0700, Tom Herbert wrote:
>> On Wed, Sep 20, 2017 at 12:45 PM, David Miller <davem@davemloft.net> wrote:
>> > There is a socket associated with the tunnel to do the encapsulation
>> > and it has an address family, right?
>>
>> If fd's are set from userspace for the sockets then we could derive
>> the address family from them. I'll change that. Although, looking at
>> now I am wondering why were passing fds into GTP instead of just
>> having the kernel create the UDP port like is done for other encaps.
>
> because the userspace process has to take care of those bits of GTP-U
> that the kernel doesn't, such as responding to GTP ECHO requests with
> GTP echo responses. Only the "GTP Message type G-PDU" is handled in the
> kernel, as only those frames contain user plane. See table 1 of Section
> 7.1 of 3GPP TS 29.060.
>
Okay, thanks for the explanation.
Tom
^ permalink raw reply
* Re: [v2,1/3] can: m_can: Make hclk optional
From: Franklin S Cooper Jr @ 2017-09-21 0:31 UTC (permalink / raw)
To: Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-can-u79uwXL29TY76Z2rM5mHXA, wg-5Yr1BZd7O62+XT7JhA+gdA,
mkl-bIcnvbaLZ9MEGnE8C9+IrQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
Cc: Kristo, Tero, Tony Lindgren, Linux OMAP List
In-Reply-To: <6f574628-51cd-0db8-e5aa-e7ae4a9cf79a-l0cyMroinI0@public.gmane.org>
On 08/24/2017 03:00 AM, Sekhar Nori wrote:
> + some OMAP folks and Linux OMAP list
>
> On Tuesday 25 July 2017 04:21 AM, Franklin Cooper wrote:
>> Hclk is the MCAN's interface clock. However, for OMAP based devices such as
>> DRA7 SoC family the interface clock is handled by hwmod. Therefore, this
>> interface clock is managed by hwmod driver via pm_runtime_get and
>> pm_runtime_put calls. Therefore, this interface clock isn't defined in DT
>> and thus the driver shouldn't fail if this clock isn't found.
>
> I agree that hclk is defined as interface clock for M_CAN IP on DRA76x.
>
> However, there may be a need for the driver to know the value of hclk to
> properly configure the RAM watchdog register which has a counter
> counting down using hclk. Looks like the driver does not use the RAM
> watchdog today. But if there is a need to configure it in future, it
> might be a problem.
Honestly the RAM watchdog seems like a fundamental design problem.
This RAM watchdog seems to be used in case a request to access the
message ram is made but it hangs for what ever reason. Its even more
complicated since the Message RAM is external to the MCAN IP so its
implementation or how its handled probably differs from device to
device. From example say you do have this error it isn't clear how you
would recover from it. A logically answer would be to reset the entire
IP but that also assumes that Message RAM will be reset along with the
ip which likely depends on each SoC.
But if a readl/writel command hangs will the kernel eventually throw an
error on its on or will the driver just hang? If it does hang can a
driver in the ISR do something to properly terminate the driver or even
recover from it?
>
> Is there a restriction in OMAP architecture against passing the
> interface clock also in the 'clocks' property in DT. I have not tried it
> myself, but wonder if you hit an issue that led to this patch.
No but not passing the interface clock is typical.
>
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>> ---
>> Version 2 changes:
>> Used NULL instead of 0 for unused hclk handle
>>
>> drivers/net/can/m_can/m_can.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>> index f4947a7..ea48e59 100644
>> --- a/drivers/net/can/m_can/m_can.c
>> +++ b/drivers/net/can/m_can/m_can.c
>> @@ -1568,8 +1568,13 @@ static int m_can_plat_probe(struct platform_device *pdev)
>> hclk = devm_clk_get(&pdev->dev, "hclk");
>> cclk = devm_clk_get(&pdev->dev, "cclk");
>>
>> - if (IS_ERR(hclk) || IS_ERR(cclk)) {
>> - dev_err(&pdev->dev, "no clock found\n");
>> + if (IS_ERR(hclk)) {
>> + dev_warn(&pdev->dev, "hclk could not be found\n");
>> + hclk = NULL;
>
> What is the purpose of NULL setting the clock. I think this is taking it
> into a very implementation defined territory and the result could be
> different on different architectures. See Russell's explanation here:
> https://lkml.org/lkml/2016/11/10/799
>
> Thanks,
> Sekhar
>
>> + }
>> +
>> + if (IS_ERR(cclk)) {
>> + dev_err(&pdev->dev, "cclk could not be found\n");
>> ret = -ENODEV;
>> goto failed_ret;
>> }
>>
>
--
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: [v2,3/3] can: m_can: Add PM Runtime
From: Franklin S Cooper Jr @ 2017-09-21 0:36 UTC (permalink / raw)
To: Sekhar Nori, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-can-u79uwXL29TY76Z2rM5mHXA, wg-5Yr1BZd7O62+XT7JhA+gdA,
mkl-bIcnvbaLZ9MEGnE8C9+IrQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
Cc: Linux OMAP List, Alexandre Belloni, Wenyou Yang,
Mario Hüttel
In-Reply-To: <8037f5f9-d51f-99c3-f2e8-62c90e56a6fa-l0cyMroinI0@public.gmane.org>
On 08/24/2017 03:30 AM, Sekhar Nori wrote:
> + OMAP mailing list
>
> On Tuesday 25 July 2017 04:21 AM, Franklin Cooper wrote:
>> Add support for PM Runtime which is the new way to handle managing clocks.
>> However, to avoid breaking SoCs not using PM_RUNTIME leave the old clk
>> management approach in place.
>
> Since hclk is the interface clock, I think at a minimum there needs to
> be an assumption that pm_runtime_get_sync() will enable that clock and
> make the module ready for access.
>
> The only in-kernel user of this driver seems to be an atmel SoC. I am
> ccing folks who added support for M_CAN on that SoC to see if hclk
> management can be left to pm_runtime*() on their SoC.
>
> If they are okay, I think its a good to atleast drop explicit hclk
> enable disable in the driver. That way, we avoid double enable/disable
> of interface clock (hclk).
Wenyou,
Do you foresee this being a problem on your board? If not I can send a
v3 removing these clk_enable and clk_disable calls and it would be great
if you can test it out and provide your tested by.
>
>>
>> PM_RUNTIME is required by OMAP based devices to handle clock management.
>> Therefore, this allows future Texas Instruments SoCs that have the MCAN IP
>> to work with this driver.
>>
>> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>
> Thanks,
> Sekhar
>
>> ---
>> drivers/net/can/m_can/m_can.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>> index ea48e59..93e23f5 100644
>> --- a/drivers/net/can/m_can/m_can.c
>> +++ b/drivers/net/can/m_can/m_can.c
>> @@ -23,6 +23,7 @@
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> #include <linux/iopoll.h>
>> #include <linux/can/dev.h>
>>
>> @@ -633,11 +634,15 @@ static int m_can_clk_start(struct m_can_priv *priv)
>> if (err)
>> clk_disable_unprepare(priv->hclk);
>>
>> + pm_runtime_get_sync(priv->device);
>> +
>> return err;
>> }
>>
>> static void m_can_clk_stop(struct m_can_priv *priv)
>> {
>> + pm_runtime_put_sync(priv->device);
>> +
>> clk_disable_unprepare(priv->cclk);
>> clk_disable_unprepare(priv->hclk);
>> }
>> @@ -1582,6 +1587,8 @@ static int m_can_plat_probe(struct platform_device *pdev)
>> /* Enable clocks. Necessary to read Core Release in order to determine
>> * M_CAN version
>> */
>> + pm_runtime_enable(&pdev->dev);
>> +
>> ret = clk_prepare_enable(hclk);
>> if (ret)
>> goto disable_hclk_ret;
>> @@ -1626,6 +1633,8 @@ static int m_can_plat_probe(struct platform_device *pdev)
>> */
>> tx_fifo_size = mram_config_vals[7];
>>
>> + pm_runtime_get_sync(&pdev->dev);
>> +
>> /* allocate the m_can device */
>> dev = alloc_m_can_dev(pdev, addr, tx_fifo_size);
>> if (!dev) {
>> @@ -1670,6 +1679,7 @@ static int m_can_plat_probe(struct platform_device *pdev)
>> disable_hclk_ret:
>> clk_disable_unprepare(hclk);
>> failed_ret:
>> + pm_runtime_put_sync(&pdev->dev);
>> return ret;
>> }
>>
>> @@ -1726,6 +1736,9 @@ static int m_can_plat_remove(struct platform_device *pdev)
>> struct net_device *dev = platform_get_drvdata(pdev);
>>
>> unregister_m_can_dev(dev);
>> +
>> + pm_runtime_disable(&pdev->dev);
>> +
>> platform_set_drvdata(pdev, NULL);
>>
>> free_m_can_dev(dev);
>>
>
--
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: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
From: Franklin S Cooper Jr @ 2017-09-21 0:48 UTC (permalink / raw)
To: Mario Hüttel, Yang, Wenyou, Sekhar Nori, wg, mkl, socketcan,
quentin.schulz, edumazet, linux-can, netdev, linux-kernel
Cc: Wenyou Yang, Dong Aisheng
In-Reply-To: <e3a7d9a1-d75c-a39e-f122-73fced02a1dc@gmx.net>
On 09/20/2017 04:37 PM, Mario Hüttel wrote:
>
>
> On 09/20/2017 10:19 PM, Franklin S Cooper Jr wrote:
>> Hi Wenyou,
>>
>> On 09/17/2017 10:47 PM, Yang, Wenyou wrote:
>>>
>>> On 2017/9/14 13:06, Sekhar Nori wrote:
>>>> On Thursday 14 September 2017 03:28 AM, Franklin S Cooper Jr wrote:
>>>>> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote:
>>>>>> During test transmitting using CAN-FD at high bitrates (4 Mbps) only
>>>>>> resulted in errors. Scoping the signals I noticed that only a single
>>>>>> bit
>>>>>> was being transmitted and with a bit more investigation realized the
>>>>>> actual
>>>>>> MCAN IP would go back to initialization mode automatically.
>>>>>>
>>>>>> It appears this issue is due to the MCAN needing to use the Transmitter
>>>>>> Delay Compensation Mode as defined in the MCAN User's Guide. When this
>>>>>> mode is used the User's Guide indicates that the Transmitter Delay
>>>>>> Compensation Offset register should be set. The document mentions
>>>>>> that this
>>>>>> register should be set to (1/dbitrate)/2*(Func Clk Freq).
>>>>>>
>>>>>> Additional CAN-CIA's "Bit Time Requirements for CAN FD" document
>>>>>> indicates
>>>>>> that this TDC mode is only needed for data bit rates above 2.5 Mbps.
>>>>>> Therefore, only enable this mode and only set TDCO when the data bit
>>>>>> rate
>>>>>> is above 2.5 Mbps.
>>>>>>
>>>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>>>> ---
>>>>>> I'm pretty surprised that this hasn't been implemented already since
>>>>>> the primary purpose of CAN-FD is to go beyond 1 Mbps and the MCAN IP
>>>>>> supports up to 10 Mbps.
>>>>>>
>>>>>> So it will be nice to get comments from users of this driver to
>>>>>> understand
>>>>>> if they have been able to use CAN-FD beyond 2.5 Mbps without this
>>>>>> patch.
>>>>>> If they haven't what did they do to get around it if they needed higher
>>>>>> speeds.
>>>>>>
>>>>>> Meanwhile I plan on testing this using a more "realistic" CAN bus to
>>>>>> insure
>>>>>> everything still works at 5 Mbps which is the max speed of my CAN
>>>>>> transceiver.
>>>>> ping. Anyone has any thoughts on this?
>>>> I added Dong who authored the m_can driver and Wenyou who added the only
>>>> in-kernel user of the driver for any help.
>>> I tested it on SAMA5D2 Xplained board both with and without this patch,
>>> both work with the 4M bps data bit rate.
>> Thank you for testing this out. Its interesting that you have been able
>> to use higher speeds without this patch. What is the CAN transceiver
>> being used on the SAMA5D2 Xplained board? I tried looking at the
>> schematic but it seems the CAN signals are used on an extension board
>> which I can't find the schematic for. Also do you mind sharing your test
>> setup? Were you doing a short point to point test?
>>
>> Thank You,
>> Franklin
> Hello Franklin,
>
> your patch definitely makes sense.
>
> I forgot the TDC in my patches because it was not present in the
> previous driver versions and because I didn't encounter any
> problems when testing it myself.
>
> The error is highly dependent on the hardware (transceiver) setup.
> So it is definitely possible that some people don't encounter errors
> without your patch.
So the Transmission Delay Compensation feature Value register is suppose
to take into consideration the transceiver delay automatically and add
the value of TDCO on top of that. So why would TDCO be dependent on the
transceiver? I've heard conflicting things regarding TDC so any
clarification on what actually impacts it would be appreciated.
Also part of the issue I'm having is how can we properly configure TDCO?
Configuring TDCO is essentially figuring out what Secondary Sample Point
to use. However, it is unclear what value to set SSP to and which use
cases a given SSP will work or doesn't work. I've seen various
recommendations from Bosch on choosing SSP but ultimately it seems they
suggestion "real world testing" to come up with a proper value. Not
setting TDCO causes problems for my device and improperly setting TDCO
causes problems for my device. So its likely any value I use could end
up breaking something for someone else.
Currently I leaning to a DT property that can be used for setting SSP.
Perhaps use a generic default value and allow individuals to override it
via DT?
>
> Could you clarify what you meant with
>> Scoping the signals I noticed that only a single bit was being transmitted
>
> Do you mean one data bit (high bit rate) or did the core already fail
> in the arbitration phase?
A single bit during the arbitration phase. Essentially the Start of
Frame bit is sent and then nothing else and the IP resets.
>
> There is also another aspect that can lead to errors:
>
> If the CAN clock 'cclk' is above the frequency of the interface/logic
> clock 'hclk', the clock domain crossing of the CAN messages can't
> work properly. However, I will throw this topic as an extra e-mail into
> the round.
>
> Mario
>
>
>
^ permalink raw reply
* Re: [PATCH iproute2] tc: fq: support low_rate_threshold attribute
From: Stephen Hemminger @ 2017-09-21 0:53 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Neal Cardwell, Soheil Hassas Yeganeh
In-Reply-To: <1504905179.15310.101.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, 08 Sep 2017 14:12:59 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> TCA_FQ_LOW_RATE_THRESHOLD sch_fq attribute was added in linux-4.9
>
> Tested:
>
> lpaa5:/tmp# tc -qd add dev eth1 root fq
> lpaa5:/tmp# tc -s qd sh dev eth1
> qdisc fq 8003: root refcnt 5 limit 10000p flow_limit 1000p buckets 4096 \
> orphan_mask 4095 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 quantum 3648 \
> initial_quantum 18240 low_rate_threshold 550Kbit refill_delay 40.0ms
> Sent 62139 bytes 395 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> 116 flows (114 inactive, 0 throttled)
> 1 gc, 0 highprio, 0 throttled
>
> lpaa5:/tmp# ./netperf -H lpaa6 -t TCP_RR -l10 -- -q 500000 -r 300,300 -o P99_LATENCY
> 99th Percentile Latency Microseconds
> 7081
>
> lpaa5:/tmp# tc qd replace dev eth1 root fq low_rate_threshold 10Mbit
> lpaa5:/tmp# ./netperf -H lpaa6 -t TCP_RR -l10 -- -q 500000 -r 300,300 -o P99_LATENCY
> 99th Percentile Latency Microseconds
> 858
>
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied
^ 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