* Re: linux-next: Tree for Sep 18 (kernel/bpf/syscall)
From: Willem de Bruijn @ 2018-09-18 16:28 UTC (permalink / raw)
To: Daniel Borkmann
Cc: rdunlap, Stephen Rothwell, Linux-Next Mailing List, LKML,
Network Development, Alexei Starovoitov, Petar Penkov,
Willem de Bruijn
In-Reply-To: <a1129d67-97e3-8f10-c938-0de72fee9377@iogearbox.net>
On Tue, Sep 18, 2018 at 11:44 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> [ Cc Petar and Willem ]
>
> On 09/18/2018 05:37 PM, Randy Dunlap wrote:
> > On 9/17/18 10:12 PM, Stephen Rothwell wrote:
> >> Hi all,
> >>
> >> Changes since 20180913:
> >
> > on i386 or x86_64: (in 6 of 20 randconfigs)
> >
> > kernel/bpf/syscall.o: In function `__x64_sys_bpf':
> > syscall.c:(.text+0x3278): undefined reference to `skb_flow_dissector_bpf_prog_attach'
> > syscall.c:(.text+0x3310): undefined reference to `skb_flow_dissector_bpf_prog_detach'
> > kernel/bpf/syscall.o:(.rodata+0x3f0): undefined reference to `flow_dissector_prog_ops'
> > kernel/bpf/verifier.o:(.rodata+0x250): undefined reference to `flow_dissector_verifier_ops'
> >
> > Full randconfig file is attached.
>
> Looks like we need a wrapper for these, config had:
>
> # CONFIG_NET is not set
Thanks for forwarding the report.
For the functions, I think we can use a similar static inline stub as
used e.g., for bpf_map_offload_map_alloc.
The _ops references are from a macro that includes linux/bpf_types.h,
so indeed a CONFIG looks needed.
I need to look it over before I send out, but tentative patch:
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 22083712dd18..f57fa5a6be8f 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -32,7 +32,9 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
#ifdef CONFIG_INET
BPF_PROG_TYPE(BPF_PROG_TYPE_SK_REUSEPORT, sk_reuseport)
#endif
+#ifdef CONFIG_NET
BPF_PROG_TYPE(BPF_PROG_TYPE_FLOW_DISSECTOR, flow_dissector)
+#endif
BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ce0e863f02a2..d26a21f10cec 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1194,10 +1194,23 @@ void skb_flow_dissector_init(struct
flow_dissector *flow_dissector,
const struct flow_dissector_key *key,
unsigned int key_count);
+#if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL)
int skb_flow_dissector_bpf_prog_attach(const union bpf_attr *attr,
struct bpf_prog *prog);
int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr);
+#else
+static inline int skb_flow_dissector_bpf_prog_attach(const union
bpf_attr *attr,
+ struct bpf_prog *prog)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int skb_flow_dissector_bpf_prog_detach(const union
bpf_attr *attr)
+{
+ return -EOPNOTSUPP;
+}
+#endif
^ permalink raw reply related
* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
From: Simon Horman @ 2018-09-18 10:58 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli
In-Reply-To: <20180917153811.GE5458@lunn.ch>
On Mon, Sep 17, 2018 at 05:38:11PM +0200, Andrew Lunn wrote:
> On Mon, Sep 17, 2018 at 05:13:07PM +0200, Simon Horman wrote:
> > On Wed, Sep 12, 2018 at 01:53:14AM +0200, Andrew Lunn wrote:
> > > Some MAC hardware cannot support a subset of link modes. e.g. often
> > > 1Gbps Full duplex is supported, but Half duplex is not. Add a helper
> > > to remove such a link mode.
> > >
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > > ---
> > > drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 6 +++---
> > > drivers/net/ethernet/cadence/macb_main.c | 5 ++---
> > > drivers/net/ethernet/freescale/fec_main.c | 3 ++-
> > > drivers/net/ethernet/microchip/lan743x_main.c | 2 +-
> > > drivers/net/ethernet/renesas/ravb_main.c | 3 ++-
> > > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 12 ++++++++----
> > > drivers/net/phy/phy_device.c | 18 ++++++++++++++++++
> > > drivers/net/usb/lan78xx.c | 2 +-
> > > include/linux/phy.h | 1 +
> > > 9 files changed, 38 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
> > > index 078a04dc1182..4831f9de5945 100644
> >
> > ...
> >
> > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> > > index aff5516b781e..fb2a1125780d 100644
> > > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > > @@ -1074,7 +1074,8 @@ static int ravb_phy_init(struct net_device *ndev)
> > > }
> > >
> > > /* 10BASE is not supported */
> > > - phydev->supported &= ~PHY_10BT_FEATURES;
> > > + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
> > > + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
> > >
> > > phy_attached_info(phydev);
> > >
> >
> > ...
> >
> > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > > index db1172db1e7c..e9ca83a438b0 100644
> > > --- a/drivers/net/phy/phy_device.c
> > > +++ b/drivers/net/phy/phy_device.c
> > > @@ -1765,6 +1765,24 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
> > > }
> > > EXPORT_SYMBOL(phy_set_max_speed);
> > >
> > > +/**
> > > + * phy_remove_link_mode - Remove a supported link mode
> > > + * @phydev: phy_device structure to remove link mode from
> > > + * @link_mode: Link mode to be removed
> > > + *
> > > + * Description: Some MACs don't support all link modes which the PHY
> > > + * does. e.g. a 1G MAC often does not support 1000Half. Add a helper
> > > + * to remove a link mode.
> > > + */
> > > +void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
> > > +{
> > > + WARN_ON(link_mode > 31);
> > > +
> > > + phydev->supported &= ~BIT(link_mode);
> > > + phydev->advertising = phydev->supported;
> > > +}
> > > +EXPORT_SYMBOL(phy_remove_link_mode);
> > > +
> > > static void of_set_phy_supported(struct phy_device *phydev)
> > > {
> > > struct device_node *node = phydev->mdio.dev.of_node;
> >
> > Hi Andrew,
> >
> > I believe that for the RAVB the overall effect of this change is that
> > 10-BaseT modes are no longer advertised (although both with and without
> > this patch they are not supported).
> >
> > Unfortunately on R-Car Gen3 M3-W (r8a7796) based Salvator-X board
> > I have observed that this results in the link no longer being negotiated
> > on one switch (the one I usually use) while it seemed fine on another.
>
> Hi Simon
>
> Thanks for testing this.
>
> Could you dump the PHY registers with and without this patch:
>
> $ mii-tool -vv eth0
>
> Once difference is that phy_remove_link_mode() does
> phydev->advertising = phydev->supported where as the old code does
> not. I though phylib would do this anyway, it does at some point in
> time, but i didn't check when. It could be you are actually
> advertising 10, even if you don't support it.
Hi Andrew,
here are the results. I ran them with the device connected to the switch
which doesn't allow successful link negotiation when this patch is present.
1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")
# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: no link
registers for MII PHY 0:
1140 7949 0022 1622 0d81 c1e1 000f 0000
0000 0300 0000 0000 0000 0000 0000 3000
0000 0000 0000 0078 7002 0000 0000 0200
0000 0000 0000 0528 0000 0000 0000 0000
product info: vendor 00:08:85, model 34 rev 2
basic mode: autonegotiation enabled
basic status: no link
capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
advertising: 100baseTx-FD 100baseTx-HD flow-control
link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
2. net-next with this patch reverted
# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: negotiated 100baseTx-FD, link ok
registers for MII PHY 0:
1140 796d 0022 1622 0181 c1e1 000f 0000
0000 0300 3800 0000 0000 0000 0000 3000
0000 0000 0000 0c7e 54fe 0000 0000 0200
0000 0000 0000 0500 0000 0000 0000 0000
product info: vendor 00:08:85, model 34 rev 2
basic mode: autonegotiation enabled
basic status: autonegotiation complete, link ok
capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
advertising: 100baseTx-FD 100baseTx-HD
link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
3. net-next with the following modification:
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index af64a9320fb0..f531b615d80b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1779,7 +1779,6 @@ void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
WARN_ON(link_mode > 31);
phydev->supported &= ~BIT(link_mode);
- phydev->advertising = phydev->supported;
}
EXPORT_SYMBOL(phy_remove_link_mode);
# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: negotiated 100baseTx-FD, link ok
registers for MII PHY 0:
1140 796d 0022 1622 0181 c1e1 000f 0000
0000 0300 3800 0000 0000 0000 0000 3000
0000 0000 0000 087e 44fe 0000 0000 0200
0000 0000 0000 0500 0000 0000 0000 0000
product info: vendor 00:08:85, model 34 rev 2
basic mode: autonegotiation enabled
basic status: autonegotiation complete, link ok
capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
advertising: 100baseTx-FD 100baseTx-HD
link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
^ permalink raw reply related
* Re: [PATCH net-next RFC 7/8] udp: gro behind static key
From: Steffen Klassert @ 2018-09-18 10:59 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, Paolo Abeni, David Miller, Willem de Bruijn
In-Reply-To: <CAF=yD-J=i3LtO17DQ0noEhnXxC=ebpEgBwLjAA=UM546to-BCQ@mail.gmail.com>
On Mon, Sep 17, 2018 at 10:19:22AM -0400, Willem de Bruijn wrote:
> On Mon, Sep 17, 2018 at 6:37 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > Maybe in case that forwarding is enabled on the receiving device,
> > inet_gro_receive() could do a route lookup and allow GRO if the
> > route lookup returned at forwarding route.
>
> That's a better solution, if the cost is acceptable. We do have to
> be careful against increasing per packet cycle cost in this path
> given that it's a possible vector for DoS attempts.
I played with this already and I have not seen any significant
overhead when doing a route lookup in GRO. We have to do a route
lookup anyway, so it should not matter too much if we do it here
or later in the IP layer.
>
> > For flows that are likely software segmented after that, it
> > would be worth to build packet chains insted of merging the
> > payload. Packets of the same flow could travel together, but
> > it would save the cost of the packet merging and segmenting.
>
> With software GSO that is faster, as it would have to allocate
> all the separate segment skbs in skb_segment later. Though
> there is some complexity if MTUs differ.
This can be handled the same way as it is done for TCP GSO
packets. If the size of the original packets is bigger than
the outgoing MTU, we either signal ICMP_FRAG_NEEDED to the
sender, or split the chain back into single packets and do
fragmentation on them.
>
> With hardware UDP GSO, having a single skb will be cheaper in
> the forwarding path. Using napi_gro_frags, device drivers really
> do only end up allocating one skb for the GSO packet.
Right, this is why it would be nice to have this
configurable.
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the net tree
From: David Miller @ 2018-09-18 16:32 UTC (permalink / raw)
To: daniel
Cc: vakul.garg, sfr, netdev, linux-next, linux-kernel, davejwatson,
doronrk
In-Reply-To: <5959dad0-dd02-1c3d-2487-13a69f8c507b@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Tue, 18 Sep 2018 11:53:17 +0200
> Ok, I think usually tests assert current kernel behavior to make sure any changes
> coming in don't accidentally break expectations from applications as opposed to
> future tests that still need fixing, but I guess I'm fine either way how to resolve
> the conflict; leaving it up to DaveM. Thanks for clarifying!
I'm doing the merge right now and will leave both tests in.
^ permalink raw reply
* Re: [PATCHv3] net: bnxt: Fix a uninitialized variable warning.
From: Michael Chan @ 2018-09-18 16:32 UTC (permalink / raw)
To: zhong jiang; +Cc: David Miller, Vasundhara Volam, Netdev, open list
In-Reply-To: <1537254944-41068-1-git-send-email-zhongjiang@huawei.com>
On Tue, Sep 18, 2018 at 12:15 AM, zhong jiang <zhongjiang@huawei.com> wrote:
> Fix the following compile warning:
>
> drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c:49:5: warning: ‘nvm_param.dir_type’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> if (nvm_param.dir_type == BNXT_NVM_PORT_CFG)
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Acked-by: Michael Chan <michael.chan@broadcom.com>
^ permalink raw reply
* OK
From: Ahmed Zama @ 2018-09-18 11:11 UTC (permalink / raw)
Good Day,
I am in need of a reliable and trust worthy person to work with me in
receiving huge sum of money into his or her account. I will give you
the full details immediately you respond to this email.
Ahmed Zama
+22675844869
^ permalink raw reply
* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-18 16:45 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andy Lutomirski, Andrew Lutomirski, David Miller, Andrew Lunn,
Eric Biggers, Greg Kroah-Hartman, LKML, Netdev, Samuel Neves,
Jean-Philippe Aumasson, Linux Crypto Mailing List
In-Reply-To: <CAKv+Gu-t5HPj+Y3-7JqkaKTdt-0ewOUobQaEuikpeOaX_SF+zg@mail.gmail.com>
Hi Ard,
On Tue, Sep 18, 2018 at 6:06 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> as long as we have data that supports the claim that it
> is actually faster on hardware people care about.
Seems reasonable. I'll next be turning my attention back to ARM
performance. I'll try to gather some numbers. Expect data at some
point next week.
Regards,
Jason
^ permalink raw reply
* Re: KMSAN: uninit-value in pppoe_rcv
From: Guillaume Nault @ 2018-09-18 16:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexander Potapenko, syzbot+f5f6080811c849739212, LKML, mostrows,
Networking, syzkaller-bugs
In-Reply-To: <7424e094-afda-084a-ad80-299f219ced92@gmail.com>
On Thu, Sep 13, 2018 at 06:57:54AM -0700, Eric Dumazet wrote:
>
>
> I guess the following patch would fix the issue
>
> (I will submit it more formally)
>
Hi Eric,
Do you still plan to submit this patch? Otherwise I can take care of it.
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index ce61231e96ea5fe27f512fbd0d80d4609997e508..333e967ed968ea3ff2dda25289f7f657263db2b9 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -423,6 +423,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
> struct pppoe_hdr *ph;
> struct pppox_sock *po;
> struct pppoe_net *pn;
> + __be16 sid;
> int len;
>
> skb = skb_share_check(skb, GFP_ATOMIC);
> @@ -434,6 +435,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>
> ph = pppoe_hdr(skb);
> len = ntohs(ph->length);
> + sid = ph->sid;
>
> skb_pull_rcsum(skb, sizeof(*ph));
> if (skb->len < len)
> @@ -447,7 +449,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
> /* Note that get_item does a sock_hold(), so sk_pppox(po)
> * is known to be safe.
> */
> - po = get_item(pn, ph->sid, eth_hdr(skb)->h_source, dev->ifindex);
> + po = get_item(pn, sid, eth_hdr(skb)->h_source, dev->ifindex);
> if (!po)
> goto drop;
>
>
>
^ permalink raw reply
* Re: KMSAN: uninit-value in pppoe_rcv
From: Eric Dumazet @ 2018-09-18 17:03 UTC (permalink / raw)
To: Guillaume Nault, Eric Dumazet
Cc: Alexander Potapenko, syzbot+f5f6080811c849739212, LKML, mostrows,
Networking, syzkaller-bugs
In-Reply-To: <20180918165254.GB1473@alphalink.fr>
On 09/18/2018 09:52 AM, Guillaume Nault wrote:
> On Thu, Sep 13, 2018 at 06:57:54AM -0700, Eric Dumazet wrote:
>>
>>
>> I guess the following patch would fix the issue
>>
>> (I will submit it more formally)
>>
> Hi Eric,
>
> Do you still plan to submit this patch? Otherwise I can take care of it.
>
Yes I will submit it. Thanks.
^ permalink raw reply
* Re: [PATCH net-next v5 19/20] security/keys: rewrite big_key crypto to use Zinc
From: Jason A. Donenfeld @ 2018-09-18 17:12 UTC (permalink / raw)
To: David Howells
Cc: linux-kernel, netdev, linux-crypto, davem, gregkh, Samuel Neves,
Andy Lutomirski, Jean-Philippe Aumasson, Eric Biggers
In-Reply-To: <2826.1537290107@warthog.procyon.org.uk>
Hi David,
On Tue, Sep 18, 2018 at 06:01:47PM +0100, David Howells wrote:
> Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> > A while back, I noticed that the crypto and crypto API usage in big_keys
> > were entirely broken in multiple ways, so I rewrote it. Now, I'm
> > rewriting it again, but this time using Zinc's ChaCha20Poly1305
> > function.
>
> warthog>git grep chacha20poly1305_decrypt net-next/master
> warthog1>
>
> Where do I find this?
>
> David
Previously in the patchset:
https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/linux.git/log/?h=jd/wireguard
https://lore.kernel.org/lkml/20180918161646.19105-1-Jason@zx2c4.com/
Regards,
Jason
^ permalink raw reply
* Announcing Netdev 0x13 conference
From: Jamal Hadi Salim @ 2018-09-18 12:11 UTC (permalink / raw)
To: people; +Cc: netdev@vger.kernel.org, linux-wireless, netfilter-devel, lwn
On behalf of the NetDev Society, this is a formal announcement that
Netdev 0x13 conference will be held in Prague, Czech Republic.
Tentative dates are March 20-22, 2019.
We had a very successful 0x12 meeting. The summaries are posted here:
https://lwn.net/Articles/763627/rss
For more details, all the slides, papers and videos are now up. Visit:
https://www.netdevconf.org/0x12/accepted-sessions.html
At 0x12 we tried a new format where the first day constitutes developer
working sessions via workshops and educational sessions via tutorials.
We have received feedback which we are in the process of evaluating
and we will be notifying you of any format changes. The window for
suggestions is still open - please send email to people@netdevconf.org.
We hope to announce the CFP much earlier than usual to give folks
ample time to submit.
For more frequent announcements join the people@netdevconf mailing list.
If tweeter is your thing we also do announce @netdev01
The official 0x13 website is at: https://www.netdevconf.org/0x13
cheers,
jamal
^ permalink raw reply
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Darren Hart @ 2018-09-18 17:51 UTC (permalink / raw)
To: Al Viro
Cc: Arnd Bergmann, linux-fsdevel, Greg Kroah-Hartman, David S. Miller,
devel, linux-kernel, qat-linux, linux-crypto, linux-media,
dri-devel, linaro-mm-sig, amd-gfx, linux-input, linux-iio,
linux-rdma, linux-nvdimm, linux-nvme, linux-pci,
platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi,
linux-usb, linux-fbdev, linuxppc-dev, linux-btrfs
In-Reply-To: <20180914205748.GC19965@ZenIV.linux.org.uk>
On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote:
> On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote:
>
> > Acked-by: Darren Hart (VMware) <dvhart@infradead.org>
> >
> > As for a longer term solution, would it be possible to init fops in such
> > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg
> > so we don't have to duplicate this boilerplate for every ioctl fops
> > structure?
>
> Bad idea, that... Because several years down the road somebody will add
> an ioctl that takes an unsigned int for argument. Without so much as looking
> at your magical mystery macro being used to initialize file_operations.
Fair, being explicit in the declaration as it is currently may be
preferable then.
--
Darren Hart
VMware Open Source Technology Center
^ permalink raw reply
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Jason Gunthorpe @ 2018-09-18 17:59 UTC (permalink / raw)
To: Darren Hart
Cc: Al Viro, Arnd Bergmann, linux-fsdevel, Greg Kroah-Hartman,
David S. Miller, devel, linux-kernel, qat-linux, linux-crypto,
linux-media, dri-devel, linaro-mm-sig, amd-gfx, linux-input,
linux-iio, linux-rdma, linux-nvdimm, linux-nvme, linux-pci,
platform-driver-x86, linux-remoteproc, sparclinux, linux-scsi,
linux-usb, linux-fbdev, linuxppc-d
In-Reply-To: <20180918175108.GF35251@wrath>
On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote:
> On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote:
> > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote:
> >
> > > Acked-by: Darren Hart (VMware) <dvhart@infradead.org>
> > >
> > > As for a longer term solution, would it be possible to init fops in such
> > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg
> > > so we don't have to duplicate this boilerplate for every ioctl fops
> > > structure?
> >
> > Bad idea, that... Because several years down the road somebody will add
> > an ioctl that takes an unsigned int for argument. Without so much as looking
> > at your magical mystery macro being used to initialize file_operations.
>
> Fair, being explicit in the declaration as it is currently may be
> preferable then.
It would be much cleaner and safer if you could arrange things to add
something like this to struct file_operations:
long (*ptr_ioctl) (struct file *, unsigned int, void __user *);
Where the core code automatically converts the unsigned long to the
void __user * as appropriate.
Then it just works right always and the compiler will help address
Al's concern down the road.
Cheers,
Jason
^ permalink raw reply
* [PATCH net-next] cxgb4vf: Add ethtool private flags for changing force_link_up
From: Arjun Vynipadath @ 2018-09-18 13:07 UTC (permalink / raw)
To: netdev, davem
Cc: dt, nirranjan, indranil, Arjun Vynipadath, Casey Leedom,
Ganesh Goudar
Forcing link up of virtual interfaces even when physical link is down
causes packet drops and ping failures during bonding failover. Hence
adding a ethtool private flag to toggle force_link_up whenever required.
Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
Signed-off-by: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4vf/adapter.h | 16 +++++++
.../net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 54 +++++++++++++++++++++-
2 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h b/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h
index 5883f09..444a5c8 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h
@@ -78,6 +78,18 @@ enum {
MAX_EGRQ = MAX_ETH_QSETS*2,
};
+enum {
+ PRIV_FLAG_PORT_FORCE_LINKUP_BIT,
+};
+
+#define PRIV_FLAG_PORT_FORCE_LINKUP \
+ BIT(PRIV_FLAG_PORT_FORCE_LINKUP_BIT)
+
+#define PRIV_FLAGS_ADAP 0x0
+#define DEFAULT_PRIV_FLAGS_ADAP 0x0
+#define PRIV_FLAGS_PORT PRIV_FLAG_PORT_FORCE_LINKUP
+#define DEFAULT_PRIV_FLAGS_PORT PRIV_FLAG_PORT_FORCE_LINKUP
+
/*
* Forward structure definition references.
*/
@@ -103,6 +115,7 @@ struct port_info {
u8 port_id; /* physical port ID */
u8 nqsets; /* # of "Queue Sets" */
u8 first_qset; /* index of first "Queue Set" */
+ u32 eth_flags; /* ethtool private flags */
struct link_config link_cfg; /* physical port configuration */
};
@@ -374,6 +387,9 @@ struct adapter {
unsigned long flags;
struct adapter_params params;
+ /* ethtool private flags */
+ u32 eth_flags;
+
/* queue and interrupt resources */
struct {
unsigned short vec;
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
index ff84791..ac10b5b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
@@ -138,6 +138,7 @@ static struct dentry *cxgb4vf_debugfs_root;
void t4vf_os_link_changed(struct adapter *adapter, int pidx, int link_ok)
{
struct net_device *dev = adapter->port[pidx];
+ const struct port_info *pi = netdev_priv(dev);
/*
* If the port is disabled or the current recorded "link up"
@@ -153,7 +154,9 @@ void t4vf_os_link_changed(struct adapter *adapter, int pidx, int link_ok)
if (link_ok) {
const char *s;
const char *fc;
- const struct port_info *pi = netdev_priv(dev);
+
+ if (!(pi->eth_flags & PRIV_FLAG_PORT_FORCE_LINKUP))
+ netif_carrier_on(dev);
switch (pi->link_cfg.speed) {
case 100:
@@ -200,6 +203,8 @@ void t4vf_os_link_changed(struct adapter *adapter, int pidx, int link_ok)
netdev_info(dev, "link up, %s, full-duplex, %s PAUSE\n", s, fc);
} else {
+ if (!(pi->eth_flags & PRIV_FLAG_PORT_FORCE_LINKUP))
+ netif_carrier_off(dev);
netdev_info(dev, "link down\n");
}
}
@@ -283,7 +288,7 @@ static int link_start(struct net_device *dev)
* no errors in enabling vi.
*/
- if (ret == 0)
+ if (ret == 0 && (pi->eth_flags & PRIV_FLAG_PORT_FORCE_LINKUP))
netif_carrier_on(dev);
return ret;
@@ -1502,6 +1507,10 @@ static int cxgb4vf_get_fecparam(struct net_device *dev,
return 0;
}
+static const char cxgb4vf_priv_flags_strings[][ETH_GSTRING_LEN] = {
+ [PRIV_FLAG_PORT_FORCE_LINKUP_BIT] = "port_force_linkup",
+};
+
/*
* Return our driver information.
*/
@@ -1524,6 +1533,7 @@ static void cxgb4vf_get_drvinfo(struct net_device *dev,
FW_HDR_FW_VER_MINOR_G(adapter->params.dev.tprev),
FW_HDR_FW_VER_MICRO_G(adapter->params.dev.tprev),
FW_HDR_FW_VER_BUILD_G(adapter->params.dev.tprev));
+ drvinfo->n_priv_flags = ARRAY_SIZE(cxgb4vf_priv_flags_strings);
}
/*
@@ -1728,6 +1738,8 @@ static int cxgb4vf_get_sset_count(struct net_device *dev, int sset)
switch (sset) {
case ETH_SS_STATS:
return ARRAY_SIZE(stats_strings);
+ case ETH_SS_PRIV_FLAGS:
+ return ARRAY_SIZE(cxgb4vf_priv_flags_strings);
default:
return -EOPNOTSUPP;
}
@@ -1745,6 +1757,10 @@ static void cxgb4vf_get_strings(struct net_device *dev,
case ETH_SS_STATS:
memcpy(data, stats_strings, sizeof(stats_strings));
break;
+ case ETH_SS_PRIV_FLAGS:
+ memcpy(data, cxgb4vf_priv_flags_strings,
+ sizeof(cxgb4vf_priv_flags_strings));
+ break;
}
}
@@ -1868,6 +1884,36 @@ static void cxgb4vf_get_wol(struct net_device *dev,
memset(&wol->sopass, 0, sizeof(wol->sopass));
}
+static u32 cxgb4vf_get_priv_flags(struct net_device *netdev)
+{
+ struct port_info *pi = netdev_priv(netdev);
+ struct adapter *adapter = pi->adapter;
+
+ return (adapter->eth_flags | pi->eth_flags);
+}
+
+/**
+ * set_flags - set/unset specified flags if passed in new_flags
+ * @cur_flags: pointer to current flags
+ * @new_flags: new incoming flags
+ * @flags: set of flags to set/unset
+ */
+static inline void set_flags(u32 *cur_flags, u32 new_flags, u32 flags)
+{
+ *cur_flags = (*cur_flags & ~flags) | (new_flags & flags);
+}
+
+static int cxgb4vf_set_priv_flags(struct net_device *netdev, u32 flags)
+{
+ struct port_info *pi = netdev_priv(netdev);
+ struct adapter *adapter = pi->adapter;
+
+ set_flags(&adapter->eth_flags, flags, PRIV_FLAGS_ADAP);
+ set_flags(&pi->eth_flags, flags, PRIV_FLAGS_PORT);
+
+ return 0;
+}
+
/*
* TCP Segmentation Offload flags which we support.
*/
@@ -1892,6 +1938,8 @@ static const struct ethtool_ops cxgb4vf_ethtool_ops = {
.get_regs_len = cxgb4vf_get_regs_len,
.get_regs = cxgb4vf_get_regs,
.get_wol = cxgb4vf_get_wol,
+ .get_priv_flags = cxgb4vf_get_priv_flags,
+ .set_priv_flags = cxgb4vf_set_priv_flags,
};
/*
@@ -3138,6 +3186,7 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
dev_info(&pdev->dev,
"Using assigned MAC ACL: %pM\n", mac);
}
+ pi->eth_flags = DEFAULT_PRIV_FLAGS_PORT;
}
/* See what interrupts we'll be using. If we've been configured to
@@ -3168,6 +3217,7 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
}
adapter->flags |= USING_MSI;
}
+ adapter->eth_flags = DEFAULT_PRIV_FLAGS_ADAP;
/* Now that we know how many "ports" we have and what interrupt
* mechanism we're going to use, we can configure our queue resources.
--
2.3.5
^ permalink raw reply related
* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Andrew Lunn @ 2018-09-18 12:35 UTC (permalink / raw)
To: Wang, Dongsheng
Cc: Florian Fainelli, timur@kernel.org, davem@davemloft.net,
Zheng, Joey, netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <71ba0057c5e547c382793be899e3268a@HXTBJIDCEMVIW02.hxtcorp.net>
> > If you want to describe the MDIO controller, then you embed a mdio
> > subnode into your Ethernet MAC node:
> >
> > emac0: ethernet@feb20000 {
> > mdio {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > phy0: ethernet-phy@0 {
> > reg = <0>;
> > };
> > };
> > };
> >
> > And then each Ethernet MAC controller refers to their appropriate PHY
> > device tree node using a phy-handle property to point to either their
> > own MDIO controller, or another MAC's MDIO controller.
> Sorry, I do not understand how phy-handle point to MDIO controller,
> because phy-handle is defined to point to a phy.
The MAC driver does not care what MDIO controller a PHY is on. All you
need to do to register the PHY is:
phy_node = of_parse_phandle(np, "phy-handle", 0);
phy_interface = of_get_phy_mode(np);
phydev = of_phy_connect(dev, phy_node,
&handle_link_change, 0,
phy_interface);
Andrew
^ permalink raw reply
* Re: [PATCH 1/2] netlink: add NLA_REJECT policy type
From: Johannes Berg @ 2018-09-18 12:39 UTC (permalink / raw)
To: Jamal Hadi Salim, Marcelo Ricardo Leitner, Michal Kubecek
Cc: linux-wireless, netdev, jbenc
In-Reply-To: <847cc635-cb90-821d-5824-07e7f941db75@mojatatu.com>
On Tue, 2018-09-18 at 08:34 -0400, Jamal Hadi Salim wrote:
> > > Maybe it would be better to have NLA_IGNORE instead? </idea>
> >
> > I don't think so, it doesn't give any feedback to the application author
> > that they're doing something wrong.
> >
>
> Maybe time to introduce kernel side access-control flags?
> Read/Write permissions for example. Attrs marked as read only
> (in the kernel) cannot be written to.
I dunno, that might work for ethtool, but I want to use it for something
that's not even an attribute you could think about writing to, but the
result of some operation you started.
What would the practical difference be though? Hopefully you wouldn't
have write-only attributes, and then NLA_REJECT is basically equivalent?
johannes
^ permalink raw reply
* Re: [PATCH 1/2] netlink: add NLA_REJECT policy type
From: Jamal Hadi Salim @ 2018-09-18 12:55 UTC (permalink / raw)
To: Johannes Berg, Marcelo Ricardo Leitner, Michal Kubecek
Cc: linux-wireless, netdev, jbenc
In-Reply-To: <1537274378.2957.23.camel@sipsolutions.net>
On 2018-09-18 8:39 a.m., Johannes Berg wrote:
> On Tue, 2018-09-18 at 08:34 -0400, Jamal Hadi Salim wrote:
>
>> Maybe time to introduce kernel side access-control flags?
>> Read/Write permissions for example. Attrs marked as read only
>> (in the kernel) cannot be written to.
>
> I dunno, that might work for ethtool, but I want to use it for something
> that's not even an attribute you could think about writing to, but the
> result of some operation you started.
>
Execute permission kind of thing? i.e if i understood you correctly
if acl is "rwx" then attribute can only be written to (or read from) if
the "thing executing" is complete
> What would the practical difference be though? Hopefully you wouldn't
> have write-only attributes, and then NLA_REJECT is basically equivalent?
>
If ACL says "-w-" then reading should get explicit permission denied
code possibly with an extack which is more descriptive that reading
is not allowed.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 1/2] netlink: add NLA_REJECT policy type
From: Johannes Berg @ 2018-09-18 12:57 UTC (permalink / raw)
To: Jamal Hadi Salim, Marcelo Ricardo Leitner, Michal Kubecek
Cc: linux-wireless, netdev, jbenc
In-Reply-To: <26dd9a66-9515-93aa-e21f-51c37db6be2c@mojatatu.com>
On Tue, 2018-09-18 at 08:55 -0400, Jamal Hadi Salim wrote:
> Execute permission kind of thing? i.e if i understood you correctly
> if acl is "rwx" then attribute can only be written to (or read from) if
> the "thing executing" is complete
But it's not an attribute that you're executing, it's some kind of
command, and then you get the return value of that command in that
attribute?
Say you want to scan for wifi networks - you trigger a scan, later you
get a notification giving you some data about the scan (let's say the
time it took) - there's no way you can set that time attribute.
(NB: it doesn't work this way, we don't have that attribute now, but I
didn't want to pick a more complicated example)
> > What would the practical difference be though? Hopefully you wouldn't
> > have write-only attributes, and then NLA_REJECT is basically equivalent?
> >
>
> If ACL says "-w-" then reading should get explicit permission denied
> code possibly with an extack which is more descriptive that reading
> is not allowed.
Perhaps. But NLA_REJECT comes with an extack string to tell you, so ...
I dunno. I think we already bloated the policies too much by including
the validation_data pointer, and would hate to add more to that :-)
johannes
^ permalink raw reply
* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
From: Andrew Lunn @ 2018-09-18 13:02 UTC (permalink / raw)
To: Simon Horman; +Cc: David Miller, netdev, Florian Fainelli
In-Reply-To: <20180918105817.z2o5yybcth7diqsu@verge.net.au>
> Hi Andrew,
Hi Simon
Thanks for the dumps
> 1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")
>
> basic status: no link
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> advertising: 100baseTx-FD 100baseTx-HD flow-control
> link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
>
> 2. net-next with this patch reverted
>
> basic status: autonegotiation complete, link ok
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> advertising: 100baseTx-FD 100baseTx-HD
> link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
So flow-control is not present here.
> basic status: autonegotiation complete, link ok
> capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
> advertising: 100baseTx-FD 100baseTx-HD
> link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
And here also.
Looking at the code, i see:
/* E-MAC init function */
static void ravb_emac_init(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
/* Receive frame limit set register */
ravb_write(ndev, ndev->mtu + ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN, RFLR);
/* EMAC Mode: PAUSE prohibition; Duplex; RX Checksum; TX; RX */
ravb_write(ndev, ECMR_ZPF | (priv->duplex ? ECMR_DM : 0) |
(ndev->features & NETIF_F_RXCSUM ? ECMR_RCSC : 0) |
ECMR_TE | ECMR_RE, ECMR);
Does this mean Pause is not supported in the hardware?
Thanks
Andrew
^ permalink raw reply
* [RFC 1/5] netlink: remove NLA_NESTED_COMPAT
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
This isn't used anywhere, so we might as well get rid of it.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/net/netlink.h | 2 --
lib/nlattr.c | 11 -----------
2 files changed, 13 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 318b1ded3833..b680fe365e91 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -172,7 +172,6 @@ enum {
NLA_FLAG,
NLA_MSECS,
NLA_NESTED,
- NLA_NESTED_COMPAT,
NLA_NUL_STRING,
NLA_BINARY,
NLA_S8,
@@ -203,7 +202,6 @@ enum {
* NLA_BINARY Maximum length of attribute payload
* NLA_NESTED Don't use `len' field -- length verification is
* done by checking len of nested header (or empty)
- * NLA_NESTED_COMPAT Minimum length of structure payload
* NLA_U8, NLA_U16,
* NLA_U32, NLA_U64,
* NLA_S8, NLA_S16,
diff --git a/lib/nlattr.c b/lib/nlattr.c
index bb6fe5ed4ecf..120ad569e13d 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -140,17 +140,6 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
return -ERANGE;
break;
- case NLA_NESTED_COMPAT:
- if (attrlen < pt->len)
- return -ERANGE;
- if (attrlen < NLA_ALIGN(pt->len))
- break;
- if (attrlen < NLA_ALIGN(pt->len) + NLA_HDRLEN)
- return -ERANGE;
- nla = nla_data(nla) + NLA_ALIGN(pt->len);
- if (attrlen < NLA_ALIGN(pt->len) + NLA_HDRLEN + nla_len(nla))
- return -ERANGE;
- break;
case NLA_NESTED:
/* a nested attributes is allowed to be empty; if its not,
* it must have a size of at least NLA_HDRLEN.
--
2.14.4
^ permalink raw reply related
* [RFC 2/5] netlink: set extack error message in nla_validate()
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
In-Reply-To: <20180918131212.20266-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
In nla_parse() we already set this, but it makes sense to
also do it in nla_validate() which already also sets the
bad attribute pointer.
CC: David Ahern <dsahern@gmail.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
lib/nlattr.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 120ad569e13d..efbd6c1aff29 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -181,9 +181,13 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
int rem;
nla_for_each_attr(nla, head, len, rem) {
- int err = validate_nla(nla, maxtype, policy, NULL);
+ static const char _msg[] = "Attribute failed policy validation";
+ const char *msg = _msg;
+ int err = validate_nla(nla, maxtype, policy, &msg);
if (err < 0) {
+ if (extack)
+ extack->_msg = msg;
NL_SET_BAD_ATTR(extack, nla);
return err;
}
--
2.14.4
^ permalink raw reply related
* [RFC 3/5] netlink: combine validate/parse functions
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
In-Reply-To: <20180918131212.20266-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
The parse function of course contains validate, but it's
implemented a second time, sharing just the validation
of a single attribute.
Introduce nla_validate_parse() that can be used for both
parsing/validation and only validation, to share code.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
lib/nlattr.c | 76 +++++++++++++++++++++++++++++++-----------------------------
1 file changed, 39 insertions(+), 37 deletions(-)
diff --git a/lib/nlattr.c b/lib/nlattr.c
index efbd6c1aff29..46a6d79cf2d1 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -159,6 +159,37 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
return 0;
}
+static int nla_validate_parse(const struct nlattr *head, int len, int maxtype,
+ const struct nla_policy *policy,
+ struct netlink_ext_ack *extack,
+ struct nlattr **tb)
+{
+ const struct nlattr *nla;
+ int rem;
+
+ nla_for_each_attr(nla, head, len, rem) {
+ static const char _msg[] = "Attribute failed policy validation";
+ const char *msg = _msg;
+ u16 type = nla_type(nla);
+
+ if (policy) {
+ int err = validate_nla(nla, maxtype, policy, &msg);
+
+ if (err < 0) {
+ if (extack)
+ extack->_msg = msg;
+ NL_SET_BAD_ATTR(extack, nla);
+ return err;
+ }
+ }
+
+ if (tb && type > 0 && type <= maxtype)
+ tb[type] = (struct nlattr *)nla;
+ }
+
+ return rem;
+}
+
/**
* nla_validate - Validate a stream of attributes
* @head: head of attribute stream
@@ -177,21 +208,12 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
const struct nla_policy *policy,
struct netlink_ext_ack *extack)
{
- const struct nlattr *nla;
int rem;
- nla_for_each_attr(nla, head, len, rem) {
- static const char _msg[] = "Attribute failed policy validation";
- const char *msg = _msg;
- int err = validate_nla(nla, maxtype, policy, &msg);
+ rem = nla_validate_parse(head, len, maxtype, policy, extack, NULL);
- if (err < 0) {
- if (extack)
- extack->_msg = msg;
- NL_SET_BAD_ATTR(extack, nla);
- return err;
- }
- }
+ if (rem < 0)
+ return rem;
return 0;
}
@@ -245,39 +267,19 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
int len, const struct nla_policy *policy,
struct netlink_ext_ack *extack)
{
- const struct nlattr *nla;
- int rem, err;
+ int rem;
memset(tb, 0, sizeof(struct nlattr *) * (maxtype + 1));
- nla_for_each_attr(nla, head, len, rem) {
- u16 type = nla_type(nla);
-
- if (type > 0 && type <= maxtype) {
- static const char _msg[] = "Attribute failed policy validation";
- const char *msg = _msg;
-
- if (policy) {
- err = validate_nla(nla, maxtype, policy, &msg);
- if (err < 0) {
- NL_SET_BAD_ATTR(extack, nla);
- if (extack)
- extack->_msg = msg;
- goto errout;
- }
- }
-
- tb[type] = (struct nlattr *)nla;
- }
- }
+ rem = nla_validate_parse(head, len, maxtype, policy, extack, tb);
+ if (rem < 0)
+ return rem;
if (unlikely(rem > 0))
pr_warn_ratelimited("netlink: %d bytes leftover after parsing attributes in process `%s'.\n",
rem, current->comm);
- err = 0;
-errout:
- return err;
+ return 0;
}
EXPORT_SYMBOL(nla_parse);
--
2.14.4
^ permalink raw reply related
* [RFC 4/5] netlink: prepare validate extack setting for recursion
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
In-Reply-To: <20180918131212.20266-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
In one of my previous patches in this area I introduced code
to pass out just the error message to store in the extack, for
use in NLA_REJECT.
Change this code now to set both the error message and the bad
attribute pointer, and carry around a boolean indicating that
the values have been set.
This will be used in the next patch to allow recursive validation
of nested policies, while preserving the innermost error message
rather than overwriting it with a generic out-level message.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
lib/nlattr.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 46a6d79cf2d1..fecc7b834706 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -70,7 +70,7 @@ static int validate_nla_bitfield32(const struct nlattr *nla,
static int validate_nla(const struct nlattr *nla, int maxtype,
const struct nla_policy *policy,
- const char **error_msg)
+ struct netlink_ext_ack *extack, bool *extack_set)
{
const struct nla_policy *pt;
int minlen = 0, attrlen = nla_len(nla), type = nla_type(nla);
@@ -95,8 +95,11 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
break;
case NLA_REJECT:
- if (pt->validation_data && error_msg)
- *error_msg = pt->validation_data;
+ if (pt->validation_data && extack && !*extack_set) {
+ *extack_set = true;
+ extack->_msg = pt->validation_data;
+ NL_SET_BAD_ATTR(extack, nla);
+ }
return -EINVAL;
case NLA_FLAG:
@@ -161,24 +164,25 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
static int nla_validate_parse(const struct nlattr *head, int len, int maxtype,
const struct nla_policy *policy,
- struct netlink_ext_ack *extack,
+ struct netlink_ext_ack *extack, bool *extack_set,
struct nlattr **tb)
{
const struct nlattr *nla;
int rem;
nla_for_each_attr(nla, head, len, rem) {
- static const char _msg[] = "Attribute failed policy validation";
- const char *msg = _msg;
u16 type = nla_type(nla);
if (policy) {
- int err = validate_nla(nla, maxtype, policy, &msg);
+ int err = validate_nla(nla, maxtype, policy,
+ extack, extack_set);
if (err < 0) {
- if (extack)
- extack->_msg = msg;
- NL_SET_BAD_ATTR(extack, nla);
+ if (!*extack_set) {
+ *extack_set = true;
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "Attribute failed policy validation");
+ }
return err;
}
}
@@ -208,9 +212,11 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
const struct nla_policy *policy,
struct netlink_ext_ack *extack)
{
+ bool extack_set = false;
int rem;
- rem = nla_validate_parse(head, len, maxtype, policy, extack, NULL);
+ rem = nla_validate_parse(head, len, maxtype, policy,
+ extack, &extack_set, NULL);
if (rem < 0)
return rem;
@@ -267,11 +273,13 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
int len, const struct nla_policy *policy,
struct netlink_ext_ack *extack)
{
+ bool extack_set = false;
int rem;
memset(tb, 0, sizeof(struct nlattr *) * (maxtype + 1));
- rem = nla_validate_parse(head, len, maxtype, policy, extack, tb);
+ rem = nla_validate_parse(head, len, maxtype, policy,
+ extack, &extack_set, tb);
if (rem < 0)
return rem;
--
2.14.4
^ permalink raw reply related
* [RFC 5/5] netlink: allow NLA_NESTED to specify nested policy to validate
From: Johannes Berg @ 2018-09-18 13:12 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
In-Reply-To: <20180918131212.20266-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
Now that we have a validation_data pointer, and the len field in
the policy is unused for NLA_NESTED, we can allow using them both
to have nested validation. This can be nice in code, although we
still have to use nla_parse_nested() or similar which would also
take a policy; however, it also serves as documentation in the
policy without requiring a look at the code.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/net/netlink.h | 10 ++++++++--
lib/nlattr.c | 17 +++++++++++++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index b680fe365e91..6efa25a004f5 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -200,8 +200,10 @@ enum {
* NLA_NUL_STRING Maximum length of string (excluding NUL)
* NLA_FLAG Unused
* NLA_BINARY Maximum length of attribute payload
- * NLA_NESTED Don't use `len' field -- length verification is
- * done by checking len of nested header (or empty)
+ * NLA_NESTED Length verification is done by checking len of
+ * nested header (or empty); len field is used if
+ * validation_data is also used, for the max attr
+ * number in the nested policy.
* NLA_U8, NLA_U16,
* NLA_U32, NLA_U64,
* NLA_S8, NLA_S16,
@@ -224,6 +226,10 @@ enum {
* NLA_REJECT This attribute is always rejected and validation data
* may point to a string to report as the error instead
* of the generic one in extended ACK.
+ * NLA_NESTED Points to a nested policy to validate, must also set
+ * `len' to the max attribute number.
+ * Note that nla_parse() will validate, but of course not
+ * parse, the nested sub-policies.
* All other Unused
*
* Example:
diff --git a/lib/nlattr.c b/lib/nlattr.c
index fecc7b834706..4c8c4fffb20d 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -68,6 +68,11 @@ static int validate_nla_bitfield32(const struct nlattr *nla,
return 0;
}
+static int nla_validate_parse(const struct nlattr *head, int len, int maxtype,
+ const struct nla_policy *policy,
+ struct netlink_ext_ack *extack, bool *extack_set,
+ struct nlattr **tb);
+
static int validate_nla(const struct nlattr *nla, int maxtype,
const struct nla_policy *policy,
struct netlink_ext_ack *extack, bool *extack_set)
@@ -149,6 +154,18 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
*/
if (attrlen == 0)
break;
+ if (attrlen < NLA_HDRLEN)
+ return -ERANGE;
+ if (pt->validation_data) {
+ int err;
+
+ err = nla_validate_parse(nla_data(nla), nla_len(nla),
+ pt->len, pt->validation_data,
+ extack, extack_set, NULL);
+ if (err < 0)
+ return err;
+ }
+ break;
default:
if (pt->len)
minlen = pt->len;
--
2.14.4
^ permalink raw reply related
* Re: [PATCH 1/2] netlink: add NLA_REJECT policy type
From: Jamal Hadi Salim @ 2018-09-18 13:12 UTC (permalink / raw)
To: Johannes Berg, Marcelo Ricardo Leitner, Michal Kubecek
Cc: linux-wireless, netdev, jbenc
In-Reply-To: <1537275441.2957.26.camel@sipsolutions.net>
On 2018-09-18 8:57 a.m., Johannes Berg wrote:
> On Tue, 2018-09-18 at 08:55 -0400, Jamal Hadi Salim wrote:
>
>> Execute permission kind of thing? i.e if i understood you correctly
>> if acl is "rwx" then attribute can only be written to (or read from) if
>> the "thing executing" is complete
>
> But it's not an attribute that you're executing, it's some kind of
> command, and then you get the return value of that command in that
> attribute?
>
> Say you want to scan for wifi networks - you trigger a scan, later you
> get a notification giving you some data about the scan (let's say the
> time it took) - there's no way you can set that time attribute.
>
Not very familiar with how wifi scan gets initiated. I am guessing
you issue some GET or SET to start a scan - and you get an async
response when it is complete (and it would include the time it took)?
Or maybe you get an immediate response and event notification later
and the time spent is in that notification?
I would still see that as a read-only attribute.
And the utility of "execute" bit is only in blocking another scan
from being initiated when one is in progress, if that is a desired
goal.
Note in most net devices stats can only be read but not written to
for example.
> (NB: it doesn't work this way, we don't have that attribute now, but I
> didn't want to pick a more complicated example)
>
>>> What would the practical difference be though? Hopefully you wouldn't
>>> have write-only attributes, and then NLA_REJECT is basically equivalent?
>>>
>>
>> If ACL says "-w-" then reading should get explicit permission denied
>> code possibly with an extack which is more descriptive that reading
>> is not allowed.
>
> Perhaps. But NLA_REJECT comes with an extack string to tell you, so ...
>
> I dunno. I think we already bloated the policies too much by including
> the validation_data pointer, and would hate to add more to that :-)
Your mileage may vary. NLA_REJECT may work acls offer more fine grained
controls.
cheers,
jamal
^ 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