* Re: [PATCH net 2/2] macsec: fix checksumming after decryption
From: Willem de Bruijn @ 2019-07-03 1:47 UTC (permalink / raw)
To: Andreas Steinmetz; +Cc: Willem de Bruijn, Network Development, Sabrina Dubroca
In-Reply-To: <a7c67e7e22103fc7cf02c520a8b42d9aa525700f.camel@domdv.de>
On Tue, Jul 2, 2019 at 3:48 PM Andreas Steinmetz <ast@domdv.de> wrote:
>
> On Tue, 2019-07-02 at 10:35 -0400, Willem de Bruijn wrote:
> > On Tue, Jul 2, 2019 at 12:25 AM Andreas Steinmetz <ast@domdv.de> wrote:
> > > On Sun, 2019-06-30 at 21:47 -0400, Willem de Bruijn wrote:
> > > > On Sun, Jun 30, 2019 at 4:48 PM Andreas Steinmetz <ast@domdv.de>
> > > > wrote:
> > > > > Fix checksumming after decryption.
> > > > >
> > > > > Signed-off-by: Andreas Steinmetz <ast@domdv.de>
> > > > >
> > > > > --- a/drivers/net/macsec.c 2019-06-30 22:14:10.250285314 +0200
> > > > > +++ b/drivers/net/macsec.c 2019-06-30 22:15:11.931230417 +0200
> > > > > @@ -869,6 +869,7 @@
> > > > >
> > > > > static void macsec_finalize_skb(struct sk_buff *skb, u8 icv_len,
> > > > > u8 hdr_len)
> > > > > {
> > > > > + skb->ip_summed = CHECKSUM_NONE;
> > > > > memmove(skb->data + hdr_len, skb->data, 2 * ETH_ALEN);
> > > > > skb_pull(skb, hdr_len);
> > > > > pskb_trim_unique(skb, skb->len - icv_len);
> > > >
> > > > Does this belong in macset_reset_skb?
> > >
> > > Putting this in macsec_reset_skb would then miss out the "nosci:" part
> > > of the RX path in macsec_handle_frame().
> >
> > It is called on each nskb before calling netif_rx.
> >
> > It indeed is not called when returning RX_HANDLER_PASS, but that is correct?
>
> This is correct. Packets passed on with RX_HANDLER_PASS are either not for this
> driver or the special case of being destined for a KaY and in this case being
> the MACsec ethernet protocol and thus not IP, so no checksumming.
So this could have been set in macsec_reset_skb, then? As all relevant cases
call it. Anyway, it's not very important and already merged.
^ permalink raw reply
* Validation of forward_delay seems wrong...
From: Andrew Lunn @ 2019-07-02 20:47 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: Roopa Prabhu, bridge, netdev
Hi Nikolay
The man page says that the bridge forward_delay is in units of
seconds, and should be between 2 and 30.
I've tested on a couple of different kernel versions, and this appears
to be not working correctly:
ip link set br0 type bridge forward_delay 2
RTNETLINK answers: Numerical result out of range
ip link set br0 type bridge forward_delay 199
RTNETLINK answers: Numerical result out of range
ip link set br0 type bridge forward_delay 200
#
ip link set br0 type bridge forward_delay 3000
#
ip link set br0 type bridge forward_delay 3001
RTNETLINK answers: Numerical result out of range
I've not checked what delay is actually being used here, but clearly
something is mixed up.
grep HZ .config
CONFIG_HZ_PERIODIC=y
# CONFIG_NO_HZ_IDLE is not set
# CONFIG_NO_HZ_FULL is not set
# CONFIG_NO_HZ is not set
CONFIG_HZ_FIXED=0
CONFIG_HZ_100=y
# CONFIG_HZ_200 is not set
# CONFIG_HZ_250 is not set
# CONFIG_HZ_300 is not set
# CONFIG_HZ_500 is not set
# CONFIG_HZ_1000 is not set
CONFIG_HZ=100
Thanks
Andrew
^ permalink raw reply
* Re: Validation of forward_delay seems wrong...
From: Andrew Lunn @ 2019-07-02 21:35 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: Roopa Prabhu, bridge, netdev
In-Reply-To: <55f24bfb-4239-dda8-24f8-26b6b2fa9f9e@cumulusnetworks.com>
> Hi Andrew,
> The man page is wrong, these have been in USER_HZ scaled clock_t format from the beginning.
> TBH a lot of the time/delay bridge config options are messed up like that.
Hi Nikola
Yes, that is a mess.
arch/alpha/include/asm/param.h:# define USER_HZ 1024
arch/ia64/include/asm/param.h:# define USER_HZ HZ
include/asm-generic/param.h:# define USER_HZ 100
And ia64 does
# define HZ CONFIG_HZ
So it seems pretty hard for user space to get this right in a generic
fashion.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v6 05/15] ethtool: helper functions for netlink interface
From: Jakub Kicinski @ 2019-07-03 1:37 UTC (permalink / raw)
To: Michal Kubecek
Cc: David Miller, netdev, Jiri Pirko, Andrew Lunn, Florian Fainelli,
John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <44957b13e8edbced71aca893908d184eb9e57341.1562067622.git.mkubecek@suse.cz>
On Tue, 2 Jul 2019 13:50:04 +0200 (CEST), Michal Kubecek wrote:
> Add common request/reply header definition and helpers to parse request
> header and fill reply header. Provide ethnl_update_* helpers to update
> structure members from request attributes (to be used for *_SET requests).
>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
> index 3c98b41f04e5..e13f29bbd625 100644
> --- a/net/ethtool/netlink.c
> +++ b/net/ethtool/netlink.c
> @@ -1,8 +1,181 @@
> // SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
>
> +#include <net/sock.h>
> #include <linux/ethtool_netlink.h>
> #include "netlink.h"
>
> +static struct genl_family ethtool_genl_family;
> +
> +static const struct nla_policy dflt_header_policy[ETHTOOL_A_HEADER_MAX + 1] = {
> + [ETHTOOL_A_HEADER_UNSPEC] = { .type = NLA_REJECT },
I think we want strict checking on all new netlink interfaces, and
unfortunately that feature is opt-in.. so you need to add:
.strict_start_type = ETHTOOL_A_HEADER_UNSPEC + 1
To the first attr.
> + [ETHTOOL_A_HEADER_DEV_INDEX] = { .type = NLA_U32 },
> + [ETHTOOL_A_HEADER_DEV_NAME] = { .type = NLA_NUL_STRING,
> + .len = IFNAMSIZ - 1 },
> + [ETHTOOL_A_HEADER_INFOMASK] = { .type = NLA_U32 },
> + [ETHTOOL_A_HEADER_GFLAGS] = { .type = NLA_U32 },
> + [ETHTOOL_A_HEADER_RFLAGS] = { .type = NLA_U32 },
> +};
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Kees Cook @ 2019-07-02 18:24 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Song Liu, linux-security@vger.kernel.org, Networking, bpf,
Alexei Starovoitov, Daniel Borkmann, Kernel Team, Lorenz Bauer,
Jann Horn, Greg KH, Linux API
In-Reply-To: <CALCETrWBWbNFJvsTCeUchu3BZJ3SH3dvtXLUB2EhnPrzFfsLNA@mail.gmail.com>
On Mon, Jul 01, 2019 at 06:59:13PM -0700, Andy Lutomirski wrote:
> I think I'm understanding your motivation. You're not trying to make
> bpf() generically usable without privilege -- you're trying to create
> a way to allow certain users to access dangerous bpf functionality
> within some limits.
>
> That's a perfectly fine goal, but I think you're reinventing the
> wheel, and the wheel you're reinventing is quite complicated and
> already exists. I think you should teach bpftool to be secure when
> installed setuid root or with fscaps enabled and put your policy in
> bpftool. If you want to harden this a little bit, it would seem
> entirely reasonable to add a new CAP_BPF_ADMIN and change some, but
> not all, of the capable() checks to check CAP_BPF_ADMIN instead of the
> capabilities that they currently check.
If finer grained controls are wanted, it does seem like the /dev/bpf
path makes the most sense. open, request abilities, use fd. The open can
be mediated by DAC and LSM. The request can be mediated by LSM. This
provides a way to add policy at the LSM level and at the tool level.
(i.e. For tool-level controls: leave LSM wide open, make /dev/bpf owned
by "bpfadmin" and bpftool becomes setuid "bpfadmin". For fine-grained
controls, leave /dev/bpf wide open and add policy to SELinux, etc.)
With only a new CAP, you don't get the fine-grained controls. (The
"request abilities" part is the key there.)
--
Kees Cook
^ permalink raw reply
* Re: [PATCH net-next 3/7] net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after posting IB_WR_LOCAL_INV
From: Gerd Rausch @ 2019-07-02 21:05 UTC (permalink / raw)
To: santosh.shilimkar, netdev; +Cc: David Miller
In-Reply-To: <d7ab5505-92e5-888c-a230-77bce3540261@oracle.com>
On 02/07/2019 09.49, santosh.shilimkar@oracle.com wrote:
> On 7/1/19 10:11 PM, Gerd Rausch wrote:
>> For the registration work-requests there is a benefit to wait a short
>> amount of time only (the trade-off described in patch #1 of this series).
>>
> Actually we should just switch this code to what Avinash has
> finally made in downstream code. That keeps the RDS_GET_MR
> semantics and makes sure MR is really valid before handing over
> the key to userland. There is no need for any timeout
> for registration case.
>
What do you call "RDS_GET_MR" semantics?
The purpose of waiting for a IB_WR_REG_MR request to complete
(inside rds_ib_post_reg_frmr) is in fact to make sure
the memory region is valid.
Regardless of this being true after a specific time-out,
or an infinite timeout.
For the non-infinite time-out case, there is a check if the request
was handled by the firmware.
And if a time-out occurred and the firmware didn't handle the request,
function "rds_ib_post_reg_frmr" will return -EBUSY.
>> Actually, no:
>> Socket option RDS_GET_MR wasn't even in the code-path of the
>> tests I performed:
>>
>> It were there RDS_CMSG_RDMA_MAP / RDS_CMSG_RDMA_DEST control
>> messages that ended up calling '__rds_rdma_map".
>>
> What option did you use ? Default option with rds-stress is
> RDS_GET_MR and hence the question.
>
Not true!:
Socket option RDS_GET_MR is only used by "rds-stress"
if it is invoked with "--rdma-use-get-mr <non-zero-value>".
Verify this with the following patch applied to rds-tools-2.0.7-1.18:
--- a/rds-stress.c
+++ b/rds-stress.c
@@ -705,6 +705,7 @@ static uint64_t get_rdma_key(int fd, uint64_t addr, uint32_t size)
if (opt.rdma_use_once)
mr_args.flags |= RDS_RDMA_USE_ONCE;
+ abort();
if (setsockopt(fd, sol, RDS_GET_MR, &mr_args, sizeof(mr_args)))
die_errno("setsockopt(RDS_GET_MR) failed (%u allocated)", mrs_allocated);
And why is socket option RDS_GET_MR a subject of this discussion?
The proposed patch suggests to wait for a firmware response in handling
_all_ cases that end up in "rds_ib_post_reg_frmr", issuing a "IB_WR_REG_MR" request.
It doesn't matter where they came from:
Whether they came from an RDS_GET_MR socket option, or a RDS_CMSG_RDMA_MAP:
They all go through "__rds_rdma_map" and end up calling "rds_ib_get_mr".
How is socket option RDS_GET_MR special with regards to this proposed fix?
>> I don't understand, please elaborate:
>> a) Are you saying this issue should not be fixed?
>> b) Or are you suggesting to replace this fix with a different fix?
>> If it's the later, please point out what you have in mind.
>> c) ???
>>
> All am saying is the code got changed for good reason and that changed
> code makes some of these race conditions possibly not applicable.
I don't understand this. Please elaborate.
> So instead of these timeout fixes, am suggesting to use that
> code as fix. At least test it with those changes and see whats
> the behavior.
>
Are you suggesting to
a) Not fix this bug right now and wait until some later point in time
b) Use a different fix. If you've got a different fix, please share.
And besides these options, is there anything wrong with this fix
(other than the discussion of what the timeout value ought to be,
which we can address)?
Thanks,
Gerd
^ permalink raw reply
* Re: [PATCH net 0/6] gtp: fix several bugs
From: Taehee Yoo @ 2019-07-03 1:31 UTC (permalink / raw)
To: Harald Welte
Cc: David Miller, Pablo Neira Ayuso, Pau Espin, osmocom-net-gprs,
Netdev
In-Reply-To: <20190703010856.GA11901@nataraja>
Hi Harald,
On Wed, 3 Jul 2019 at 09:10, Harald Welte <laforge@gnumonks.org> wrote:
>
> Hi Taehee,
>
> On Wed, Jul 03, 2019 at 12:20:34AM +0900, Taehee Yoo wrote:
> > This patch series fixes several bugs in the gtp module.
>
> thanks a lot for your patches, they are much appreciated.
>
> They look valid to me after a brief initial review.
>
> However, I'm currently on holidays and don't have the ability to test
> any patches until my return on July 17. Maybe Pablo and/or Pau can have
> a look meanwhile? Thanks in advance.
>
Thank you for letting me know.
Thanks a lot!
> Regards,
> Harald
> --
> - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
> ============================================================================
> "Privacy in residential applications is a desirable marketing option."
> (ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next 3/7] net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after posting IB_WR_LOCAL_INV
From: Gerd Rausch @ 2019-07-02 22:12 UTC (permalink / raw)
To: santosh.shilimkar, netdev; +Cc: David Miller
In-Reply-To: <1759bca6-4511-6cd9-ab5d-8c9c30e5db67@oracle.com>
On 02/07/2019 14.18, santosh.shilimkar@oracle.com wrote:
> On 7/2/19 2:05 PM, Gerd Rausch wrote:
>> What do you call "RDS_GET_MR" semantics?
>>
> Its a blocking socket call. Meaning after this call return to the
> user, the key must be valid. With async registration that can't be
> guaranteed.
>
If the "IB_WR_REG_MR" operation does not complete successfully within
the given (to-be-discussed?) timeout, "rds_ib_post_reg_frmr" will return
"-EBUSY".
And that should propagate up the entire stack and make its way into
"setsockopt" returning "-1" with "errno == EBUSY".
Do you see a problem with this approach?
Did you observe a situation where this did not work?
Are you saying that no timeout, no matter how large, is large enough?
If that's the case, we can consider turning the "wait_event_timeout"
into a "wait_event".
>> Are you suggesting to
>> a) Not fix this bug right now and wait until some later point in time
> When did I say that ? I said have you explored alternate approach to
> fix the issue and if not could you try it out.
>
Why explore an alternate approach?
Do you see a problem with the proposed patch (other than the choice of timeout)?
>> b) Use a different fix. If you've got a different fix, please share.
>>
> I don't but its a review of the fix and possible alternate needs to
> be discussed. It is not like take my fix or provide an alternate fix.
>
As it is, the upstream implementation of RDS does not work.
IMO, it is desirable to make it work.
If there are future and better implementation of existing functionality
that is fine.
That should not preclude us from fixing what is broken as soon as we can.
>> And besides these options, is there anything wrong with this fix
>> (other than the discussion of what the timeout value ought to be,
>> which we can address)?
>>
> That timeout is a problem because it doesn't guarantee the failure
> of operation since its an asyn operation for registration.
The fact that the work-request is asynchroneous is precisely what
makes it necessary to wait for the completion before moving on.
That is the proposed change of waiting for the completion
(or a time-out to put an upper bound to the wait)
does.
Replace the "wait_event_timeout" mentally with a "wait_event":
In that case, the process will be stuck in the corresponding function
(e.g. "rds_ib_post_reg_frmr") until the completion handler has occurred
and the "wake_up" call was issued.
It is the job of the "rds_ib_mr_cqe_handler" handler to inspect
the status of the work-completion and set the "fr_state" accordingly.
As far as I can tell, that is happening.
The debate on whether to use a "wait_event" or "wait_event_timeout"
is strictly a debate over whether or not there should
be an upper bound for the firmware to respond.
If there is not: It should be "wait_event".
If there is: It should be "wait_event_timeout", and we need to specify
what that upper bound is.
> Instead of timing out if you poll the CQ for that operation completion, it
> makes it full proof. That is the change Avinash has done iirc and
> am requesting to look at that fix.
>
The "wake_up" call is issued from within the completion handler.
The completion handler "rds_ib_mr_cqe_handler" is called upon
handling the work-completions coming out of "ib_poll_cq".
That is necessary in order to check the status of the completion.
There are many changes that were done in the Oracle internal repository
(including changes that Avinash had done),
that we will go through in order to see what needs to be fixed in the
upstream version of RDS.
But unless you can see something that is wrong with this proposed fix,
I would suggest we don't leave the upstream RDS broken for extended
periods of time, but rather fix it.
> Other 5 fixes from the series looks fine.
>
Thank you,
Gerd
^ permalink raw reply
* Re: [PATCH net-next v6 04/15] ethtool: introduce ethtool netlink interface
From: Jakub Kicinski @ 2019-07-03 1:29 UTC (permalink / raw)
To: Michal Kubecek
Cc: David Miller, netdev, Jiri Pirko, Andrew Lunn, Florian Fainelli,
John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <e7fa3ad7e9cf4d7a8f9a2085e3166f7260845b0a.1562067622.git.mkubecek@suse.cz>
On Tue, 2 Jul 2019 13:49:59 +0200 (CEST), Michal Kubecek wrote:
> diff --git a/Documentation/networking/ethtool-netlink.txt b/Documentation/networking/ethtool-netlink.txt
> new file mode 100644
> index 000000000000..97c369aa290b
> --- /dev/null
> +++ b/Documentation/networking/ethtool-netlink.txt
> @@ -0,0 +1,208 @@
> + Netlink interface for ethtool
> + =============================
> +
> +
> +Basic information
> +-----------------
Probably not a blocker for initial merging, but please note a TODO to
convert the documentation to ReST.
^ permalink raw reply
* Re: net: micrel: confusion about phyids used in driver
From: Uwe Kleine-König @ 2019-07-02 20:31 UTC (permalink / raw)
To: Yuiko Oshino
Cc: netdev, Andrew Lunn, Florian Fainelli, kernel, Heiner Kallweit
In-Reply-To: <20190510072243.h6h3bgvr2ovsh5g5@pengutronix.de>
Hello Yuiko Oshino,
On Fri, May 10, 2019 at 09:22:43AM +0200, Uwe Kleine-König wrote:
> On Thu, May 09, 2019 at 11:07:45PM +0200, Andrew Lunn wrote:
> > On Thu, May 09, 2019 at 10:55:29PM +0200, Heiner Kallweit wrote:
> > > On 09.05.2019 22:29, Uwe Kleine-König wrote:
> > > > I have a board here that has a KSZ8051MLL (datasheet:
> > > > http://ww1.microchip.com/downloads/en/DeviceDoc/ksz8051mll.pdf, phyid:
> > > > 0x0022155x) assembled. The actual phyid is 0x00221556.
> > >
> > > I think the datasheets are the source of the confusion. If the
> > > datasheets for different chips list 0x0022155x as PHYID each, and
> > > authors of support for additional chips don't check the existing code,
> > > then happens what happened.
> > >
> > > However it's not a rare exception and not Microchip-specific that
> > > sometimes vendors use the same PHYID for different chips.
>
> From the vendor's POV it is even sensible to reuse the phy IDs iff the
> chips are "compatible".
>
> Assuming that the last nibble of the phy ID actually helps to
> distinguish the different (not completely) compatible chips, we need
> some more detailed information than available in the data sheets I have.
> There is one person in the recipents of this mail with an @microchip.com
> address (hint, hint!).
can you give some input here or forward to a person who can?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH net-next v6 05/15] ethtool: helper functions for netlink interface
From: Jakub Kicinski @ 2019-07-03 1:28 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, Jiri Pirko, David Miller, Andrew Lunn, Florian Fainelli,
John Linville, Stephen Hemminger, Johannes Berg, linux-kernel
In-Reply-To: <20190702163437.GE20101@unicorn.suse.cz>
On Tue, 2 Jul 2019 18:34:37 +0200, Michal Kubecek wrote:
> > >+ ret = nla_parse_nested(tb, ETHTOOL_A_HEADER_MAX, nest,
> > >+ policy ?: dflt_header_policy, extack);
> > >+ if (ret < 0)
> >
> > if (ret)
> >
> > Same remark goes to the rest of the code (also the rest of the patches),
> > in case called function cannot return positive values.
>
> The "if (ret < 0)" idiom for "on error do ..." is so ubiquitous through
> the whole kernel that I don't think it's worth it to carefully check
> which function can return a positive value and which cannot and risk
> that one day I overlook that some function. And yet another question is
> what exactly "cannot return" means: is it whenever the function does not
> return a positive value or only if it's explicitly documented not to?
>
> Looking at existing networking code, e.g. net/netfilter (except ipvs),
> net/sched or net/core/rtnetlink.c are using "if (ret < 0)" rather
> uniformly. And (as you objected to the check of genl_register_family()
> previous patch) even genetlink itself has
>
> err = genl_register_family(&genl_ctrl);
> if (err < 0)
> goto problem;
>
> in genl_init().
I agree with Jiri, if a function only returns "0, or -errno" it's
easier to parse if the error check is not only for negative values.
At least to my eyes.
What I'm not sure about is whether we want to delay the merging of this
interface over this..
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Song Liu @ 2019-07-02 23:48 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Kees Cook, linux-security@vger.kernel.org, Networking, bpf,
Alexei Starovoitov, Daniel Borkmann, Kernel Team, Lorenz Bauer,
Jann Horn, Greg KH, Linux API
In-Reply-To: <CALCETrXTta26CTtEDnzvtd03-WOGdXcnsAogP8JjLkcj4-mHvg@mail.gmail.com>
> On Jul 3, 2019, at 5:32 AM, Andy Lutomirski <luto@kernel.org> wrote:
>
> On Tue, Jul 2, 2019 at 2:04 PM Kees Cook <keescook@chromium.org> wrote:
>>
>> On Mon, Jul 01, 2019 at 06:59:13PM -0700, Andy Lutomirski wrote:
>>> I think I'm understanding your motivation. You're not trying to make
>>> bpf() generically usable without privilege -- you're trying to create
>>> a way to allow certain users to access dangerous bpf functionality
>>> within some limits.
>>>
>>> That's a perfectly fine goal, but I think you're reinventing the
>>> wheel, and the wheel you're reinventing is quite complicated and
>>> already exists. I think you should teach bpftool to be secure when
>>> installed setuid root or with fscaps enabled and put your policy in
>>> bpftool. If you want to harden this a little bit, it would seem
>>> entirely reasonable to add a new CAP_BPF_ADMIN and change some, but
>>> not all, of the capable() checks to check CAP_BPF_ADMIN instead of the
>>> capabilities that they currently check.
>>
>> If finer grained controls are wanted, it does seem like the /dev/bpf
>> path makes the most sense. open, request abilities, use fd. The open can
>> be mediated by DAC and LSM. The request can be mediated by LSM. This
>> provides a way to add policy at the LSM level and at the tool level.
>> (i.e. For tool-level controls: leave LSM wide open, make /dev/bpf owned
>> by "bpfadmin" and bpftool becomes setuid "bpfadmin". For fine-grained
>> controls, leave /dev/bpf wide open and add policy to SELinux, etc.)
>>
>> With only a new CAP, you don't get the fine-grained controls. (The
>> "request abilities" part is the key there.)
>
> Sure you do: the effective set. It has somewhat bizarre defaults, but
> I don't think that's a real problem. Also, this wouldn't be like
> CAP_DAC_READ_SEARCH -- you can't accidentally use your BPF caps.
>
> I think that a /dev capability-like object isn't totally nuts, but I
> think we should do it well, and this patch doesn't really achieve
> that. But I don't think bpf wants fine-grained controls like this at
> all -- as I pointed upthread, a fine-grained solution really wants
> different treatment for the different capable() checks, and a bunch of
> them won't resemble capabilities or /dev/bpf at all.
Thanks everyone again for great inputs. We will discuss this again and
respin the set.
Best,
Song
^ permalink raw reply
* RE: net: micrel: confusion about phyids used in driver
From: Yuiko.Oshino @ 2019-07-02 20:55 UTC (permalink / raw)
To: u.kleine-koenig
Cc: netdev, andrew, f.fainelli, kernel, hkallweit1, Ravi.Hegde,
Tristram.Ha
In-Reply-To: <20190702203152.gviukfldjhdnmu7j@pengutronix.de>
>-----Original Message-----
>From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>Sent: Tuesday, July 2, 2019 4:32 PM
>To: Yuiko Oshino - C18177 <Yuiko.Oshino@microchip.com>
>Cc: netdev@vger.kernel.org; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
><f.fainelli@gmail.com>; kernel@pengutronix.de; Heiner Kallweit
><hkallweit1@gmail.com>
>Subject: Re: net: micrel: confusion about phyids used in driver
>
>External E-Mail
>
>
>Hello Yuiko Oshino,
>
>On Fri, May 10, 2019 at 09:22:43AM +0200, Uwe Kleine-König wrote:
>> On Thu, May 09, 2019 at 11:07:45PM +0200, Andrew Lunn wrote:
>> > On Thu, May 09, 2019 at 10:55:29PM +0200, Heiner Kallweit wrote:
>> > > On 09.05.2019 22:29, Uwe Kleine-König wrote:
>> > > > I have a board here that has a KSZ8051MLL (datasheet:
>> > > > http://ww1.microchip.com/downloads/en/DeviceDoc/ksz8051mll.pdf,
>phyid:
>> > > > 0x0022155x) assembled. The actual phyid is 0x00221556.
>> > >
>> > > I think the datasheets are the source of the confusion. If the
>> > > datasheets for different chips list 0x0022155x as PHYID each, and
>> > > authors of support for additional chips don't check the existing
>> > > code, then happens what happened.
>> > >
>> > > However it's not a rare exception and not Microchip-specific that
>> > > sometimes vendors use the same PHYID for different chips.
>>
>> From the vendor's POV it is even sensible to reuse the phy IDs iff the
>> chips are "compatible".
>>
>> Assuming that the last nibble of the phy ID actually helps to
>> distinguish the different (not completely) compatible chips, we need
>> some more detailed information than available in the data sheets I have.
>> There is one person in the recipents of this mail with an
>> @microchip.com address (hint, hint!).
>
>can you give some input here or forward to a person who can?
>
>Best regards
>Uwe
>
>--
>Pengutronix e.K. | Uwe Kleine-König |
>Industrial Linux Solutions | http://www.pengutronix.de/ |
Hello Uwe,
I forward this to the team.
Best regards,
Yuiko
^ permalink raw reply
* [PATCH v2] net: don't warn in inet diag when IPV6 is disabled
From: Stephen Hemminger @ 2019-07-02 22:20 UTC (permalink / raw)
To: davem, kuznet, yoshfuji; +Cc: netdev, Stephen Hemminger
If IPV6 was disabled, then ss command would cause a kernel warning
because the command was attempting to dump IPV6 socket information.
The fix is to just remove the warning.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202249
Fixes: 432490f9d455 ("net: ip, diag -- Add diag interface for raw sockets")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/ipv4/raw_diag.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/ipv4/raw_diag.c b/net/ipv4/raw_diag.c
index 899e34ceb560..e35736b99300 100644
--- a/net/ipv4/raw_diag.c
+++ b/net/ipv4/raw_diag.c
@@ -24,9 +24,6 @@ raw_get_hashinfo(const struct inet_diag_req_v2 *r)
return &raw_v6_hashinfo;
#endif
} else {
- pr_warn_once("Unexpected inet family %d\n",
- r->sdiag_family);
- WARN_ON_ONCE(1);
return ERR_PTR(-EINVAL);
}
}
--
2.20.1
^ permalink raw reply related
* Re: Validation of forward_delay seems wrong...
From: Nikolay Aleksandrov @ 2019-07-02 21:19 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Roopa Prabhu, bridge, netdev
In-Reply-To: <20190702204705.GC28471@lunn.ch>
On 02/07/2019 23:47, Andrew Lunn wrote:
> Hi Nikolay
>
> The man page says that the bridge forward_delay is in units of
> seconds, and should be between 2 and 30.
>
> I've tested on a couple of different kernel versions, and this appears
> to be not working correctly:
>
> ip link set br0 type bridge forward_delay 2
> RTNETLINK answers: Numerical result out of range
>
> ip link set br0 type bridge forward_delay 199
> RTNETLINK answers: Numerical result out of range
>
> ip link set br0 type bridge forward_delay 200
> #
>
> ip link set br0 type bridge forward_delay 3000
> #
>
> ip link set br0 type bridge forward_delay 3001
> RTNETLINK answers: Numerical result out of range
>
> I've not checked what delay is actually being used here, but clearly
> something is mixed up.
>
> grep HZ .config
> CONFIG_HZ_PERIODIC=y
> # CONFIG_NO_HZ_IDLE is not set
> # CONFIG_NO_HZ_FULL is not set
> # CONFIG_NO_HZ is not set
> CONFIG_HZ_FIXED=0
> CONFIG_HZ_100=y
> # CONFIG_HZ_200 is not set
> # CONFIG_HZ_250 is not set
> # CONFIG_HZ_300 is not set
> # CONFIG_HZ_500 is not set
> # CONFIG_HZ_1000 is not set
> CONFIG_HZ=100
>
> Thanks
> Andrew
>
Hi Andrew,
The man page is wrong, these have been in USER_HZ scaled clock_t format from the beginning.
TBH a lot of the time/delay bridge config options are messed up like that.
We've been discussing adding special _ms versions in iproute2 to make them
more user-friendly and understandable. Will cook a patch for the man page.
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-07-02 21:32 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Song Liu, linux-security@vger.kernel.org,
Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Kernel Team,
Lorenz Bauer, Jann Horn, Greg KH, Linux API
In-Reply-To: <201907021115.DCD56BBABB@keescook>
On Tue, Jul 2, 2019 at 2:04 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Jul 01, 2019 at 06:59:13PM -0700, Andy Lutomirski wrote:
> > I think I'm understanding your motivation. You're not trying to make
> > bpf() generically usable without privilege -- you're trying to create
> > a way to allow certain users to access dangerous bpf functionality
> > within some limits.
> >
> > That's a perfectly fine goal, but I think you're reinventing the
> > wheel, and the wheel you're reinventing is quite complicated and
> > already exists. I think you should teach bpftool to be secure when
> > installed setuid root or with fscaps enabled and put your policy in
> > bpftool. If you want to harden this a little bit, it would seem
> > entirely reasonable to add a new CAP_BPF_ADMIN and change some, but
> > not all, of the capable() checks to check CAP_BPF_ADMIN instead of the
> > capabilities that they currently check.
>
> If finer grained controls are wanted, it does seem like the /dev/bpf
> path makes the most sense. open, request abilities, use fd. The open can
> be mediated by DAC and LSM. The request can be mediated by LSM. This
> provides a way to add policy at the LSM level and at the tool level.
> (i.e. For tool-level controls: leave LSM wide open, make /dev/bpf owned
> by "bpfadmin" and bpftool becomes setuid "bpfadmin". For fine-grained
> controls, leave /dev/bpf wide open and add policy to SELinux, etc.)
>
> With only a new CAP, you don't get the fine-grained controls. (The
> "request abilities" part is the key there.)
Sure you do: the effective set. It has somewhat bizarre defaults, but
I don't think that's a real problem. Also, this wouldn't be like
CAP_DAC_READ_SEARCH -- you can't accidentally use your BPF caps.
I think that a /dev capability-like object isn't totally nuts, but I
think we should do it well, and this patch doesn't really achieve
that. But I don't think bpf wants fine-grained controls like this at
all -- as I pointed upthread, a fine-grained solution really wants
different treatment for the different capable() checks, and a bunch of
them won't resemble capabilities or /dev/bpf at all.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 6/9] libbpf: use negative fd to specify missing BTF
From: Stanislav Fomichev @ 2019-07-02 23:18 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: andrii.nakryiko, netdev, bpf, ast, daniel, kernel-team
In-Reply-To: <20190702225733.GK6757@mini-arch>
On 07/02, Stanislav Fomichev wrote:
> On 05/29, Andrii Nakryiko wrote:
> > 0 is a valid FD, so it's better to initialize it to -1, as is done in
> > other places. Also, technically, BTF type ID 0 is valid (it's a VOID
> > type), so it's more reliable to check btf_fd, instead of
> > btf_key_type_id, to determine if there is any BTF associated with a map.
> >
> > Acked-by: Song Liu <songliubraving@fb.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/libbpf.c | 13 +++++++------
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index c972fa10271f..a27a0351e595 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -1751,7 +1751,7 @@ bpf_object__create_maps(struct bpf_object *obj)
> > create_attr.key_size = def->key_size;
> > create_attr.value_size = def->value_size;
> > create_attr.max_entries = def->max_entries;
> > - create_attr.btf_fd = 0;
> > + create_attr.btf_fd = -1;
> > create_attr.btf_key_type_id = 0;
> > create_attr.btf_value_type_id = 0;
> > if (bpf_map_type__is_map_in_map(def->type) &&
> > @@ -1765,11 +1765,11 @@ bpf_object__create_maps(struct bpf_object *obj)
> > }
> >
> > *pfd = bpf_create_map_xattr(&create_attr);
> > - if (*pfd < 0 && create_attr.btf_key_type_id) {
> > + if (*pfd < 0 && create_attr.btf_fd >= 0) {
> > cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
> > pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
> > map->name, cp, errno);
> > - create_attr.btf_fd = 0;
> > + create_attr.btf_fd = -1;
> This breaks libbpf compatibility with the older kernels. If the kernel
> doesn't know about btf_fd and we set it to -1, then CHECK_ATTR
> fails :-(
>
> Any objections to converting BTF retries to bpf_capabilities and then
> knowingly passing bft_fd==0 or proper fd?
Oh, nevermind, it looks like you fixed it already in e55d54f43d3f.
> > create_attr.btf_key_type_id = 0;
> > create_attr.btf_value_type_id = 0;
> > map->btf_key_type_id = 0;
> > @@ -2053,6 +2053,9 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > char *log_buf;
> > int ret;
> >
> > + if (!insns || !insns_cnt)
> > + return -EINVAL;
> > +
> > memset(&load_attr, 0, sizeof(struct bpf_load_program_attr));
> > load_attr.prog_type = prog->type;
> > load_attr.expected_attach_type = prog->expected_attach_type;
> > @@ -2063,7 +2066,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > load_attr.license = license;
> > load_attr.kern_version = kern_version;
> > load_attr.prog_ifindex = prog->prog_ifindex;
> > - load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
> > + load_attr.prog_btf_fd = prog->btf_fd;
> > load_attr.func_info = prog->func_info;
> > load_attr.func_info_rec_size = prog->func_info_rec_size;
> > load_attr.func_info_cnt = prog->func_info_cnt;
> > @@ -2072,8 +2075,6 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
> > load_attr.line_info_cnt = prog->line_info_cnt;
> > load_attr.log_level = prog->log_level;
> > load_attr.prog_flags = prog->prog_flags;
> > - if (!load_attr.insns || !load_attr.insns_cnt)
> > - return -EINVAL;
> >
> > retry_load:
> > log_buf = malloc(log_buf_size);
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: [PATCH net-next] Revert "r8169: improve handling VLAN tag"
From: David Miller @ 2019-07-02 22:28 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev, viro
In-Reply-To: <df6fcca2-6db6-0d76-deb2-6b2e98e8bf54@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 2 Jul 2019 07:59:17 +0200
> This reverts commit 759d095741721888b6ee51afa74e0a66ce65e974.
>
> The patch was based on a misunderstanding. As Al Viro pointed out [0]
> it's simply wrong on big endian. So let's revert it.
>
> [0] https://marc.info/?t=156200975600004&r=1&w=2
>
> Reported-by: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] r8169: add random MAC address fallback
From: David Miller @ 2019-07-02 22:29 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <60c26de4-23bc-a94b-d4a0-1216d8053e1f@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 2 Jul 2019 18:49:14 +0200
> On 02.07.2019 08:18, Heiner Kallweit wrote:
>>>From 1c8bacf724f1450e5256c68fbff407305faf9cbd Mon Sep 17 00:00:00 2001
>>
>>
>>
>
> Sorry, something went wrong when preparing the commit message. I'll resubmit.
It helps to speak closer to the microphone.
^ permalink raw reply
* Re: [PATCH v2 net-next] r8169: add random MAC address fallback
From: David Miller @ 2019-07-02 22:28 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <637b6d1f-85b1-dc9a-8e77-68ffcd82eae9@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Tue, 2 Jul 2019 20:46:09 +0200
> It was reported that the GPD MicroPC is broken in a way that no valid
> MAC address can be read from the network chip. The vendor driver deals
> with this by assigning a random MAC address as fallback. So let's do
> the same.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - fix broken commit message
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Ilias Apalodimas @ 2019-07-02 21:15 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Ivan Khoronzhuk, netdev, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap
In-Reply-To: <20190702230241.3be6d787@carbon>
Hi Jesper,
Getting late here, i'll respond in detail tomorrow. One point though
[...]
>
> This special use-case, seems confined to your driver. And Ilias told me
> that XDP is not really a performance benefit for this driver as the HW
> PPS-limit is hit before the XDP and netstack limit. I ask, does it
> make sense to add XDP to this driver, if it complicates the code for
> everybody else?
I think yes. This is a widely used driver on TI embedded devices so having XDP
to play along is a nice feature. It's also the first and only armv7 we have
supporting this. Ivan already found a couple of issues due to the 32-bit
architecture he is trying to fix, i think there's real benefit in having that,
performance aside.
I fully agree we should not impact the performance of the API to support a
special hardware though. I'll have a look on the 2 solutions tomorrow, but the
general approach on this one should be 'the simpler the better'
Cheers
/Ilias
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH 1/2 nf-next v3] netfilter: nft_meta: Add NFT_META_BRI_IIFVPROTO support
From: Pablo Neira Ayuso @ 2019-07-02 23:46 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <1561682975-21790-1-git-send-email-wenxu@ucloud.cn>
On Fri, Jun 28, 2019 at 08:49:34AM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> This patch provide a meta to get the bridge vlan proto
>
> nft add rule bridge firewall zones counter meta br_vlan_proto 0x8100
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v4 bpf-next] bpf: Add support for fq's EDT to HBM
From: Yonghong Song @ 2019-07-02 23:02 UTC (permalink / raw)
To: Lawrence Brakmo, netdev
Cc: Alexei Starovoitov, Daniel Borkmann, Eric Dumazet, Kernel Team
In-Reply-To: <20190702220952.3929270-1-brakmo@fb.com>
On 7/2/19 3:09 PM, brakmo wrote:
> Adds support for fq's Earliest Departure Time to HBM (Host Bandwidth
> Manager). Includes a new BPF program supporting EDT, and also updates
> corresponding programs.
>
> It will drop packets with an EDT of more than 500us in the future
> unless the packet belongs to a flow with less than 2 packets in flight.
> This is done so each flow has at least 2 packets in flight, so they
> will not starve, and also to help prevent delayed ACK timeouts.
>
> It will also work with ECN enabled traffic, where the packets will be
> CE marked if their EDT is more than 50us in the future.
>
> The table below shows some performance numbers. The flows are back to
> back RPCS. One server sending to another, either 2 or 4 flows.
> One flow is a 10KB RPC, the rest are 1MB RPCs. When there are more
> than one flow of a given RPC size, the numbers represent averages.
>
> The rate limit applies to all flows (they are in the same cgroup).
> Tests ending with "-edt" ran with the new BPF program supporting EDT.
> Tests ending with "-hbt" ran on top HBT qdisc with the specified rate
> (i.e. no HBM). The other tests ran with the HBM BPF program included
> in the HBM patch-set.
>
> EDT has limited value when using DCTCP, but it helps in many cases when
> using Cubic. It usually achieves larger link utilization and lower
> 99% latencies for the 1MB RPCs.
> HBM ends up queueing a lot of packets with its default parameter values,
> reducing the goodput of the 10KB RPCs and increasing their latency. Also,
> the RTTs seen by the flows are quite large.
>
> Aggr 10K 10K 10K 1MB 1MB 1MB
> Limit rate drops RTT rate P90 P99 rate P90 P99
> Test rate Flows Mbps % us Mbps us us Mbps ms ms
> -------- ---- ----- ---- ----- --- ---- ---- ---- ---- ---- ----
> cubic 1G 2 904 0.02 108 257 511 539 647 13.4 24.5
> cubic-edt 1G 2 982 0.01 156 239 656 967 743 14.0 17.2
> dctcp 1G 2 977 0.00 105 324 408 744 653 14.5 15.9
> dctcp-edt 1G 2 981 0.01 142 321 417 811 660 15.7 17.0
> cubic-htb 1G 2 919 0.00 1825 40 2822 4140 879 9.7 9.9
>
> cubic 200M 2 155 0.30 220 81 532 655 74 283 450
> cubic-edt 200M 2 188 0.02 222 87 1035 1095 101 84 85
> dctcp 200M 2 188 0.03 111 77 912 939 111 76 325
> dctcp-edt 200M 2 188 0.03 217 74 1416 1738 114 76 79
> cubic-htb 200M 2 188 0.00 5015 8 14ms 15ms 180 48 50
>
> cubic 1G 4 952 0.03 110 165 516 546 262 38 154
> cubic-edt 1G 4 973 0.01 190 111 1034 1314 287 65 79
> dctcp 1G 4 951 0.00 103 180 617 905 257 37 38
> dctcp-edt 1G 4 967 0.00 163 151 732 1126 272 43 55
> cubic-htb 1G 4 914 0.00 3249 13 7ms 8ms 300 29 34
>
> cubic 5G 4 4236 0.00 134 305 490 624 1310 10 17
> cubic-edt 5G 4 4865 0.00 156 306 425 759 1520 10 16
> dctcp 5G 4 4936 0.00 128 485 221 409 1484 7 9
> dctcp-edt 5G 4 4924 0.00 148 390 392 623 1508 11 26
>
> v1 -> v2: Incorporated Andrii's suggestions
> v2 -> v3: Incorporated Yonghong's suggestions
> v3 -> v4: Removed credit update that is not needed
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH net] net: don't warn in inet diag when IPV6 is disabled
From: David Miller @ 2019-07-02 21:23 UTC (permalink / raw)
To: stephen; +Cc: kuznet, yoshfuji, netdev
In-Reply-To: <20190701152303.4031-1-stephen@networkplumber.org>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 1 Jul 2019 08:23:03 -0700
> @@ -19,9 +19,11 @@ raw_get_hashinfo(const struct inet_diag_req_v2 *r)
> {
> if (r->sdiag_family == AF_INET) {
> return &raw_v4_hashinfo;
> -#if IS_ENABLED(CONFIG_IPV6)
> } else if (r->sdiag_family == AF_INET6) {
> +#if IS_ENABLED(CONFIG_IPV6)
> return &raw_v6_hashinfo;
> +#else
> + return ERR_PTR(-EOPNOTSUPP);
> #endif
> } else {
> pr_warn_once("Unexpected inet family %d\n",
Let's make some consistency in this area please.
The inet_diag code returns -EINVAL, and that's been that way forever.
It also doesn't print a weird warning for unexpected sdiag_family
values outside of AF_INET and AF_INET6.
That's been that way for so long that's probably the behavior to
revolve everything around.
Therefore, please just get rid of the warning message instead of
all of these other changes.
Thank you.
^ permalink raw reply
* Re: [PATCH] net: stmmac: make "snps,reset-delays-us" optional again
From: David Miller @ 2019-07-02 22:23 UTC (permalink / raw)
To: martin.blumenstingl
Cc: netdev, colin.king, peppe.cavallaro, alexandre.torgue, joabreu,
linux-arm-kernel, linux-kernel
In-Reply-To: <20190701224225.19701-1-martin.blumenstingl@googlemail.com>
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date: Tue, 2 Jul 2019 00:42:25 +0200
> Commit 760f1dc2958022 ("net: stmmac: add sanity check to
> device_property_read_u32_array call") introduced error checking of the
> device_property_read_u32_array() call in stmmac_mdio_reset().
> This results in the following error when the "snps,reset-delays-us"
> property is not defined in devicetree:
> invalid property snps,reset-delays-us
>
> This sanity check made sense until commit 84ce4d0f9f55b4 ("net: stmmac:
> initialize the reset delay array") ensured that there are fallback
> values for the reset delay if the "snps,reset-delays-us" property is
> absent. That was at the cost of making that property mandatory though.
>
> Drop the sanity check for device_property_read_u32_array() and thus make
> the "snps,reset-delays-us" property optional again (avoiding the error
> message while loading the stmmac driver with a .dtb where the property
> is absent).
>
> Fixes: 760f1dc2958022 ("net: stmmac: add sanity check to device_property_read_u32_array call")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> This is a fix for a patch in net-next and should either go into net-next
> or 5.3-rcX.
Ok, applied to net-next.
^ 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