* Re: [PATCH] MAINTAINERS: change bridge maintainers
From: David Miller @ 2018-09-28 17:28 UTC (permalink / raw)
To: stephen; +Cc: netdev, sthemmin
In-Reply-To: <20180927084701.3468-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 27 Sep 2018 10:47:01 +0200
> I haven't been doing reviews only but not active development on bridge
> code for several years. Roopa and Nikolay have been doing most of
> the new features and have agreed to take over as new co-maintainers.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Thanks for all of the years of watching over the bridge code Stephen.
Applied.
^ permalink raw reply
* [PATCH net-next] tcp/fq: move back to CLOCK_MONOTONIC
From: Eric Dumazet @ 2018-09-28 17:28 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet, Leonard Crestez
In the recent TCP/EDT patch series, I switched TCP and sch_fq
clocks from MONOTONIC to TAI, in order to meet the choice done
earlier for sch_etf packet scheduler.
But sure enough, this broke some setups were the TAI clock
jumps forward (by almost 50 year...), as reported
by Leonard Crestez.
If we want to converge later, we'll probably need to add
an skb field to differentiate the clock bases, or a socket option.
In the meantime, an UDP application will need to use CLOCK_MONOTONIC
base for its SCM_TXTIME timestamps if using fq packet scheduler.
Fixes: 72b0094f9182 ("tcp: switch tcp_clock_ns() to CLOCK_TAI base")
Fixes: 142537e41923 ("net_sched: sch_fq: switch to CLOCK_TAI")
Fixes: fd2bca2aa789 ("tcp: switch internal pacing timer to CLOCK_TAI")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Leonard Crestez <leonard.crestez@nxp.com>
---
include/net/tcp.h | 2 +-
net/ipv4/tcp_timer.c | 2 +-
net/sched/sch_fq.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index ff15d8e0d525715b17671e64f6abdead9df0a8f3..0d2929223c703c0aaabef0d485aabf7dc707aae1 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -732,7 +732,7 @@ void tcp_send_window_probe(struct sock *sk);
static inline u64 tcp_clock_ns(void)
{
- return ktime_get_tai_ns();
+ return ktime_get_ns();
}
static inline u64 tcp_clock_us(void)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 4f661e178da8465203266ff4dfa3e8743e60ff82..61023d50cd604d5e19464a32c33b65d29c75c81e 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -758,7 +758,7 @@ void tcp_init_xmit_timers(struct sock *sk)
{
inet_csk_init_xmit_timers(sk, &tcp_write_timer, &tcp_delack_timer,
&tcp_keepalive_timer);
- hrtimer_init(&tcp_sk(sk)->pacing_timer, CLOCK_TAI,
+ hrtimer_init(&tcp_sk(sk)->pacing_timer, CLOCK_MONOTONIC,
HRTIMER_MODE_ABS_PINNED_SOFT);
tcp_sk(sk)->pacing_timer.function = tcp_pace_kick;
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 628a2cdcfc6f2fa69d9402f06881949d2e1423d9..338222a6c664b1825aaada4355e2fc0a01db9c73 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -412,7 +412,7 @@ static void fq_check_throttled(struct fq_sched_data *q, u64 now)
static struct sk_buff *fq_dequeue(struct Qdisc *sch)
{
struct fq_sched_data *q = qdisc_priv(sch);
- u64 now = ktime_get_tai_ns();
+ u64 now = ktime_get_ns();
struct fq_flow_head *head;
struct sk_buff *skb;
struct fq_flow *f;
@@ -776,7 +776,7 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt,
q->fq_trees_log = ilog2(1024);
q->orphan_mask = 1024 - 1;
q->low_rate_threshold = 550000 / 8;
- qdisc_watchdog_init_clockid(&q->watchdog, sch, CLOCK_TAI);
+ qdisc_watchdog_init_clockid(&q->watchdog, sch, CLOCK_MONOTONIC);
if (opt)
err = fq_change(sch, opt, extack);
@@ -831,7 +831,7 @@ static int fq_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
st.flows_plimit = q->stat_flows_plimit;
st.pkts_too_long = q->stat_pkts_too_long;
st.allocation_errors = q->stat_allocation_errors;
- st.time_next_delayed_flow = q->time_next_delayed_flow - ktime_get_tai_ns();
+ st.time_next_delayed_flow = q->time_next_delayed_flow - ktime_get_ns();
st.flows = q->flows;
st.inactive_flows = q->inactive_flows;
st.throttled_flows = q->throttled_flows;
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: [PATCH v3 0/5] netlink: nested policy validation
From: David Miller @ 2018-09-28 17:25 UTC (permalink / raw)
To: johannes; +Cc: netdev, dsahern
In-Reply-To: <1538036562.14416.23.camel@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 27 Sep 2018 10:22:42 +0200
> On Wed, 2018-09-26 at 10:21 -0700, David Miller wrote:
>> From: Johannes Berg <johannes@sipsolutions.net>
>> Date: Wed, 26 Sep 2018 11:15:29 +0200
>>
>> > This adds nested policy validation, which lets you specify the
>> > nested attribute type, e.g. NLA_NESTED with sub-policy, or the
>> > new NLA_NESTED_ARRAY with sub-sub-policy.
>> >
>> >
>> > Changes in v2:
>> > * move setting the bad attr pointer/message into validate_nla()
>> > * remove the recursion patch since that's no longer needed
>> > * simply skip the generic bad attr pointer/message setting in
>> > case of nested nla_validate() failing since that could fail
>> > only due to validate_nla() failing inside, which already sets
>> > the extack information
>> >
>> > Changes in v3:
>> > * fix NLA_REJECT to have an error message if none is in policy
>>
>> Looks great Johannes, series applied.
>
> Sorry to nag, but I see patches that you replied to later than this in
> the tree, but not this.
>
> Or did you see something wrong with this later and dropped it?
Ugh, the perils of working on multiple machines :-/
This should be fixed now and your netlink changes pushed out to net-next.
Sorry about that.
^ permalink raw reply
* Re: bond: take rcu lock in bond_poll_controller
From: Dave Jones @ 2018-09-28 17:25 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpUVMszSA4ZyiJJgJAu053DZ3RbU91kBewrMMiWd7E19Lg@mail.gmail.com>
On Fri, Sep 28, 2018 at 09:55:52AM -0700, Cong Wang wrote:
> On Fri, Sep 28, 2018 at 9:18 AM Dave Jones <davej@codemonkey.org.uk> wrote:
> >
> > Callers of bond_for_each_slave_rcu are expected to hold the rcu lock,
> > otherwise a trace like below is shown
>
> So why not take rcu read lock in netpoll_send_skb_on_dev() where
> RCU is also assumed?
that does seem to solve the backtrace spew I saw too, so if that's
preferable I can respin the patch.
> As I said, I can't explain why you didn't trigger the RCU warning in
> netpoll_send_skb_on_dev()...
netpoll_send_skb_on_dev takes the rcu lock itself.
Dave
^ permalink raw reply
* rcu_read_lock() in tcf_block_find()
From: Cong Wang @ 2018-09-28 17:19 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Network Developers, Jiri Pirko, Jamal Hadi Salim,
Vlad Buslov
In-Reply-To: <4817f6db-bd55-2885-ac16-6fc7d6879b7e@gmail.com>
(Changing $subject as the discussion is going to a completely different topic)
On Thu, Sep 27, 2018 at 3:19 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/27/2018 02:36 PM, Cong Wang wrote:
>
> > I don't understand what you mean by changing ip command, you must
> > mean tc command, but still, I have no idea about how restarting failed
> > syscall could be related to my patch and why we need to restart anything
> > here. If the refcnt goes to 0, it will never come back, retrying won't help
> > anything.
> >
>
> Yep, tc command it is.
>
> I was not especially commenting your patch (replacing an english message by another does
> not seem very big deal), but the fact that the code right there seems to be prepared
> for parallel changes.
>
> But using RCU lookups in control path will lead to occasional failures
> that most user space tools would not expect.
>
I already discussed this with Vlad in the beginning of his RTNL
removal patches, we both agreed some lock is still needed, it is not
completely lockless. Take a look at tc action code now, two spinlocks
are still needed even after we will remove the RTNL there.
> Lets assume two tasks are launching "tc qdisc replace dev eth0 root XXX" in whatever order/parallelism.
>
> Both should succeed, after/before major RTNL->other_locking_mechanism
Yes, it is never going to be completely lockless.
>
> Control paths are usually using a mutex or a spinlock so that they never hit a 0-refcount at all.
For dev->qdisc, sure, we should already hold a refcnt, it can't be gone.
For qdisc_lookup_rcu(), it could be that refcnt goes to 0 before we
remove it from hashtable, right? call_rcu() is only called after
refcnt==0, so rcu read lock can't help here.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] qed: Remove set but not used variable 'p_archipelago'
From: David Miller @ 2018-09-28 17:15 UTC (permalink / raw)
To: yuehaibing; +Cc: Ariel.Elior, everest-linux-l2, netdev, kernel-janitors
In-Reply-To: <1538030706-138186-1-git-send-email-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Thu, 27 Sep 2018 06:45:06 +0000
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/qlogic/qed/qed_ooo.c: In function 'qed_ooo_delete_isles':
> drivers/net/ethernet/qlogic/qed/qed_ooo.c:354:30: warning:
> variable 'p_archipelago' set but not used [-Wunused-but-set-variable]
>
> drivers/net/ethernet/qlogic/qed/qed_ooo.c: In function 'qed_ooo_join_isles':
> drivers/net/ethernet/qlogic/qed/qed_ooo.c:463:30: warning:
> variable 'p_archipelago' set but not used [-Wunused-but-set-variable]
>
> Since commit 1eec2437d14c ("qed: Make OOO archipelagos into an array"),
> 'p_archipelago' is no longer in use.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thanks.
^ permalink raw reply
* [PATCH iproute2] lib/libnetlink: fix response seq check
From: vlad @ 2018-09-28 17:13 UTC (permalink / raw)
To: stephen; +Cc: chrism, netdev, maheshb, Vlad Dumitrescu
From: Vlad Dumitrescu <vladum@google.com>
Taking a one-iovec example, with rtnl->seq at 42. iovlen == 1, seq
becomes 43 on line 604, and a message is sent with nlmsg_seq == 43. If
a response with nlmsg_seq of 42 is received, the condition being fixed
in this patch would incorrectly accept it.
Fixes: 72a2ff3916e5 ("lib/libnetlink: Add a new function rtnl_talk_iov")
Signed-off-by: Vlad Dumitrescu <vladum@google.com>
---
lib/libnetlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index f18dceac..4d2416bf 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -647,7 +647,7 @@ static int __rtnl_talk_iov(struct rtnl_handle *rtnl, struct iovec *iov,
if (nladdr.nl_pid != 0 ||
h->nlmsg_pid != rtnl->local.nl_pid ||
- h->nlmsg_seq > seq || h->nlmsg_seq < seq - iovlen) {
+ h->nlmsg_seq > seq || h->nlmsg_seq < seq - iovlen + 1) {
/* Don't forget to skip that message. */
status -= NLMSG_ALIGN(len);
h = (struct nlmsghdr *)((char *)h + NLMSG_ALIGN(len));
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: [Patch net-next] net_sched: fix an extack message in tcf_block_find()
From: Cong Wang @ 2018-09-28 17:04 UTC (permalink / raw)
To: Linux Kernel Network Developers; +Cc: Jiri Pirko, Jamal Hadi Salim, Vlad Buslov
In-Reply-To: <20180927204219.17846-1-xiyou.wangcong@gmail.com>
On Thu, Sep 27, 2018 at 1:42 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> It is clearly a copy-n-paste.
>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
I regret again for wasting my time, so:
NAcked-by: Cong Wang <xiyou.wangcong@gmail.com>
I really don't care, do you?
^ permalink raw reply
* Re: [Patch net-next] net_sched: fix an extack message in tcf_block_find()
From: Cong Wang @ 2018-09-28 17:03 UTC (permalink / raw)
To: Vlad Buslov; +Cc: Linux Kernel Network Developers, Jiri Pirko, Jamal Hadi Salim
In-Reply-To: <vbf1s9dyhub.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
On Fri, Sep 28, 2018 at 4:36 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> On Thu 27 Sep 2018 at 20:42, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > It is clearly a copy-n-paste.
> >
> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> > ---
> > net/sched/cls_api.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> > index 3de47e99b788..8dd7f8af6d54 100644
> > --- a/net/sched/cls_api.c
> > +++ b/net/sched/cls_api.c
> > @@ -655,7 +655,7 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
> >
> > *q = qdisc_refcount_inc_nz(*q);
> > if (!*q) {
> > - NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
> > + NL_SET_ERR_MSG(extack, "Can't increase Qdisc refcount");
> > err = -EINVAL;
> > goto errout_rcu;
> > }
>
> Is there a benefit in exposing this info to user?
Depends on what user you mean here. For kernel developers, yes,
this is useful. For others, no.
> For all intents and purposes Qdisc is gone at that point.
I don't want to be a language lawyer, but there is a difference between
"it doesn't exist" and "it exists but being removed". The errno EINVAL
betrays what you said too, it must be ENOENT to mach "Qdisc is gone".
I don't want to waste my time on this any more. Let's just drop it.
I really don't care, do you?
^ permalink raw reply
* Re: [PATCH iproute2 net-next] ipneigh: support setting of NTF_ROUTER on neigh entries
From: David Ahern @ 2018-09-28 16:57 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: netdev
In-Reply-To: <1537910139-21859-1-git-send-email-roopa@cumulusnetworks.com>
On 9/25/18 3:15 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> ip/ipneigh.c | 2 ++
> 1 file changed, 2 insertions(+)
>
applied to iproute2-next.
And then I noticed you did not update the help or the man page. Please
send a follow up.
^ permalink raw reply
* Re: bond: take rcu lock in bond_poll_controller
From: Cong Wang @ 2018-09-28 16:55 UTC (permalink / raw)
To: Dave Jones; +Cc: Linux Kernel Network Developers
In-Reply-To: <20180928161631.3pab3m2k6zvoemnz@codemonkey.org.uk>
On Fri, Sep 28, 2018 at 9:18 AM Dave Jones <davej@codemonkey.org.uk> wrote:
>
> Callers of bond_for_each_slave_rcu are expected to hold the rcu lock,
> otherwise a trace like below is shown
So why not take rcu read lock in netpoll_send_skb_on_dev() where
RCU is also assumed?
As I said, I can't explain why you didn't trigger the RCU warning in
netpoll_send_skb_on_dev()...
^ permalink raw reply
* [PATCH net v2 7/7] smsc95xx: Check for Wake-on-LAN modes
From: Florian Fainelli @ 2018-09-28 23:18 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, David S. Miller, Woojung Huh,
Microchip Linux Driver Support, Steve Glendinning, Kees Cook,
Alexander Kurz, Hayes Wang, Kai-Heng Feng, Grant Grundler,
zhong jiang, Sebastian Andrzej Siewior, Ran Wang, Eric Dumazet,
open list:USB NETWORKING DRIVERS, open list
In-Reply-To: <20180928231856.3587-1-f.fainelli@gmail.com>
The driver does not check for Wake-on-LAN modes specified by an user,
but will conditionally set the device as wake-up enabled or not based on
that, which could be a very confusing user experience.
Fixes: e0e474a83c18 ("smsc95xx: add wol magic packet support")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/usb/smsc95xx.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 06b4d290784d..262e7a3c23cb 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -774,6 +774,9 @@ static int smsc95xx_ethtool_set_wol(struct net_device *net,
struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
int ret;
+ if (wolinfo->wolopts & ~SUPPORTED_WAKE)
+ return -EINVAL;
+
pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
ret = device_set_wakeup_enable(&dev->udev->dev, pdata->wolopts);
--
2.17.1
^ permalink raw reply related
* [PATCH net v2 5/7] r8152: Check for supported Wake-on-LAN Modes
From: Florian Fainelli @ 2018-09-28 23:18 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, David S. Miller, Woojung Huh,
Microchip Linux Driver Support, Steve Glendinning, Kees Cook,
Alexander Kurz, Hayes Wang, Kai-Heng Feng, Grant Grundler,
zhong jiang, Sebastian Andrzej Siewior, Ran Wang, Eric Dumazet,
open list:USB NETWORKING DRIVERS, open list
In-Reply-To: <20180928231856.3587-1-f.fainelli@gmail.com>
The driver does not check for Wake-on-LAN modes specified by an user,
but will conditionally set the device as wake-up enabled or not based on
that, which could be a very confusing user experience.
Fixes: 21ff2e8976b1 ("r8152: support WOL")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/usb/r8152.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2cd71bdb6484..f1b5201cc320 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -4506,6 +4506,9 @@ static int rtl8152_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
if (!rtl_can_wakeup(tp))
return -EOPNOTSUPP;
+ if (wol->wolopts & ~WAKE_ANY)
+ return -EINVAL;
+
ret = usb_autopm_get_interface(tp->intf);
if (ret < 0)
goto out_set_wol;
--
2.17.1
^ permalink raw reply related
* [PATCH net v2 3/7] lan78xx: Check for supported Wake-on-LAN modes
From: Florian Fainelli @ 2018-09-28 23:18 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, David S. Miller, Woojung Huh,
Microchip Linux Driver Support, Steve Glendinning, Kees Cook,
Alexander Kurz, Hayes Wang, Kai-Heng Feng, Grant Grundler,
zhong jiang, Sebastian Andrzej Siewior, Ran Wang, Eric Dumazet,
open list:USB NETWORKING DRIVERS, open list
In-Reply-To: <20180928231856.3587-1-f.fainelli@gmail.com>
The driver supports a fair amount of Wake-on-LAN modes, but is not
checking that the user specified one that is supported.
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/usb/lan78xx.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index a9991c5f4736..c3c9ba44e2a1 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1401,19 +1401,10 @@ static int lan78xx_set_wol(struct net_device *netdev,
if (ret < 0)
return ret;
- pdata->wol = 0;
- if (wol->wolopts & WAKE_UCAST)
- pdata->wol |= WAKE_UCAST;
- if (wol->wolopts & WAKE_MCAST)
- pdata->wol |= WAKE_MCAST;
- if (wol->wolopts & WAKE_BCAST)
- pdata->wol |= WAKE_BCAST;
- if (wol->wolopts & WAKE_MAGIC)
- pdata->wol |= WAKE_MAGIC;
- if (wol->wolopts & WAKE_PHY)
- pdata->wol |= WAKE_PHY;
- if (wol->wolopts & WAKE_ARP)
- pdata->wol |= WAKE_ARP;
+ if (wol->wolopts & ~WAKE_ALL)
+ return -EINVAL;
+
+ pdata->wol = wol->wolopts;
device_set_wakeup_enable(&dev->udev->dev, (bool)wol->wolopts);
--
2.17.1
^ permalink raw reply related
* [PATCH net v2 2/7] ax88179_178a: Check for supported Wake-on-LAN modes
From: Florian Fainelli @ 2018-09-28 23:18 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, David S. Miller, Woojung Huh,
Microchip Linux Driver Support, Steve Glendinning, Kees Cook,
Alexander Kurz, Hayes Wang, Kai-Heng Feng, Grant Grundler,
zhong jiang, Sebastian Andrzej Siewior, Ran Wang, Eric Dumazet,
open list:USB NETWORKING DRIVERS, open list
In-Reply-To: <20180928231856.3587-1-f.fainelli@gmail.com>
The driver currently silently accepts unsupported Wake-on-LAN modes
(other than WAKE_PHY or WAKE_MAGIC) without reporting that to the user,
which is confusing.
Fixes: e2ca90c276e1 ("ax88179_178a: ASIX AX88179_178A USB 3.0/2.0 to gigabit ethernet adapter driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/usb/ax88179_178a.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 9e8ad372f419..2207f7a7d1ff 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -566,6 +566,9 @@ ax88179_set_wol(struct net_device *net, struct ethtool_wolinfo *wolinfo)
struct usbnet *dev = netdev_priv(net);
u8 opt = 0;
+ if (wolinfo->wolopts & ~(WAKE_PHY | WAKE_MAGIC))
+ return -EINVAL;
+
if (wolinfo->wolopts & WAKE_PHY)
opt |= AX_MONITOR_MODE_RWLC;
if (wolinfo->wolopts & WAKE_MAGIC)
--
2.17.1
^ permalink raw reply related
* [PATCH net v2 0/7] net: usb: Check for Wake-on-LAN modes
From: Florian Fainelli @ 2018-09-28 23:18 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, David S. Miller, Woojung Huh,
Microchip Linux Driver Support, Steve Glendinning, Kees Cook,
Alexander Kurz, Hayes Wang, Kai-Heng Feng, Grant Grundler,
zhong jiang, Sebastian Andrzej Siewior, Ran Wang, Eric Dumazet,
open list:USB NETWORKING DRIVERS, open list
Hi all,
Most of our USB Ethernet drivers don't seem to be checking properly
whether the user is supplying a correct Wake-on-LAN mode to enter, so
the experience as an user could be confusing, since it would generally
lead to either no wake-up, or the device not being marked for wake-up.
Please review!
Changes in v2:
- fixed lan78xx handling, thanks Woojung!
Florian Fainelli (7):
asix: Check for supported Wake-on-LAN modes
ax88179_178a: Check for supported Wake-on-LAN modes
lan78xx: Check for supported Wake-on-LAN modes
sr9800: Check for supported Wake-on-LAN modes
r8152: Check for supported Wake-on-LAN Modes
smsc75xx: Check for Wake-on-LAN modes
smsc95xx: Check for Wake-on-LAN modes
drivers/net/usb/asix_common.c | 3 +++
drivers/net/usb/ax88179_178a.c | 3 +++
drivers/net/usb/lan78xx.c | 17 ++++-------------
drivers/net/usb/r8152.c | 3 +++
drivers/net/usb/smsc75xx.c | 3 +++
drivers/net/usb/smsc95xx.c | 3 +++
drivers/net/usb/sr9800.c | 3 +++
7 files changed, 22 insertions(+), 13 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Andrew Lunn @ 2018-09-28 16:45 UTC (permalink / raw)
To: Edward Cree
Cc: Ariel Almog, linville, Linux Netdev List, ganeshgr,
jakub.kicinski, dustin, dirk.vandermerwe, shayag, ariela
In-Reply-To: <0c1a3a90-e6f5-7f8e-044c-bd43e0cb830a@solarflare.com>
> I see where you're coming from, but if people start needing to manually
> configure FEC on their consumer devices, possibly we have bigger
> problems.
Yes, i agree with that. For the consumer market, SFPs needs to grow up
and start doing full and reliable auto-neg, just like copper Ethernet.
However, there is often an intermediate step after the really niche
market like TOR routers. Industrial applications start using this
stuff. There are a lot of planes flying today using SFPs for the
inflight entertainment systems. Fibre weights less than copper. It is
a somewhat specialist market, so you probably can still force them to
read the hardware manual, but i think they would prefer not to. And
i'm sure they are not the only industrial users. There are likely to
be more industrial users than TOR users.
In general, it is hard to know which APIs are going to remain Unix
Wizard level, and which are going to be used by mere mortals. So
ideally, we want consistency everywhere.
> I think the alternative, of finding a set of semantics that fits
> everyone's hardware and covers everyone's requirements, is likely to
> be difficult (and probably require changing the ethtool API).
Now is a good time to change the API, since we are moving to a netlink
socket. Which is why these questions were asked in the first place...
Andrew
^ permalink raw reply
* [PATCH] net/usb: cancel pending work when unbinding smsc75xx
From: Yu Zhao @ 2018-09-28 23:04 UTC (permalink / raw)
To: David S. Miller, Steve Glendinning
Cc: netdev, linux-usb, linux-kernel, Yu Zhao
Cancel pending work before freeing smsc75xx private data structure
during binding. This fixes the following crash in the driver:
BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
IP: mutex_lock+0x2b/0x3f
<snipped>
Workqueue: events smsc75xx_deferred_multicast_write [smsc75xx]
task: ffff8caa83e85700 task.stack: ffff948b80518000
RIP: 0010:mutex_lock+0x2b/0x3f
<snipped>
Call Trace:
smsc75xx_deferred_multicast_write+0x40/0x1af [smsc75xx]
process_one_work+0x18d/0x2fc
worker_thread+0x1a2/0x269
? pr_cont_work+0x58/0x58
kthread+0xfa/0x10a
? pr_cont_work+0x58/0x58
? rcu_read_unlock_sched_notrace+0x48/0x48
ret_from_fork+0x22/0x40
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
drivers/net/usb/smsc75xx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 05553d252446..b64b1ee56d2d 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -1517,6 +1517,7 @@ static void smsc75xx_unbind(struct usbnet *dev, struct usb_interface *intf)
{
struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
if (pdata) {
+ cancel_work_sync(&pdata->set_multicast);
netif_dbg(dev, ifdown, dev->net, "free pdata\n");
kfree(pdata);
pdata = NULL;
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: WARN_ON in TLP causing RT throttling
From: stranche @ 2018-09-28 16:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Yuchung Cheng, soheil, netdev
In-Reply-To: <7e05c738-8188-8cae-a726-33f780f6b05f@gmail.com>
On 2018-09-27 18:25, Eric Dumazet wrote:
> On 09/27/2018 05:16 PM, stranche@codeaurora.org wrote:
>
>> Hi Yuchung,
>>
>> Based on the dumps we were able to get, it appears that TFO was not
>> used in this case.
>> We also tried some local experiments where we dropped incoming SYN
>> packets after already
>> successful TFO connections on the receive side to see if TFO would
>> trigger this scenario, but
>> have not been able to reproduce it.
>>
>> One other interesting thing we found is that the socket never sent or
>> received any data. It only
>> sent/received the packets for the initial handshake and the outgoing
>> FIN.
>
> Just to make sure : Was this some sort of syzkaller (or other fuzzer)
> run ?
No, this was a normal run.
^ permalink raw reply
* bond: take rcu lock in bond_poll_controller
From: Dave Jones @ 2018-09-28 16:16 UTC (permalink / raw)
To: netdev
In-Reply-To: <20180926.201541.2129538853102457012.davem@davemloft.net>
Callers of bond_for_each_slave_rcu are expected to hold the rcu lock,
otherwise a trace like below is shown
WARNING: CPU: 2 PID: 179 at net/core/dev.c:6567 netdev_lower_get_next_private_rcu+0x34/0x40
CPU: 2 PID: 179 Comm: kworker/u16:15 Not tainted 4.19.0-rc5-backup+ #1
Workqueue: bond0 bond_mii_monitor
RIP: 0010:netdev_lower_get_next_private_rcu+0x34/0x40
Code: 48 89 fb e8 fe 29 63 ff 85 c0 74 1e 48 8b 45 00 48 81 c3 c0 00 00 00 48 8b 00 48 39 d8 74 0f 48 89 45 00 48 8b 40 f8 5b 5d c3 <0f> 0b eb de 31 c0 eb f5 0f 1f 40 00 0f 1f 44 00 00 48 8>
RSP: 0018:ffffc9000087fa68 EFLAGS: 00010046
RAX: 0000000000000000 RBX: ffff880429614560 RCX: 0000000000000000
RDX: 0000000000000001 RSI: 00000000ffffffff RDI: ffffffffa184ada0
RBP: ffffc9000087fa80 R08: 0000000000000001 R09: 0000000000000000
R10: ffffc9000087f9f0 R11: ffff880429798040 R12: ffff8804289d5980
R13: ffffffffa1511f60 R14: 00000000000000c8 R15: 00000000ffffffff
FS: 0000000000000000(0000) GS:ffff88042f880000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f4b78fce180 CR3: 000000018180f006 CR4: 00000000001606e0
Call Trace:
bond_poll_controller+0x52/0x170
netpoll_poll_dev+0x79/0x290
netpoll_send_skb_on_dev+0x158/0x2c0
netpoll_send_udp+0x2d5/0x430
write_ext_msg+0x1e0/0x210
console_unlock+0x3c4/0x630
vprintk_emit+0xfa/0x2f0
printk+0x52/0x6e
? __netdev_printk+0x12b/0x220
netdev_info+0x64/0x80
? bond_3ad_set_carrier+0xe9/0x180
bond_select_active_slave+0x1fc/0x310
bond_mii_monitor+0x709/0x9b0
process_one_work+0x221/0x5e0
worker_thread+0x4f/0x3b0
kthread+0x100/0x140
? process_one_work+0x5e0/0x5e0
? kthread_delayed_work_timer_fn+0x90/0x90
ret_from_fork+0x24/0x30
Signed-off-by: Dave Jones <davej@codemonkey.org.uk>
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c05c01a00755..77a3607a7099 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -977,6 +977,7 @@ static void bond_poll_controller(struct net_device *bond_dev)
if (bond_3ad_get_active_agg_info(bond, &ad_info))
return;
+ rcu_read_lock();
bond_for_each_slave_rcu(bond, slave, iter) {
if (!bond_slave_is_up(slave))
continue;
@@ -992,6 +993,7 @@ static void bond_poll_controller(struct net_device *bond_dev)
netpoll_poll_dev(slave->dev);
}
+ rcu_read_unlock();
}
static void bond_netpoll_cleanup(struct net_device *bond_dev)
^ permalink raw reply related
* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Edward Cree @ 2018-09-28 16:11 UTC (permalink / raw)
To: Andrew Lunn
Cc: Ariel Almog, linville, Linux Netdev List, ganeshgr,
jakub.kicinski, dustin, dirk.vandermerwe, shayag, ariela
In-Reply-To: <20180928153942.GM12979@lunn.ch>
On 28/09/18 16:39, Andrew Lunn wrote:
> I wonder how true that will be in 5 years time, about reading the
> manual? SFP sockets are starting to appear in consumer devices. There
> are some Marvell SoC reference boards with SFP and SFP+. Broadcom also
> have some boards with SFP. With time, SFP will move out of the data
> centre and comms rack and into more everyday systems. In such context,
> reading the manual becomes less likely. It would be nice to avoid a
> future inconsistent mess caused be this sentiment now.
>
> Andrew
I see where you're coming from, but if people start needing to manually
configure FEC on their consumer devices, possibly we have bigger
problems.
Ethtool FEC control is for those situations where autoneg, autodetect,
autoconfigure etc. don't work (e.g. owing to out-of-spec switches, or
just a user wanting to disable FEC to save a few hundred nanos). I
would hope that FEC won't show up in consumer gear until these kinds
of problems have settled down somewhat.
Perhaps we can add something to the man page saying that not only can
the semantics vary from NIC to NIC, but that the semantics for a given
NIC might change in the future? Then if in five years' time we know
what the Right Thing™ is, we can move everyone over to that (with
appropriately *loud* release-notes).
I think the alternative, of finding a set of semantics that fits
everyone's hardware and covers everyone's requirements, is likely to
be difficult (and probably require changing the ethtool API).
-Ed
^ permalink raw reply
* RE: [PATCH net] net/ncsi: Extend NC-SI Netlink interface to allow user space to send NC-SI command
From: Justin.Lee1 @ 2018-09-28 16:06 UTC (permalink / raw)
To: sam, joel
Cc: amithash, vijaykhemka, linux-aspeed, openbmc, sdasari, netdev,
christian
In-Reply-To: <6e0a46384c10d380c16b07c0bed3a4f30f53f0cc.camel@mendozajonas.com>
Hi Samuel,
Please see my comment below.
Thanks,
Justin
> On Thu, 2018-09-27 at 21:08 +0000, Justin.Lee1@Dell.com wrote:
> > The new command (NCSI_CMD_SEND_CMD) is added to allow user space application
> > to send NC-SI command to the network card.
> > Also, add a new attribute (NCSI_ATTR_DATA) for transferring request and response.
> >
> > The work flow is as below.
> >
> > Request:
> > User space application -> Netlink interface (msg)
> > -> new Netlink handler - ncsi_send_cmd_nl()
> > -> ncsi_xmit_cmd()
> > Response:
> > Response received - ncsi_rcv_rsp() -> internal response handler - ncsi_rsp_handler_xxx()
> > -> ncsi_rsp_handler_netlink()
> > -> ncsi_send_netlink_rsp ()
> > -> Netlink interface (msg)
> > -> user space application
> > Command timeout - ncsi_request_timeout() -> ncsi_send_netlink_timeout ()
> > -> Netlink interface (msg with zero data length)
> > -> user space application
> > Error:
> > Error detected -> ncsi_send_netlink_err () -> Netlink interface (err msg)
> > -> user space application
> >
> >
> > Signed-off-by: Justin Lee <justin.lee1@dell.com>
> >
>
> Hi Justin,
>
> Thanks for posting this on the list! The overall design looks good and so
> far looks like it should fit relatively well with the other OEM command
> patch. I'll try and run some OEM commands against my machine.
> Some comments below:
>
> >
> > ---
> > include/uapi/linux/ncsi.h | 3 +
> > net/ncsi/internal.h | 12 ++-
> > net/ncsi/ncsi-aen.c | 10 ++-
> > net/ncsi/ncsi-cmd.c | 106 ++++++++++++++++--------
> > net/ncsi/ncsi-manage.c | 74 ++++++++++++++---
> > net/ncsi/ncsi-netlink.c | 199 +++++++++++++++++++++++++++++++++++++++++++++-
> > net/ncsi/ncsi-netlink.h | 4 +
> > net/ncsi/ncsi-rsp.c | 70 ++++++++++++++--
> > 8 files changed, 420 insertions(+), 58 deletions(-)
> >
> > diff --git a/include/uapi/linux/ncsi.h b/include/uapi/linux/ncsi.h
> > index 4c292ec..4992bfc 100644
> > --- a/include/uapi/linux/ncsi.h
> > +++ b/include/uapi/linux/ncsi.h
> > @@ -30,6 +30,7 @@ enum ncsi_nl_commands {
> > NCSI_CMD_PKG_INFO,
> > NCSI_CMD_SET_INTERFACE,
> > NCSI_CMD_CLEAR_INTERFACE,
> > + NCSI_CMD_SEND_CMD,
> >
> > __NCSI_CMD_AFTER_LAST,
> > NCSI_CMD_MAX = __NCSI_CMD_AFTER_LAST - 1
> > @@ -43,6 +44,7 @@ enum ncsi_nl_commands {
> > * @NCSI_ATTR_PACKAGE_LIST: nested array of NCSI_PKG_ATTR attributes
> > * @NCSI_ATTR_PACKAGE_ID: package ID
> > * @NCSI_ATTR_CHANNEL_ID: channel ID
> > + * @NCSI_ATTR_DATA: command payload
> > * @NCSI_ATTR_MAX: highest attribute number
> > */
> > enum ncsi_nl_attrs {
> > @@ -51,6 +53,7 @@ enum ncsi_nl_attrs {
> > NCSI_ATTR_PACKAGE_LIST,
> > NCSI_ATTR_PACKAGE_ID,
> > NCSI_ATTR_CHANNEL_ID,
> > + NCSI_ATTR_DATA,
> >
> > __NCSI_ATTR_AFTER_LAST,
> > NCSI_ATTR_MAX = __NCSI_ATTR_AFTER_LAST - 1
> > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> > index 8055e39..20ce735 100644
> > --- a/net/ncsi/internal.h
> > +++ b/net/ncsi/internal.h
> > @@ -215,12 +215,17 @@ struct ncsi_request {
> > unsigned char id; /* Request ID - 0 to 255 */
> > bool used; /* Request that has been assigned */
> > unsigned int flags; /* NCSI request property */
> > -#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> > +#define NCSI_REQ_FLAG_EVENT_DRIVEN 1
> > +#define NCSI_REQ_FLAG_NETLINK_DRIVEN 2
> > struct ncsi_dev_priv *ndp; /* Associated NCSI device */
> > struct sk_buff *cmd; /* Associated NCSI command packet */
> > struct sk_buff *rsp; /* Associated NCSI response packet */
> > struct timer_list timer; /* Timer on waiting for response */
> > bool enabled; /* Time has been enabled or not */
> > +
> > + u32 snd_seq; /* netlink sending sequence number */
> > + u32 snd_portid; /* netlink portid of sender */
> > + struct nlmsghdr nlhdr; /* netlink message header */
> > };
> >
> > enum {
> > @@ -301,10 +306,13 @@ struct ncsi_cmd_arg {
> > unsigned short payload; /* Command packet payload length */
> > unsigned int req_flags; /* NCSI request properties */
> > union {
> > - unsigned char bytes[16]; /* Command packet specific data */
> > + unsigned char bytes[16]; /* Command packet specific data */
> > unsigned short words[8];
> > unsigned int dwords[4];
> > };
> > +
> > + unsigned char *data; /* Netlink data */
> > + struct genl_info *info; /* Netlink information */
> > };
> >
> > extern struct list_head ncsi_dev_list;
> > diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
> > index 25e483e..b5ec193 100644
> > --- a/net/ncsi/ncsi-aen.c
> > +++ b/net/ncsi/ncsi-aen.c
> > @@ -16,6 +16,7 @@
> > #include <net/ncsi.h>
> > #include <net/net_namespace.h>
> > #include <net/sock.h>
> > +#include <net/genetlink.h>
> >
> > #include "internal.h"
> > #include "ncsi-pkt.h"
> > @@ -73,8 +74,8 @@ static int ncsi_aen_handler_lsc(struct ncsi_dev_priv *ndp,
> > ncm->data[2] = data;
> > ncm->data[4] = ntohl(lsc->oem_status);
> >
> > - netdev_dbg(ndp->ndev.dev, "NCSI: LSC AEN - channel %u state %s\n",
> > - nc->id, data & 0x1 ? "up" : "down");
> > + netdev_dbg(ndp->ndev.dev, "NCSI: LSC AEN - pkg %u ch %u state %s\n",
> > + nc->package->id, nc->id, data & 0x1 ? "up" : "down");
>
> There's a few places where you've changed or added some debug statements;
> these are good but could probably be in a separate patch since not all of
> them are related to the OEM command handling.
>
Sure, I will remove those.
> >
> > chained = !list_empty(&nc->link);
> > state = nc->state;
> > @@ -148,9 +149,10 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
> > hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
> > ncm->data[3] = ntohl(hncdsc->status);
> > spin_unlock_irqrestore(&nc->lock, flags);
> > +
> > netdev_dbg(ndp->ndev.dev,
> > - "NCSI: host driver %srunning on channel %u\n",
> > - ncm->data[3] & 0x1 ? "" : "not ", nc->id);
> > + "NCSI: host driver %srunning on pkg %u ch %u\n",
> > + ncm->data[3] & 0x1 ? "" : "not ", nc->package->id, nc->id);
> >
> > return 0;
> > }
> > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> > index 7567ca63..b291297 100644
> > --- a/net/ncsi/ncsi-cmd.c
> > +++ b/net/ncsi/ncsi-cmd.c
> > @@ -17,6 +17,7 @@
> > #include <net/ncsi.h>
> > #include <net/net_namespace.h>
> > #include <net/sock.h>
> > +#include <net/genetlink.h>
> >
> > #include "internal.h"
> > #include "ncsi-pkt.h"
> > @@ -211,42 +212,75 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
> > return 0;
> > }
> >
> > +static int ncsi_cmd_handler_oem(struct sk_buff *skb,
> > + struct ncsi_cmd_arg *nca)
> > +{
> > + struct ncsi_cmd_pkt *cmd;
> > + unsigned char *dest, *source;
> > + unsigned short len;
> > +
> > + /* struct ncsi_cmd_pkt = minimum length
> > + * - frame checksum
> > + * - Ethernet header
> > + * = 64 - 4 - 14 = 46
> > + * minimum payload = 46 - ncsi header - ncsi checksum
> > + * = 46 - 16 - 4 = 26
> > + */
> > + len = nca->payload;
> > +
> > + /* minimum payload length is 26 bytes to meet minimum packet
> > + * length 64
> > + */
> > + if (len < 26)
> > + cmd = skb_put_zero(skb, sizeof(*cmd));
> > + else
> > + cmd = skb_put_zero(skb, len + sizeof(struct ncsi_pkt_hdr) + 4);
> > +
> > + dest = (unsigned char *)cmd + sizeof(struct ncsi_pkt_hdr);
> > + source = (unsigned char *)nca->data + sizeof(struct ncsi_pkt_hdr);
> > + memcpy(dest, source, len);
> > +
> > + ncsi_cmd_build_header(&cmd->cmd.common, nca);
> > +
> > + return 0;
> > +}
>
> This is quite similar to the other patch which is good, they shouldn't
> conflict much.
>
> > +
> > static struct ncsi_cmd_handler {
> > unsigned char type;
> > int payload;
> > int (*handler)(struct sk_buff *skb,
> > struct ncsi_cmd_arg *nca);
> > } ncsi_cmd_handlers[] = {
> > - { NCSI_PKT_CMD_CIS, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_SP, 4, ncsi_cmd_handler_sp },
> > - { NCSI_PKT_CMD_DP, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_EC, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_DC, 4, ncsi_cmd_handler_dc },
> > - { NCSI_PKT_CMD_RC, 4, ncsi_cmd_handler_rc },
> > - { NCSI_PKT_CMD_ECNT, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_DCNT, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae },
> > - { NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl },
> > - { NCSI_PKT_CMD_GLS, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_SVF, 8, ncsi_cmd_handler_svf },
> > - { NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev },
> > - { NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_SMA, 8, ncsi_cmd_handler_sma },
> > - { NCSI_PKT_CMD_EBF, 4, ncsi_cmd_handler_ebf },
> > - { NCSI_PKT_CMD_DBF, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_EGMF, 4, ncsi_cmd_handler_egmf },
> > - { NCSI_PKT_CMD_DGMF, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_SNFC, 4, ncsi_cmd_handler_snfc },
> > - { NCSI_PKT_CMD_GVI, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_GC, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_GP, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_GCPS, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
> > - { NCSI_PKT_CMD_OEM, 0, NULL },
> > - { NCSI_PKT_CMD_PLDM, 0, NULL },
> > - { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
> > + { NCSI_PKT_CMD_CIS, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_SP, 4, ncsi_cmd_handler_sp },
> > + { NCSI_PKT_CMD_DP, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_EC, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_DC, 4, ncsi_cmd_handler_dc },
> > + { NCSI_PKT_CMD_RC, 4, ncsi_cmd_handler_rc },
> > + { NCSI_PKT_CMD_ECNT, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_DCNT, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae },
> > + { NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl },
> > + { NCSI_PKT_CMD_GLS, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_SVF, 8, ncsi_cmd_handler_svf },
> > + { NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev },
> > + { NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_SMA, 8, ncsi_cmd_handler_sma },
> > + { NCSI_PKT_CMD_EBF, 4, ncsi_cmd_handler_ebf },
> > + { NCSI_PKT_CMD_DBF, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_EGMF, 4, ncsi_cmd_handler_egmf },
> > + { NCSI_PKT_CMD_DGMF, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_SNFC, 4, ncsi_cmd_handler_snfc },
> > + { NCSI_PKT_CMD_GVI, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_GC, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_GP, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_GCPS, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
> > + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
> > + { NCSI_PKT_CMD_PLDM, 0, NULL },
> > + { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
> > };
>
> You've changed the whitespace here which has made the diff unnecessarily
> large; please just change the single _OEM line and keep the whitespace
> the same.
>
I will remove the whitespace for alignment.
> >
> > static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg *nca)
> > @@ -317,11 +351,20 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
> > }
> >
> > /* Get packet payload length and allocate the request */
> > - nca->payload = nch->payload;
> > + if (nch->payload >= 0)
> > + nca->payload = nch->payload;
> > +
> > nr = ncsi_alloc_command(nca);
> > if (!nr)
> > return -ENOMEM;
> >
> > + /* track netlink information */
> > + if (nca->req_flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> > + nr->snd_seq = nca->info->snd_seq;
> > + nr->snd_portid = nca->info->snd_portid;
> > + nr->nlhdr = *nca->info->nlhdr;
> > + }
> > +
> > /* Prepare the packet */
> > nca->id = nr->id;
> > ret = nch->handler(nr->cmd, nca);
> > @@ -341,6 +384,7 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
> > * connection a 1 second delay should be sufficient.
> > */
> > nr->enabled = true;
> > +
> > mod_timer(&nr->timer, jiffies + 1 * HZ);
> >
> > /* Send NCSI packet */
> > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> > index 0912847..6629103 100644
> > --- a/net/ncsi/ncsi-manage.c
> > +++ b/net/ncsi/ncsi-manage.c
> > @@ -19,6 +19,7 @@
> > #include <net/addrconf.h>
> > #include <net/ipv6.h>
> > #include <net/if_inet6.h>
> > +#include <net/genetlink.h>
> >
> > #include "internal.h"
> > #include "ncsi-pkt.h"
> > @@ -110,8 +111,9 @@ static void ncsi_channel_monitor(struct timer_list *t)
> > case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
> > break;
> > default:
> > - netdev_err(ndp->ndev.dev, "NCSI Channel %d timed out!\n",
> > - nc->id);
> > + netdev_err(ndp->ndev.dev, "NCSI: pkg %u ch %u timed out!\n",
> > + np->id, nc->id);
> > +
> > if (!(ndp->flags & NCSI_DEV_HWA)) {
> > ncsi_report_link(ndp, true);
> > ndp->flags |= NCSI_DEV_RESHUFFLE;
> > @@ -143,6 +145,10 @@ void ncsi_start_channel_monitor(struct ncsi_channel *nc)
> > {
> > unsigned long flags;
> >
> > + netdev_dbg(nc->package->ndp->ndev.dev,
> > + "NCSI: %s pkg %u ch %u\n",
> > + __func__, nc->package->id, nc->id);
> > +
> > spin_lock_irqsave(&nc->lock, flags);
> > WARN_ON_ONCE(nc->monitor.enabled);
> > nc->monitor.enabled = true;
> > @@ -156,6 +162,10 @@ void ncsi_stop_channel_monitor(struct ncsi_channel *nc)
> > {
> > unsigned long flags;
> >
> > + netdev_dbg(nc->package->ndp->ndev.dev,
> > + "NCSI: %s pkg %u ch %u\n",
> > + __func__, nc->package->id, nc->id);
> > +
> > spin_lock_irqsave(&nc->lock, flags);
> > if (!nc->monitor.enabled) {
> > spin_unlock_irqrestore(&nc->lock, flags);
> > @@ -406,8 +416,13 @@ static void ncsi_request_timeout(struct timer_list *t)
> > {
> > struct ncsi_request *nr = from_timer(nr, t, timer);
> > struct ncsi_dev_priv *ndp = nr->ndp;
> > + struct ncsi_package *np;
> > + struct ncsi_channel *nc;
> > + struct ncsi_cmd_pkt *cmd;
> > unsigned long flags;
> >
> > + netdev_dbg(ndp->ndev.dev, "NCSI: %s\n", __func__);
> > +
> > /* If the request already had associated response,
> > * let the response handler to release it.
> > */
> > @@ -415,10 +430,23 @@ static void ncsi_request_timeout(struct timer_list *t)
> > nr->enabled = false;
> > if (nr->rsp || !nr->cmd) {
> > spin_unlock_irqrestore(&ndp->lock, flags);
> > +
> > + netdev_dbg(ndp->ndev.dev, "NCSI: %s - early return\n", __func__);
> > +
> > return;
> > }
> > spin_unlock_irqrestore(&ndp->lock, flags);
> >
> > + if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> > + if (nr->cmd) {
> > + /* Find the package */
> > + cmd = (struct ncsi_cmd_pkt *)skb_network_header(nr->cmd);
> > + ncsi_find_package_and_channel(ndp, cmd->cmd.common.channel,
> > + &np, &nc);
> > + ncsi_send_netlink_timeout(nr, np, nc);
> > + }
> > + }
> > +
> > /* Release the request */
> > ncsi_free_request(nr);
> > }
> > @@ -432,6 +460,10 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
> > unsigned long flags;
> > int ret;
> >
> > + netdev_dbg(ndp->ndev.dev,
> > + "NCSI: %s pkg %u ch %u state %04x\n",
> > + __func__, np->id, nc->id, nd->state);
> > +
> > nca.ndp = ndp;
> > nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
> > switch (nd->state) {
> > @@ -647,6 +679,10 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> > unsigned long flags;
> > int ret;
> >
> > + netdev_dbg(ndp->ndev.dev,
> > + "NCSI: %s pkg %u ch %u state %04x\n",
> > + __func__, np->id, nc->id, nd->state);
> > +
> > nca.ndp = ndp;
> > nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
> > switch (nd->state) {
> > @@ -788,8 +824,9 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> > }
> > break;
> > case ncsi_dev_state_config_done:
> > - netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
> > - nc->id);
> > + netdev_dbg(ndp->ndev.dev,
> > + "NCSI: pkg %u ch %u config done\n",
> > + nc->package->id, nc->id);
> > spin_lock_irqsave(&nc->lock, flags);
> > if (nc->reconfigure_needed) {
> > /* This channel's configuration has been updated
> > @@ -815,9 +852,10 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> > } else {
> > hot_nc = NULL;
> > nc->state = NCSI_CHANNEL_INACTIVE;
> > +
> > netdev_dbg(ndp->ndev.dev,
> > - "NCSI: channel %u link down after config\n",
> > - nc->id);
> > + "NCSI: pkg %u ch %u link down after config\n",
> > + nc->package->id, nc->id);
> > }
> > spin_unlock_irqrestore(&nc->lock, flags);
> >
> > @@ -853,6 +891,8 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
> > force_package = ndp->force_package;
> > spin_unlock_irqrestore(&ndp->lock, flags);
> >
> > + netdev_dbg(ndp->ndev.dev, "NCSI: %s\n", __func__);
> > +
> > /* Force a specific channel whether or not it has link if we have been
> > * configured to do so
> > */
> > @@ -861,8 +901,8 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
> > ncm = &found->modes[NCSI_MODE_LINK];
> > if (!(ncm->data[2] & 0x1))
> > netdev_info(ndp->ndev.dev,
> > - "NCSI: Channel %u forced, but it is link down\n",
> > - found->id);
> > + "NCSI: pkg %u ch %u forced, but it is link down\n",
> > + found->package->id, found->id);
> > goto out;
> > }
> >
> > @@ -914,6 +954,7 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
> > out:
> > spin_lock_irqsave(&ndp->lock, flags);
> > list_add_tail_rcu(&found->link, &ndp->channel_queue);
> > +
> > spin_unlock_irqrestore(&ndp->lock, flags);
> >
> > return ncsi_process_next_channel(ndp);
> > @@ -1154,6 +1195,8 @@ static void ncsi_dev_work(struct work_struct *work)
> > struct ncsi_dev_priv, work);
> > struct ncsi_dev *nd = &ndp->ndev;
> >
> > + netdev_dbg(ndp->ndev.dev, "NCSI: %s\n", __func__);
> > +
> > switch (nd->state & ncsi_dev_state_major) {
> > case ncsi_dev_state_probe:
> > ncsi_probe_channel(ndp);
> > @@ -1176,6 +1219,8 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
> > int old_state;
> > unsigned long flags;
> >
> > + netdev_dbg(ndp->ndev.dev, "NCSI: %s\n", __func__);
> > +
> > spin_lock_irqsave(&ndp->lock, flags);
> > nc = list_first_or_null_rcu(&ndp->channel_queue,
> > struct ncsi_channel, link);
> > @@ -1198,14 +1243,14 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
> > switch (old_state) {
> > case NCSI_CHANNEL_INACTIVE:
> > ndp->ndev.state = ncsi_dev_state_config;
> > - netdev_dbg(ndp->ndev.dev, "NCSI: configuring channel %u\n",
> > - nc->id);
> > + netdev_dbg(ndp->ndev.dev, "NCSI: configuring pkg %u ch %u\n",
> > + nc->package->id, nc->id);
> > ncsi_configure_channel(ndp);
> > break;
> > case NCSI_CHANNEL_ACTIVE:
> > ndp->ndev.state = ncsi_dev_state_suspend;
> > - netdev_dbg(ndp->ndev.dev, "NCSI: suspending channel %u\n",
> > - nc->id);
> > + netdev_dbg(ndp->ndev.dev, "NCSI: suspending pkg %u ch %u\n",
> > + nc->package->id, nc->id);
> > ncsi_suspend_channel(ndp);
> > break;
> > default:
> > @@ -1225,6 +1270,9 @@ int ncsi_process_next_channel(struct ncsi_dev_priv *ndp)
> > return ncsi_choose_active_channel(ndp);
> > }
> >
> > + netdev_dbg(ndp->ndev.dev,
> > + "NCSI: No more channels to process\n");
> > +
> > ncsi_report_link(ndp, false);
> > return -ENODEV;
> > }
> > @@ -1475,6 +1523,7 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
> > if (list_empty(&ncsi_dev_list))
> > register_inet6addr_notifier(&ncsi_inet6addr_notifier);
> > #endif
> > +
> > list_add_tail_rcu(&ndp->node, &ncsi_dev_list);
> > spin_unlock_irqrestore(&ncsi_dev_lock, flags);
> >
> > @@ -1564,6 +1613,7 @@ void ncsi_unregister_dev(struct ncsi_dev *nd)
> > if (list_empty(&ncsi_dev_list))
> > unregister_inet6addr_notifier(&ncsi_inet6addr_notifier);
> > #endif
> > +
> > spin_unlock_irqrestore(&ncsi_dev_lock, flags);
> >
> > ncsi_unregister_netlink(nd->dev);
> > diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
> > index 45f33d6..ab1a959 100644
> > --- a/net/ncsi/ncsi-netlink.c
> > +++ b/net/ncsi/ncsi-netlink.c
> > @@ -20,6 +20,7 @@
> > #include <uapi/linux/ncsi.h>
> >
> > #include "internal.h"
> > +#include "ncsi-pkt.h"
> > #include "ncsi-netlink.h"
> >
> > static struct genl_family ncsi_genl_family;
> > @@ -29,6 +30,7 @@ static const struct nla_policy ncsi_genl_policy[NCSI_ATTR_MAX + 1] = {
> > [NCSI_ATTR_PACKAGE_LIST] = { .type = NLA_NESTED },
> > [NCSI_ATTR_PACKAGE_ID] = { .type = NLA_U32 },
> > [NCSI_ATTR_CHANNEL_ID] = { .type = NLA_U32 },
> > + [NCSI_ATTR_DATA] = { .type = NLA_BINARY, .len = 2048 },
> > };
>
> Is there a particular reason for setting len to 2048, or just a decent
> buffer?
>
It is a decent buffer size as it can contain the whole network packet and also have the room to grow
if we want to extend the usage.
> >
> > static struct ncsi_dev_priv *ndp_from_ifindex(struct net *net, u32 ifindex)
> > @@ -240,7 +242,7 @@ static int ncsi_pkg_info_all_nl(struct sk_buff *skb,
> > return 0; /* done */
> >
> > hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
> > - &ncsi_genl_family, NLM_F_MULTI, NCSI_CMD_PKG_INFO);
> > + &ncsi_genl_family, NLM_F_MULTI, NCSI_CMD_PKG_INFO);
> > if (!hdr) {
> > rc = -EMSGSIZE;
> > goto err;
> > @@ -316,8 +318,8 @@ static int ncsi_set_interface_nl(struct sk_buff *msg, struct genl_info *info)
> > * package
> > */
> > spin_unlock_irqrestore(&ndp->lock, flags);
> > - netdev_info(ndp->ndev.dev, "NCSI: Channel %u does not exist!\n",
> > - channel_id);
> > + netdev_info(ndp->ndev.dev, "NCSI: pkg %u ch %u does not exist!\n",
> > + package_id, channel_id);
> > return -ERANGE;
> > }
> >
> > @@ -366,6 +368,191 @@ static int ncsi_clear_interface_nl(struct sk_buff *msg, struct genl_info *info)
> > return 0;
> > }
> >
> > +static int ncsi_send_cmd_nl(struct sk_buff *msg, struct genl_info *info)
> > +{
> > + struct ncsi_dev_priv *ndp;
> > +
> > + struct ncsi_cmd_arg nca;
> > + struct ncsi_pkt_hdr *hdr;
> > +
> > + u32 package_id, channel_id;
> > + unsigned char *data;
> > + void *head;
> > + int len, ret;
> > +
> > + if (!info || !info->attrs) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + if (!info->attrs[NCSI_ATTR_IFINDEX]) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + if (!info->attrs[NCSI_ATTR_PACKAGE_ID]) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + if (!info->attrs[NCSI_ATTR_CHANNEL_ID]) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + ndp = ndp_from_ifindex(get_net(sock_net(msg->sk)),
> > + nla_get_u32(info->attrs[NCSI_ATTR_IFINDEX]));
> > + if (!ndp) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + package_id = nla_get_u32(info->attrs[NCSI_ATTR_PACKAGE_ID]);
> > + channel_id = nla_get_u32(info->attrs[NCSI_ATTR_CHANNEL_ID]);
> > +
> > + if ((package_id & ~0x07) || (channel_id & ~0x1f)) {
> > + ret = -ERANGE;
> > + goto out_netlink;
> > + }
>
> This is correct but hard to read; we could instead just have
> if ((package_id >= 8) ...
> which is easier to interpret.
> (Probably we should also define the max packages/channels somewhere too)
>
I will add the definition for the max packages/channels and use it instead.
> > +
> > + len = nla_len(info->attrs[NCSI_ATTR_DATA]);
> > + if (len < sizeof(struct ncsi_pkt_hdr)) {
> > + netdev_info(ndp->ndev.dev, "NCSI: no OEM command to send %u\n",
> > + package_id);
>
> For statements like these follow the netdev format, eg:
> netdev_info(ndp->ndev.dev, "NCSI: no OEM command to send %u\n",
> package_id);
>
> > + ret = -EINVAL;
> > + goto out_netlink;
> > + } else {
> > + head = nla_data(info->attrs[NCSI_ATTR_DATA]);
> > + data = (unsigned char *)head;
> > + }
> > +
> > + hdr = (struct ncsi_pkt_hdr *)data;
> > +
> > + nca.ndp = ndp;
> > + nca.package = (unsigned char)package_id;
> > + nca.channel = (unsigned char)channel_id;
> > + nca.type = hdr->type;
> > + nca.req_flags = NCSI_REQ_FLAG_NETLINK_DRIVEN;
> > + nca.info = info;
> > + nca.payload = ntohs(hdr->length);
> > + nca.data = data;
> > +
> > + ret = ncsi_xmit_cmd(&nca);
> > +out_netlink:
> > + if (ret != 0) {
> > + netdev_err(ndp->ndev.dev, "Error %d sending OEM command\n", ret);
> > + ncsi_send_netlink_err(ndp->ndev.dev,
> > + info->snd_seq, info->snd_portid, info->nlhdr,
> > + ret);
> > + }
> > +out:
> > + return ret;
> > +}
> > +
> > +int ncsi_send_netlink_rsp(struct ncsi_request *nr, struct ncsi_package *np, struct ncsi_channel *nc)
> > +{
> > + struct sk_buff *skb;
> > + struct net *net;
> > + void *hdr;
> > + int rc;
> > +
> > + netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
> > +
> > + net = dev_net(nr->rsp->dev);
> > +
> > + skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
> > + &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
> > + if (!hdr) {
> > + kfree_skb(skb);
> > + return -EMSGSIZE;
> > + }
> > +
> > + nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->rsp->dev->ifindex);
> > + if (np)
> > + nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
> > + if (nc)
> > + nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
> > + else
> > + nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
> > +
> > + rc = nla_put(skb, NCSI_ATTR_DATA, nr->rsp->len, (void *)nr->rsp->data);
> > + if (rc)
> > + goto err;
> > +
> > + genlmsg_end(skb, hdr);
> > + return genlmsg_unicast(net, skb, nr->snd_portid);
> > +
> > +err:
> > + kfree_skb(skb);
> > + return rc;
> > +}
> > +
> > +int ncsi_send_netlink_timeout(struct ncsi_request *nr, struct ncsi_package *np, struct ncsi_channel *nc)
> > +{
> > + struct sk_buff *skb;
> > + struct net *net;
> > + void *hdr;
> > +
> > + netdev_dbg(nr->ndp->ndev.dev, "NCSI: %s\n", __func__);
> > +
> > + skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + hdr = genlmsg_put(skb, nr->snd_portid, nr->snd_seq,
> > + &ncsi_genl_family, 0, NCSI_CMD_SEND_CMD);
> > + if (!hdr) {
> > + kfree_skb(skb);
> > + return -EMSGSIZE;
> > + }
> > +
> > + net = dev_net(nr->cmd->dev);
> > +
> > + nla_put_u32(skb, NCSI_ATTR_IFINDEX, nr->cmd->dev->ifindex);
> > +
> > + if (np)
> > + nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID, np->id);
> > + else
> > + nla_put_u32(skb, NCSI_ATTR_PACKAGE_ID,
> > + NCSI_PACKAGE_INDEX((((struct ncsi_pkt_hdr *)nr->cmd->data)->channel)));
> > +
> > + if (nc)
> > + nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, nc->id);
> > + else
> > + nla_put_u32(skb, NCSI_ATTR_CHANNEL_ID, NCSI_RESERVED_CHANNEL);
> > +
> > + genlmsg_end(skb, hdr);
> > + return genlmsg_unicast(net, skb, nr->snd_portid);
>
> How does the receiver of this message know it represents a timeout? Just that
> it's missing the NCSI_ATTR_DATA attribute?
>
That is correct. The missing NCSI_ATTR_DATA attribute indicates that there is no data received.
> > +}
> > +
> > +int ncsi_send_netlink_err(struct net_device *dev, u32 snd_seq, u32 snd_portid, struct nlmsghdr *nlhdr, int err)
> > +{
> > + struct sk_buff *skb;
> > + struct nlmsghdr *nlh;
> > + struct nlmsgerr *nle;
> > + struct net *net;
> > +
> > + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + net = dev_net(dev);
> > +
> > + nlh = nlmsg_put(skb, snd_portid, snd_seq,
> > + NLMSG_ERROR, sizeof(*nle), 0);
> > + nle = (struct nlmsgerr *)nlmsg_data(nlh);
> > + nle->error = err;
> > + memcpy(&nle->msg, nlhdr, sizeof(*nlh));
> > +
> > + nlmsg_end(skb, nlh);
> > +
> > + return nlmsg_unicast(net->genl_sock, skb, snd_portid);
> > +}
> > +
> > static const struct genl_ops ncsi_ops[] = {
> > {
> > .cmd = NCSI_CMD_PKG_INFO,
> > @@ -386,6 +573,12 @@ static const struct genl_ops ncsi_ops[] = {
> > .doit = ncsi_clear_interface_nl,
> > .flags = GENL_ADMIN_PERM,
> > },
> > + {
> > + .cmd = NCSI_CMD_SEND_CMD,
> > + .policy = ncsi_genl_policy,
> > + .doit = ncsi_send_cmd_nl,
> > + .flags = GENL_ADMIN_PERM,
> > + },
> > };
> >
> > static struct genl_family ncsi_genl_family __ro_after_init = {
> > diff --git a/net/ncsi/ncsi-netlink.h b/net/ncsi/ncsi-netlink.h
> > index 91a5c25..dadaf32 100644
> > --- a/net/ncsi/ncsi-netlink.h
> > +++ b/net/ncsi/ncsi-netlink.h
> > @@ -14,6 +14,10 @@
> >
> > #include "internal.h"
> >
> > +int ncsi_send_netlink_rsp(struct ncsi_request *nr, struct ncsi_package *np, struct ncsi_channel *nc);
> > +int ncsi_send_netlink_timeout(struct ncsi_request *nr, struct ncsi_package *np, struct ncsi_channel *nc);
> > +int ncsi_send_netlink_err(struct net_device *dev, u32 snd_seq, u32 snd_portid, struct nlmsghdr *nlhdr, int err);
> > +
> > int ncsi_init_netlink(struct net_device *dev);
> > int ncsi_unregister_netlink(struct net_device *dev);
> >
> > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> > index 930c1d3..bdf9519 100644
> > --- a/net/ncsi/ncsi-rsp.c
> > +++ b/net/ncsi/ncsi-rsp.c
> > @@ -16,9 +16,11 @@
> > #include <net/ncsi.h>
> > #include <net/net_namespace.h>
> > #include <net/sock.h>
> > +#include <net/genetlink.h>
> >
> > #include "internal.h"
> > #include "ncsi-pkt.h"
> > +#include "ncsi-netlink.h"
> >
> > static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
> > unsigned short payload)
> > @@ -32,15 +34,22 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
> > * before calling this function.
> > */
> > h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
> > - if (h->common.revision != NCSI_PKT_REVISION)
> > +
> > + if (h->common.revision != NCSI_PKT_REVISION) {
> > + netdev_dbg(nr->ndp->ndev.dev, "NCSI: unsupported header revision\n");
> > return -EINVAL;
> > - if (ntohs(h->common.length) != payload)
> > + }
> > + if (ntohs(h->common.length) != payload) {
> > + netdev_dbg(nr->ndp->ndev.dev, "NCSI: payload length mismatched\n");
> > return -EINVAL;
> > + }
> >
> > /* Check on code and reason */
> > if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
> > - ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR)
> > - return -EINVAL;
> > + ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
> > + netdev_dbg(nr->ndp->ndev.dev, "NCSI: non zero response/reason code\n");
> > + return -EPERM;
> > + }
>
> Why the change to EPERM?
>
We need to return the response/reason code to the caller. If this function returns -EPERM,
ncsi_rsp_handler() will call ncsi_rsp_handler_netlink(nr) to send back the.
> >
> > /* Validate checksum, which might be zeroes if the
> > * sender doesn't support checksum according to NCSI
> > @@ -52,8 +61,11 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
> >
> > checksum = ncsi_calculate_checksum((unsigned char *)h,
> > sizeof(*h) + payload - 4);
> > - if (*pchecksum != htonl(checksum))
> > +
> > + if (*pchecksum != htonl(checksum)) {
> > + netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
> > return -EINVAL;
> > + }
> >
> > return 0;
> > }
> > @@ -900,6 +912,31 @@ static int ncsi_rsp_handler_gpuuid(struct ncsi_request *nr)
> > return 0;
> > }
> >
> > +static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
> > +{
> > + return 0;
> > +}
> > +
> > +static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
> > +{
> > + struct ncsi_rsp_pkt *rsp;
> > + struct ncsi_dev_priv *ndp = nr->ndp;
> > + struct ncsi_package *np;
> > + struct ncsi_channel *nc;
> > + int ret;
> > +
> > + /* Find the package */
> > + rsp = (struct ncsi_rsp_pkt *)skb_network_header(nr->rsp);
> > + ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
> > + &np, &nc);
> > + if (!np)
> > + return -ENODEV;
> > +
> > + ret = ncsi_send_netlink_rsp(nr, np, nc);
> > +
> > + return ret;
> > +}
> > +
> > static struct ncsi_rsp_handler {
> > unsigned char type;
> > int payload;
> > @@ -932,7 +969,7 @@ static struct ncsi_rsp_handler {
> > { NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns },
> > { NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts },
> > { NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps },
> > - { NCSI_PKT_RSP_OEM, 0, NULL },
> > + { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem },
> > { NCSI_PKT_RSP_PLDM, 0, NULL },
> > { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }
> > };
> > @@ -950,6 +987,7 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
> >
> > /* Find the NCSI device */
> > nd = ncsi_find_dev(dev);
> > +
> > ndp = nd ? TO_NCSI_DEV_PRIV(nd) : NULL;
>
> There's a few spots around where you've added or changed whitespace,
> please clean these bits up.
>
> > if (!ndp)
> > return -ENODEV;
> > @@ -1002,6 +1040,15 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
> > netdev_warn(ndp->ndev.dev,
> > "NCSI: 'bad' packet ignored for type 0x%x\n",
> > hdr->type);
> > +
> > + if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> > + if (ret == -EPERM)
> > + goto out_netlink;
> > + else
> > + ncsi_send_netlink_err(ndp->ndev.dev,
> > + nr->snd_seq, nr->snd_portid, &nr->nlhdr,
> > + ret);
> > + }
>
> More netdev formatting, multi-line statements should be like:
>
> ncsi_send_netlink_err(ndp->ndev.dev,
> nr->snd_seq, nr->snd_portid, &nr->nlhdr,
> ret);
>
> > goto out;
> > }
> >
> > @@ -1011,6 +1058,17 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
> > netdev_err(ndp->ndev.dev,
> > "NCSI: Handler for packet type 0x%x returned %d\n",
> > hdr->type, ret);
> > +
> > +out_netlink:
> > + if (nr->flags == NCSI_REQ_FLAG_NETLINK_DRIVEN) {
> > + ret = ncsi_rsp_handler_netlink(nr);
> > + if (ret) {
> > + netdev_err(ndp->ndev.dev,
> > + "NCSI: Netlink handler for packet type 0x%x returned %d\n",
> > + hdr->type, ret);
> > + }
> > + }
> > +
> > out:
> > ncsi_free_request(nr);
> > return ret;
^ permalink raw reply
* Re: [PATCH net-next v6 04/23] zinc: ChaCha20 x86_64 implementation
From: Ard Biesheuvel @ 2018-09-28 15:47 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Linux Kernel Mailing List, <netdev@vger.kernel.org>,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
Greg Kroah-Hartman, Samuel Neves, Andy Lutomirski,
Jean-Philippe Aumasson, Andy Polyakov, Thomas Gleixner,
Ingo Molnar, the arch/x86 maintainers
In-Reply-To: <20180925145622.29959-5-Jason@zx2c4.com>
On 25 September 2018 at 16:56, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> This provides SSSE3, AVX-2, AVX-512F, and AVX-512VL implementations for
> ChaCha20. The AVX-512F implementation is disabled on Skylake, due to
> throttling, and the VL ymm implementation is used instead. These come
> from Andy Polyakov's implementation, with the following modifications
> from Samuel Neves:
>
> - Some cosmetic changes, like renaming labels to .Lname, constants,
> and other Linux conventions.
>
> - CPU feature checking is done in C by the glue code, so that has been
> removed from the assembly.
>
> - Eliminate translating certain instructions, such as pshufb, palignr,
> vprotd, etc, to .byte directives. This is meant for compatibility
> with ancient toolchains, but presumably it is unnecessary here,
> since the build system already does checks on what GNU as can
> assemble.
>
> - When aligning the stack, the original code was saving %rsp to %r9.
> To keep objtool happy, we use instead the DRAP idiom to save %rsp
> to %r10:
>
> leaq 8(%rsp),%r10
> ... code here ...
> leaq -8(%r10),%rsp
>
> - The original code assumes the stack comes aligned to 16 bytes. This
> is not necessarily the case, and to avoid crashes,
> `andq $-alignment, %rsp` was added in the prolog of a few functions.
>
> - The original hardcodes returns as .byte 0xf3,0xc3, aka "rep ret".
> We replace this by "ret". "rep ret" was meant to help with AMD K8
> chips, cf. http://repzret.org/p/repzret. It makes no sense to
> continue to use this kludge for code that won't even run on ancient
> AMD chips.
>
> While this is CRYPTOGAMS code, the originating code for this happens to
> be the same as OpenSSL's commit cded951378069a478391843f5f8653c1eb5128da
>
I'd still prefer the kernel side changes to be presented as a separate
followup patch (preferably based on the .pl but I understand that is
more difficult with x86 than with ARM code)
> Cycle counts on a Core i7 6700HQ using the AVX-2 codepath:
>
> size old new
> ---- ---- ----
> 0 62 52
> 16 414 376
> 32 410 400
> 48 414 422
> 64 362 356
> 80 714 666
> 96 714 700
> 112 712 718
> 128 692 646
> 144 1042 674
> 160 1042 694
> 176 1042 726
> 192 1018 650
> 208 1366 686
> 224 1366 696
> 240 1366 722
> 256 640 656
> 272 988 1246
> 288 988 1276
> 304 992 1296
> 320 972 1222
> 336 1318 1256
> 352 1318 1276
> 368 1316 1294
> 384 1294 1218
> 400 1642 1258
> 416 1642 1282
> 432 1642 1302
> 448 1628 1224
> 464 1970 1258
> 480 1970 1280
> 496 1970 1300
> 512 656 676
> 528 1010 1290
> 544 1010 1306
> 560 1010 1332
> 576 986 1254
> 592 1340 1284
> 608 1334 1310
> 624 1340 1334
> 640 1314 1254
> 656 1664 1282
> 672 1674 1306
> 688 1662 1336
> 704 1638 1250
> 720 1992 1292
> 736 1994 1308
> 752 1988 1334
> 768 1252 1254
> 784 1596 1290
> 800 1596 1314
> 816 1596 1330
> 832 1576 1256
> 848 1922 1286
> 864 1922 1314
> 880 1926 1338
> 896 1898 1258
> 912 2248 1288
> 928 2248 1320
> 944 2248 1338
> 960 2226 1268
> 976 2574 1288
> 992 2576 1312
> 1008 2574 1340
>
> Cycle counts on a Xeon Gold 5120 using the AVX-512 codepath:
>
> size old new
> ---- ---- ----
> 0 64 54
> 16 386 372
> 32 388 396
> 48 388 420
> 64 366 350
> 80 708 666
> 96 708 692
> 112 706 736
> 128 692 648
> 144 1036 682
> 160 1036 708
> 176 1036 730
> 192 1016 658
> 208 1360 684
> 224 1362 708
> 240 1360 732
> 256 644 500
> 272 990 526
> 288 988 556
> 304 988 576
> 320 972 500
> 336 1314 532
> 352 1316 558
> 368 1318 578
> 384 1308 506
> 400 1644 532
> 416 1644 556
> 432 1644 594
> 448 1624 508
> 464 1970 534
> 480 1970 556
> 496 1968 582
> 512 660 624
> 528 1016 682
> 544 1016 702
> 560 1018 728
> 576 998 654
> 592 1344 680
> 608 1344 708
> 624 1344 730
> 640 1326 654
> 656 1670 686
> 672 1670 708
> 688 1670 732
> 704 1652 658
> 720 1998 682
> 736 1998 710
> 752 1996 734
> 768 1256 662
> 784 1606 688
> 800 1606 714
> 816 1606 736
> 832 1584 660
> 848 1948 688
> 864 1950 714
> 880 1948 736
> 896 1912 688
> 912 2258 718
> 928 2258 744
> 944 2256 768
> 960 2238 692
> 976 2584 718
> 992 2584 744
> 1008 2584 770
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Signed-off-by: Samuel Neves <sneves@dei.uc.pt>
Please drop this SOB line: SOB is not about (co-)authorship but about
who handled the patch on its way into mainline. You are sending the
patch so your SOB should come last.
> Co-developed-by: Samuel Neves <sneves@dei.uc.pt>
> Based-on-code-from: Andy Polyakov <appro@openssl.org>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
> Cc: Andy Polyakov <appro@openssl.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: x86@kernel.org
> ---
> lib/zinc/Makefile | 1 +
> lib/zinc/chacha20/chacha20-x86_64-glue.h | 105 +
> lib/zinc/chacha20/chacha20-x86_64.S | 2632 ++++++++++++++++++++++
> lib/zinc/chacha20/chacha20.c | 4 +-
> 4 files changed, 2741 insertions(+), 1 deletion(-)
> create mode 100644 lib/zinc/chacha20/chacha20-x86_64-glue.h
> create mode 100644 lib/zinc/chacha20/chacha20-x86_64.S
>
> diff --git a/lib/zinc/Makefile b/lib/zinc/Makefile
> index 3d80144d55a6..223a0816c918 100644
> --- a/lib/zinc/Makefile
> +++ b/lib/zinc/Makefile
> @@ -3,4 +3,5 @@ ccflags-y += -D'pr_fmt(fmt)="zinc: " fmt'
> ccflags-$(CONFIG_ZINC_DEBUG) += -DDEBUG
>
> zinc_chacha20-y := chacha20/chacha20.o
> +zinc_chacha20-$(CONFIG_ZINC_ARCH_X86_64) += chacha20/chacha20-x86_64.o
> obj-$(CONFIG_ZINC_CHACHA20) += zinc_chacha20.o
> diff --git a/lib/zinc/chacha20/chacha20-x86_64-glue.h b/lib/zinc/chacha20/chacha20-x86_64-glue.h
> new file mode 100644
> index 000000000000..9b47001661a6
> --- /dev/null
> +++ b/lib/zinc/chacha20/chacha20-x86_64-glue.h
> @@ -0,0 +1,105 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +/*
> + * Copyright (C) 2015-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
> + */
> +
> +#include <asm/fpu/api.h>
> +#include <asm/cpufeature.h>
> +#include <asm/processor.h>
> +#include <asm/intel-family.h>
> +
> +#ifdef CONFIG_AS_SSSE3
> +asmlinkage void hchacha20_ssse3(u32 *derived_key, const u8 *nonce,
> + const u8 *key);
> +asmlinkage void chacha20_ssse3(u8 *out, const u8 *in, const size_t len,
> + const u32 key[8], const u32 counter[4]);
> +#endif
> +#ifdef CONFIG_AS_AVX2
> +asmlinkage void chacha20_avx2(u8 *out, const u8 *in, const size_t len,
> + const u32 key[8], const u32 counter[4]);
> +#endif
> +#ifdef CONFIG_AS_AVX512
> +asmlinkage void chacha20_avx512(u8 *out, const u8 *in, const size_t len,
> + const u32 key[8], const u32 counter[4]);
> +asmlinkage void chacha20_avx512vl(u8 *out, const u8 *in, const size_t len,
> + const u32 key[8], const u32 counter[4]);
> +#endif
> +
You can drop the #ifdefs above ...
> +static bool chacha20_use_ssse3 __ro_after_init;
> +static bool chacha20_use_avx2 __ro_after_init;
> +static bool chacha20_use_avx512 __ro_after_init;
> +static bool chacha20_use_avx512vl __ro_after_init;
> +
> +static void __init chacha20_fpu_init(void)
> +{
> + chacha20_use_ssse3 = boot_cpu_has(X86_FEATURE_SSSE3);
> + chacha20_use_avx2 =
> + boot_cpu_has(X86_FEATURE_AVX) &&
> + boot_cpu_has(X86_FEATURE_AVX2) &&
> + cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM, NULL);
> + chacha20_use_avx512 =
> + boot_cpu_has(X86_FEATURE_AVX) &&
> + boot_cpu_has(X86_FEATURE_AVX2) &&
> + boot_cpu_has(X86_FEATURE_AVX512F) &&
> + cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM |
> + XFEATURE_MASK_AVX512, NULL) &&
> + /* Skylake downclocks unacceptably much when using zmm. */
> + boot_cpu_data.x86_model != INTEL_FAM6_SKYLAKE_X;
> + chacha20_use_avx512vl =
> + boot_cpu_has(X86_FEATURE_AVX) &&
> + boot_cpu_has(X86_FEATURE_AVX2) &&
> + boot_cpu_has(X86_FEATURE_AVX512F) &&
> + boot_cpu_has(X86_FEATURE_AVX512VL) &&
> + cpu_has_xfeatures(XFEATURE_MASK_SSE | XFEATURE_MASK_YMM |
> + XFEATURE_MASK_AVX512, NULL);
> +}
> +
> +static inline bool chacha20_arch(struct chacha20_ctx *state, u8 *dst,
> + const u8 *src, const size_t len,
> + simd_context_t *simd_context)
> +{
> + if (!chacha20_use_ssse3 || len <= CHACHA20_BLOCK_SIZE ||
> + !simd_use(simd_context))
> + return false;
> +
> +#ifdef CONFIG_AS_AVX512
... and use IS_ENABLED(CONFIG_AS_AVX512) here inside the if().
> + if (chacha20_use_avx512 && len >= CHACHA20_BLOCK_SIZE * 8) {
> + chacha20_avx512(dst, src, len, state->key, state->counter);
> + goto success;
> + }
> + if (chacha20_use_avx512vl && len >= CHACHA20_BLOCK_SIZE * 4) {
> + chacha20_avx512vl(dst, src, len, state->key, state->counter);
> + goto success;
> + }
> +#endif
> +#ifdef CONFIG_AS_AVX2
> + if (chacha20_use_avx2 && len >= CHACHA20_BLOCK_SIZE * 4) {
> + chacha20_avx2(dst, src, len, state->key, state->counter);
> + goto success;
> + }
> +#endif
> +#ifdef CONFIG_AS_SSSE3
> + if (chacha20_use_ssse3) {
> + chacha20_ssse3(dst, src, len, state->key, state->counter);
> + goto success;
> + }
> +#endif
> + return false;
> +success:
> + state->counter[0] += (len + 63) / 64;
> + return true;
> +}
> +
> +static inline bool hchacha20_arch(u32 derived_key[CHACHA20_KEY_WORDS],
> + const u8 nonce[HCHACHA20_NONCE_SIZE],
> + const u8 key[HCHACHA20_KEY_SIZE],
> + simd_context_t *simd_context)
> +{
> +#if defined(CONFIG_AS_SSSE3)
> + if (chacha20_use_ssse3 && simd_use(simd_context)) {
> + hchacha20_ssse3(derived_key, nonce, key);
> + return true;
> + }
> +#endif
> + return false;
> +}
...
> diff --git a/lib/zinc/chacha20/chacha20.c b/lib/zinc/chacha20/chacha20.c
> index c82d9fc71f21..4354b874a6a5 100644
> --- a/lib/zinc/chacha20/chacha20.c
> +++ b/lib/zinc/chacha20/chacha20.c
> @@ -14,7 +14,9 @@
> #include <linux/init.h>
> #include <crypto/algapi.h>
>
> -#ifndef HAVE_CHACHA20_ARCH_IMPLEMENTATION
As I mentioned in reply to the previous patch, please get rid of this
CPP symbol ^^^
> +#if defined(CONFIG_ZINC_ARCH_X86_64)
> +#include "chacha20-x86_64-glue.h"
> +#else
> void __init chacha20_fpu_init(void)
> {
> }
> --
> 2.19.0
>
^ permalink raw reply
* [PATCH RFC net-next 4/5] net/ipv4: Update inet_dump_ifaddr to support NLM_F_DUMP_PROPER_HDR
From: dsahern @ 2018-09-28 15:45 UTC (permalink / raw)
To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20180928154502.9983-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Update inet_dump_ifaddr to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ifaddrmsg struct as the header potentially followed by one or more
attributes. Any data passed in the header or as an attribute is taken as
a request to influence the data returned. Only values suppored by the
dump handler are allowed to be non-0 or set in the request. At the moment
only the IFA_TARGET_NETNSID attribute is supported. Follow on patches
will support for other fields (e.g., honor ifa_index and only return data
for the given device index).
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv4/devinet.c | 52 +++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 41 insertions(+), 11 deletions(-)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 44d931a3cd50..1e06a21cd8f4 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1661,15 +1661,15 @@ static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
{
+ struct netlink_ext_ack *extack = cb->extack;
+ const struct nlmsghdr *nlh = cb->nlh;
struct inet_fill_args fillargs = {
.portid = NETLINK_CB(cb->skb).portid,
- .seq = cb->nlh->nlmsg_seq,
+ .seq = nlh->nlmsg_seq,
.event = RTM_NEWADDR,
- .flags = NLM_F_MULTI,
.netnsid = -1,
};
struct net *net = sock_net(skb->sk);
- struct nlattr *tb[IFA_MAX+1];
struct net *tgt_net = net;
int h, s_h;
int idx, s_idx;
@@ -1683,15 +1683,45 @@ static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
s_idx = idx = cb->args[1];
s_ip_idx = ip_idx = cb->args[2];
- if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
- ifa_ipv4_policy, NULL) >= 0) {
- if (tb[IFA_TARGET_NETNSID]) {
- fillargs.netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
+ if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+ struct nlattr *tb[IFA_MAX+1];
+ struct ifaddrmsg *ifm;
+ int err, i;
+
+ if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
+ NL_SET_ERR_MSG(extack, "Invalid header");
+ return -EINVAL;
+ }
+
+ ifm = (struct ifaddrmsg *) nlmsg_data(cb->nlh);
+ if (ifm->ifa_prefixlen || ifm->ifa_flags || ifm->ifa_scope) {
+ NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+ return -EINVAL;
+ }
+ if (ifm->ifa_index) {
+ NL_SET_ERR_MSG(extack, "Filter by device index not supported");
+ return -EINVAL;
+ }
+ err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
+ ifa_ipv4_policy, NULL);
+ if (err < 0)
+ return err;
- tgt_net = rtnl_get_net_ns_capable(skb->sk,
- fillargs.netnsid);
- if (IS_ERR(tgt_net))
- return PTR_ERR(tgt_net);
+ for (i = 0; i < IFA_MAX; ++i) {
+ if (i == IFA_TARGET_NETNSID) {
+ fillargs.netnsid = nla_get_s32(tb[i]);
+
+ tgt_net = rtnl_get_net_ns_capable(skb->sk,
+ fillargs.netnsid);
+ if (IS_ERR(tgt_net))
+ return PTR_ERR(tgt_net);
+
+ fillargs.flags |= NLM_F_DUMP_FILTERED;
+ }
+ if (tb[i]) {
+ NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
+ return -EINVAL;
+ }
}
}
--
2.11.0
^ permalink raw reply related
* [PATCH RFC net-next 5/5] net/ipv6: Update inet6_dump_addr to support NLM_F_DUMP_PROPER_HDR
From: dsahern @ 2018-09-28 15:45 UTC (permalink / raw)
To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20180928154502.9983-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Update inet6_dump_addr to check for NLM_F_DUMP_PROPER_HDR in the netlink
message header. If the flag is set, the dump request is expected to have
an ifaddrmsg struct as the header potentially followed by one or more
attributes. Any data passed in the header or as an attribute is taken as
a request to influence the data returned. Only values suppored by the
dump handler are allowed to be non-0 or set in the request. At the moment
only the IFA_TARGET_NETNSID attribute is supported. Follow on patches
will support for other fields (e.g., honor ifa_index and only return data
for the given device index).
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/addrconf.c | 50 ++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 375ea9d9869b..888e5a4b8dd2 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5001,6 +5001,8 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
enum addr_type_t type)
{
+ struct netlink_ext_ack *extack = cb->extack;
+ const struct nlmsghdr *nlh = cb->nlh;
struct inet6_fill_args fillargs = {
.portid = NETLINK_CB(cb->skb).portid,
.seq = cb->nlh->nlmsg_seq,
@@ -5009,7 +5011,6 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
.type = type,
};
struct net *net = sock_net(skb->sk);
- struct nlattr *tb[IFA_MAX+1];
struct net *tgt_net = net;
int h, s_h;
int idx, ip_idx;
@@ -5022,17 +5023,46 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
s_idx = idx = cb->args[1];
s_ip_idx = ip_idx = cb->args[2];
- if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
- ifa_ipv6_policy, NULL) >= 0) {
- if (tb[IFA_TARGET_NETNSID]) {
- fillargs.netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
+ if (nlh->nlmsg_flags & NLM_F_DUMP_PROPER_HDR) {
+ struct nlattr *tb[IFA_MAX+1];
+ struct ifaddrmsg *ifm;
+ int err, i;
+
+ if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifm))) {
+ NL_SET_ERR_MSG(extack, "Invalid header");
+ return -EINVAL;
+ }
- tgt_net = rtnl_get_net_ns_capable(skb->sk,
- fillargs.netnsid);
- if (IS_ERR(tgt_net))
- return PTR_ERR(tgt_net);
+ ifm = (struct ifaddrmsg *)nlmsg_data(cb->nlh);
+ if (ifm->ifa_prefixlen || ifm->ifa_flags || ifm->ifa_scope) {
+ NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+ return -EINVAL;
+ }
+ if (ifm->ifa_index) {
+ NL_SET_ERR_MSG(extack, "Filter by device index not supported");
+ return -EINVAL;
+ }
- fillargs.flags |= NLM_F_DUMP_FILTERED;
+ err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
+ ifa_ipv6_policy, NULL);
+ if (err < 0)
+ return err;
+
+ for (i = 0; i < IFA_MAX; ++i) {
+ if (i == IFA_TARGET_NETNSID) {
+ fillargs.netnsid = nla_get_s32(tb[i]);
+
+ tgt_net = rtnl_get_net_ns_capable(skb->sk,
+ fillargs.netnsid);
+ if (IS_ERR(tgt_net))
+ return PTR_ERR(tgt_net);
+
+ fillargs.flags |= NLM_F_DUMP_FILTERED;
+ }
+ if (tb[i]) {
+ NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
+ return -EINVAL;
+ }
}
}
--
2.11.0
^ permalink raw reply related
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