Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v1 1/4] enetc: Clean up local mdio bus allocation
From: Andrew Lunn @ 2019-07-24 16:39 UTC (permalink / raw)
  To: Claudiu Manoil
  Cc: David S . Miller, Rob Herring, Leo Li, Alexandru Marginean,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR04MB4880CD977A5D58DA0A7EE56696C60@VI1PR04MB4880.eurprd04.prod.outlook.com>

> >All the horrible casts go away, the driver is structured like every
> >other driver, sparse is probably happy, etc.
> >
> 
> This looks more like a matter cosmetic preferences.  I mean, I didn't
> notice anything "horrible" in the code so far.

#define bus_to_enetc_regs(bus)  (struct enetc_mdio_regs __iomem *)((bus)->priv)

You should not need a cast here, bus->priv is a void *. But bus->priv
is being abused to hold a __iomem pointer.

enetc_wr_reg(&regs->mdio_cfg, mdio_cfg);

This is also rather odd, passing the address of something to an IO
operator? I also don't know the C standard well enough to know if it
is guaranteed that:

struct enetc_mdio_regs {
        u32     mdio_cfg;       /* MDIO configuration and status */
        u32     mdio_ctl;       /* MDIO control */
        u32     mdio_data;      /* MDIO data */
        u32     mdio_addr;      /* MDIO address */
};

actually works. On a 64bit system is the compiler allowed to put in
padding to keep the u32 64 bit aligned?

> I actually find it more
> ugly to define a new structure with only one element inside, like:
> struct enetc_mdio_priv {
>        struct enetc_hw *hw;
> }

One advantage of this is that struct enetc_hw correctly has all the
__iomem attributes. All the casts to __iomem go away, and sparse is
happy.

> Anyway, if others already did this in the kernel, what can I do?

Clean it up. Make the code more readable and easy to maintain.

      Andrew

^ permalink raw reply

* Re: Reminder: 99 open syzbot bugs in net subsystem
From: Eric Biggers @ 2019-07-24 16:30 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Dmitry Vyukov, netdev, David S. Miller, Florian Westphal,
	Ilya Maximets, Eric Dumazet, David Ahern, linux-kernel,
	syzkaller-bugs
In-Reply-To: <63f12327-dd4b-5210-4de2-705af6bc4ba4@gmail.com>

On Wed, Jul 24, 2019 at 08:39:05AM +0200, Eric Dumazet wrote:
> 
> 
> On 7/24/19 3:38 AM, Eric Biggers wrote:
> > [This email was generated by a script.  Let me know if you have any suggestions
> > to make it better, or if you want it re-generated with the latest status.]
> > 
> > Of the currently open syzbot reports against the upstream kernel, I've manually
> > marked 99 of them as possibly being bugs in the net subsystem.  This category
> > only includes the networking bugs that I couldn't assign to a more specific
> > component (bpf, xfrm, bluetooth, tls, tipc, sctp, wireless, etc.).  I've listed
> > these reports below, sorted by an algorithm that tries to list first the reports
> > most likely to be still valid, important, and actionable.
> > 
> > Of these 99 bugs, 17 were seen in mainline in the last week.
> > 
> > Of these 99 bugs, 4 were bisected to commits from the following people:
> > 
> > 	Florian Westphal <fw@strlen.de>
> > 	Ilya Maximets <i.maximets@samsung.com>
> > 	Eric Dumazet <edumazet@google.com>
> > 	David Ahern <dsahern@gmail.com>
> > 
> > If you believe a bug is no longer valid, please close the syzbot report by
> > sending a '#syz fix', '#syz dup', or '#syz invalid' command in reply to the
> > original thread, as explained at https://goo.gl/tpsmEJ#status
> > 
> > If you believe I misattributed a bug to the net subsystem, please let me know,
> > and if possible forward the report to the correct people or mailing list.
> >
> 
> Some of the bugs have been fixed already, before syzbot found them.
> 
> Why force human to be gentle to bots and actually replying to them ?
> 
> I usually simply wait that syzbot is finding the bug does not repro anymore,
> but now if you send these emails, we will have even more pressure on us.
> 

First, based on experience, I'd guess about 30-45 of these are still valid.  17
were seen in mainline in the last week, but some others are valid too.  The ones
most likely to still be valid are at the beginning of the list.  So let's try
not use the presence of outdated bugs as an excuse not to fix current bugs.

Second, all these bug reports are still open, regardless of whether reminders
are sent or not.  I think you're really suggesting that possibly outdated bug
reports should be automatically invalidated by syzbot.

syzbot already does that for bugs with no reproducer.  However, that still
leaves a lot of outdated bugs with reproducers.

Since the kernel community is basically in continuous bug bankruptcy and lots of
syzbot reports are being ignored anyway, I'm in favor of making the invalidation
criteria more aggressive, so we can best focus people's efforts.  I understand
that Dmitry has been against this though, since a significant fraction of bugs
that syzbot stopped hitting for some reason actually turn out to be still valid.

But we probably have no choice.  So I suggest we agree on new criteria for
invalidating bugs.  I'd suggest assigning a timeout to each bug, based on
attributes like "seen in mainline?", "reproducer type", "bisected?", "does it
look like a 'bad' crash (e.g. use-after-free)"; similar to the algorithm I'm
using to sort the bugs when sorting these reminders.  I.e., bugs most likely to
still be valid, important, and actionable get longest timeouts.

Then if no crash or activity was seen in the timeout, the bug is closed.

Any thoughts from anyone?

- Eric

^ permalink raw reply

* Re: kernel panic: stack is corrupted in pointer
From: John Fastabend @ 2019-07-24 16:22 UTC (permalink / raw)
  To: Dmitry Vyukov, John Fastabend
  Cc: syzbot, bpf, David Airlie, alexander.deucher, amd-gfx,
	Alexei Starovoitov, christian.koenig, Daniel Borkmann,
	david1.zhou, DRI, leo.liu, LKML, netdev, syzkaller-bugs,
	Marco Elver
In-Reply-To: <CACT4Y+ZbPmRB9T9ZzhE79VnKKD3+ieHeLpaDGRkcQ72nADKH_g@mail.gmail.com>

Dmitry Vyukov wrote:
> On Tue, Jul 23, 2019 at 7:26 PM John Fastabend <john.fastabend@gmail.com> wrote:
> >
> > Dmitry Vyukov wrote:
> > > On Wed, Jul 17, 2019 at 10:58 AM syzbot
> > > <syzbot+79f5f028005a77ecb6bb@syzkaller.appspotmail.com> wrote:
> > > >
> > > > Hello,
> > > >
> > > > syzbot found the following crash on:
> > > >
> > > > HEAD commit:    1438cde7 Add linux-next specific files for 20190716
> > > > git tree:       linux-next
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=13988058600000
> > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=3430a151e1452331
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=79f5f028005a77ecb6bb
> > > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=111fc8afa00000
> > >
> > > From the repro it looks like the same bpf stack overflow bug. +John
> > > We need to dup them onto some canonical report for this bug, or this
> > > becomes unmanageable.
> >
> > Fixes in bpf tree should fix this. Hopefully, we will squash this once fixes
> > percolate up.
> >
> > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
> 
> Cool! What is the fix?

It took a series of patches here,

https://www.spinics.net/lists/netdev/msg586986.html

The fix commits from bpf tree are,

(git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git) 

318892ac068397f40ff81d9155898da01493b1d2
ac78fc148d8249dbf382c2127456dd08ec5b161c
f87e62d45e51b12d48d2cb46b5cde8f83b866bc4
313ab004805cf52a42673b15852b3842474ccd87
32857cf57f920cdc03b5095f08febec94cf9c36b
45a4521dcbd92e71c9e53031b40e34211d3b4feb
2bb90e5cc90e1d09f631aeab041a9cf913a5bbe5
0e858739c2d2eedeeac1d35bfa0ec3cc2a7190d8
95fa145479fbc0a0c1fd3274ceb42ec03c042a4a

The last commit fixes this paticular syzbot issue,

commit 95fa145479fbc0a0c1fd3274ceb42ec03c042a4a
Author: John Fastabend <john.fastabend@gmail.com>
Date:   Fri Jul 19 10:29:22 2019 -0700

    bpf: sockmap/tls, close can race with map free

The other commits address some other issues found while testing.

> We don't need to wait for the fix to percolate up (and then down
> too!). syzbot gracefully handles when a patch is not yet present
> everywhere (it happens all the time).

Great. By the way the above should fix many of the outstanding
reports against bpf sockmap and tls side. I'll have to walk through
each one individually to double check though. I guess we can mark
them as dup reports and syzbot should sort it out?

> 
> Btw, this was due to a stack overflow, right? Or something else?

Right, stack overflow due to race in updating sock ops where build a
circular call chain.

> We are trying to make KASAN configuration detect stack overflows too,
> so that it does not cause havoc next time. But it turns out to be
> non-trivial and our current attempt seems to fail:
> https://groups.google.com/forum/#!topic/kasan-dev/IhYv7QYhLfY
> 
> 

^ permalink raw reply

* [PATCH -next v2] net/ixgbevf: fix a compilation error of skb_frag_t
From: Qian Cai @ 2019-07-24 16:17 UTC (permalink / raw)
  To: davem; +Cc: jeffrey.t.kirsher, netdev, linux-kernel, Qian Cai

The linux-next commit "net: Rename skb_frag_t size to bv_len" [1]
introduced a compilation error on powerpc as it forgot to deal with the
renaming from "size" to "bv_len" for ixgbevf.

[1] https://lore.kernel.org/netdev/20190723030831.11879-1-willy@infradead.org/T/#md052f1c7de965ccd1bdcb6f92e1990a52298eac5

In file included from ./include/linux/cache.h:5,
                 from ./include/linux/printk.h:9,
                 from ./include/linux/kernel.h:15,
                 from ./include/linux/list.h:9,
                 from ./include/linux/module.h:9,
                 from
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:12:
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c: In function
'ixgbevf_xmit_frame_ring':
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:4138:51: error:
'skb_frag_t' {aka 'struct bio_vec'} has no member named 'size'
   count += TXD_USE_COUNT(skb_shinfo(skb)->frags[f].size);
                                                   ^
./include/uapi/linux/kernel.h:13:40: note: in definition of macro
'__KERNEL_DIV_ROUND_UP'
 #define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
                                        ^
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:4138:12: note: in
expansion of macro 'TXD_USE_COUNT'
   count += TXD_USE_COUNT(skb_shinfo(skb)->frags[f].size);

Signed-off-by: Qian Cai <cai@lca.pw>
---

v2: Use the fine accessor per Matthew.

 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index bdfccaf38edd..8c011d4ce7a9 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -4134,8 +4134,11 @@ static int ixgbevf_xmit_frame_ring(struct sk_buff *skb,
 	 * otherwise try next time
 	 */
 #if PAGE_SIZE > IXGBE_MAX_DATA_PER_TXD
-	for (f = 0; f < skb_shinfo(skb)->nr_frags; f++)
-		count += TXD_USE_COUNT(skb_shinfo(skb)->frags[f].size);
+	for (f = 0; f < skb_shinfo(skb)->nr_frags; f++) {
+		skb_frag_t *frag = &skb_shinfo(skb)->frags[f];
+
+		count += TXD_USE_COUNT(skb_frag_size(frag));
+	}
 #else
 	count += skb_shinfo(skb)->nr_frags;
 #endif
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH] net: phy: mscc: initialize stats array
From: Andrew Lunn @ 2019-07-24 16:16 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: netdev, Florian Fainelli, Heiner Kallweit, David S. Miller,
	linux-kernel
In-Reply-To: <mvmh87bih1y.fsf@suse.de>

On Wed, Jul 24, 2019 at 05:32:57PM +0200, Andreas Schwab wrote:
> The memory allocated for the stats array may contain arbitrary data.
> 
> Signed-off-by: Andreas Schwab <schwab@suse.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Fixes: e4f9ba642f0b ("net: phy: mscc: add support for VSC8514 PHY.")
Fixes: 00d70d8e0e78 ("net: phy: mscc: add support for VSC8574 PHY")
Fixes: a5afc1678044 ("net: phy: mscc: add support for VSC8584 PHY")
Fixes: f76178dc5218 ("net: phy: mscc: add ethtool statistics counters")

    Andrew

^ permalink raw reply

* Problem using skb_cow_data()
From: David Howells @ 2019-07-24 16:11 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs

Hi,

I have a problem using skb_cow_data() in rxkad_verify_packet{,_1,_2}() and was
wondering if anyone can suggest a better way.

The problem is that the rxrpc packet receive routine, rxrpc_input_data(),
receives an skb from the udp socket, makes it its own and then, if it's a data
packet, stores one or more[*] pointers to the skb, each with its own usage
ref, in the rx ring.

[*] The Rx protocol allows combinable packets (jumbo packet) that need to be
    split and each segment treated as a separate data packet.

rxrpc_input_data() then drops its own ref to the packet.  Possibly
simultaneously, the receiving process wakes up and starts processing the
packets earmarked for it.  This involves decrypting the packets if necessary
(which is done in place in the skb).  To do this, the rxkad_verify_packet*()
functions call skb_cow_data() on the skb.  This, however:

 (a) Can race with the input which may not have released its ref yet.

 (b) Could be a jumbo packet with multiple refs on it in the rx ring.

This can result in an assertion failure in pskb_expand_head():

	BUG_ON(skb_shared(skb));

In this particular case it shouldn't be an issue since the input path merely
has to release a ref and the subsequent attachment points in the ring buffer
if it's a jumbo packet will not be looked at until the current attachment
point is finished with (data delivery has to proceed delivery).

So, some questions:

 (1) Do I actually have to call skb_cow_data()?

 (2) Can the assertion be relaxed?

 (3) Is it feasible to do decryption directly from an skb to the target
     iov_iter, thereby avoiding the need to modify the skb content and saving
     a copy and potentially a bunch of allocations?  This would seem to
     require calling something like skb_copy_and_hash_datagram_iter(), but
     with a bespoke cb function to copy from the skb to the iov_iter.

     This gets interesting if the target iov_iter is smaller than the content
     in the skb as decryption would have to be suspended until a new iov_iter
     comes along.

David

^ permalink raw reply

* RE: [PATCH net-next v1 1/4] enetc: Clean up local mdio bus allocation
From: Claudiu Manoil @ 2019-07-24 16:03 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S . Miller, Rob Herring, Leo Li, Alexandru Marginean,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190724151803.GR25635@lunn.ch>



>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Wednesday, July 24, 2019 6:18 PM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: David S . Miller <davem@davemloft.net>; Rob Herring
><robh+dt@kernel.org>; Leo Li <leoyang.li@nxp.com>; Alexandru Marginean
><alexandru.marginean@nxp.com>; netdev@vger.kernel.org;
>devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH net-next v1 1/4] enetc: Clean up local mdio bus allocation
>
>On Wed, Jul 24, 2019 at 05:41:38PM +0300, Claudiu Manoil wrote:
>> Though it works, this is not how it should have been.
>> What's needed is a pointer to the mdio registers.
>> Store it properly inside bus->priv allocated space.
>> Use devm_* variant to further clean up the init error /
>> remove paths.
>>
>> Fixes following sparse warning:
>>  warning: incorrect type in assignment (different address spaces)
>>     expected void *priv
>>     got struct enetc_mdio_regs [noderef] <asn:2>*[assigned] regs
>>
>> Fixes: ebfcb23d62ab ("enetc: Add ENETC PF level external MDIO support")
>>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
>> ---
>> v1 - added this patch
>>
>>  .../net/ethernet/freescale/enetc/enetc_mdio.c | 31 +++++++------------
>>  1 file changed, 12 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
>b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
>> index 77b9cd10ba2b..1e3cd21c13ee 100644
>> --- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
>> +++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
>> @@ -15,7 +15,8 @@ struct enetc_mdio_regs {
>>  	u32	mdio_addr;	/* MDIO address */
>>  };
>>
>> -#define bus_to_enetc_regs(bus)	(struct enetc_mdio_regs __iomem
>*)((bus)->priv)
>> +#define bus_to_enetc_regs(bus)	(*(struct enetc_mdio_regs __iomem
>**) \
>> +				((bus)->priv))
>>
>>  #define ENETC_MDIO_REG_OFFSET	0x1c00
>>  #define ENETC_MDC_DIV		258
>> @@ -146,12 +147,12 @@ static int enetc_mdio_read(struct mii_bus *bus, int
>phy_id, int regnum)
>>  int enetc_mdio_probe(struct enetc_pf *pf)
>>  {
>>  	struct device *dev = &pf->si->pdev->dev;
>> -	struct enetc_mdio_regs __iomem *regs;
>> +	struct enetc_mdio_regs __iomem **regsp;
>>  	struct device_node *np;
>>  	struct mii_bus *bus;
>> -	int ret;
>> +	int err;
>>
>> -	bus = mdiobus_alloc_size(sizeof(regs));
>> +	bus = devm_mdiobus_alloc_size(dev, sizeof(*regsp));
>>  	if (!bus)
>>  		return -ENOMEM;
>>
>> @@ -159,41 +160,33 @@ int enetc_mdio_probe(struct enetc_pf *pf)
>>  	bus->read = enetc_mdio_read;
>>  	bus->write = enetc_mdio_write;
>>  	bus->parent = dev;
>> +	regsp = bus->priv;
>>  	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
>>
>>  	/* store the enetc mdio base address for this bus */
>> -	regs = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
>> -	bus->priv = regs;
>> +	*regsp = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
>
>This is all very odd and different to every other driver.
>
>If i get the code write, there are 4 registers, each u32 in size,
>starting at pf->si->hw.port + ENETC_MDIO_REG_OFFSET?
>
>There are macros like enetc_port_wr() and enetc_global_wr(). It think
>it would be much cleaner to add a macro enet_mdio_wr() which takes
>hw, off, val.
>
>#define enet_mdio_wr(hw, off, val) enet_port_wr(hw, off +
>ENETC_MDIO_REG_OFFSET, val)
>
>struct enetc_mdio_priv {
>       struct enetc_hw *hw;
>}
>
>	struct enetc_mdio_priv *mdio_priv;
>
>	bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));
>
>	mdio_priv = bus->priv;
>	mdio_priv->hw = pf->si->hw;
>
>
>static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
>                            u16 value)
>{
>	struct enetc_mdio_priv *mdio_priv = bus->priv;
>...
>	enet_mdio_wr(priv->hw, ENETC_MDIO_CFG, mdio_cfg);
>}
>
>All the horrible casts go away, the driver is structured like every
>other driver, sparse is probably happy, etc.
>

This looks more like a matter cosmetic preferences.  I mean, I didn't
notice anything "horrible" in the code so far.  I actually find it more
ugly to define a new structure with only one element inside, like:
struct enetc_mdio_priv {
       struct enetc_hw *hw;
}
What is this technique called? Looks like a second type definition for
another type.
Anyway, if others already did this in the kernel, what can I do?


^ permalink raw reply

* Re: [PATCH] ip6_gre: reload ipv6h in prepare_ip6gre_xmit_ipv6
From: William Tu @ 2019-07-24 15:44 UTC (permalink / raw)
  To: Haishuang Yan; +Cc: David S. Miller, Alexey Kuznetsov, netdev, linux-kernel
In-Reply-To: <1563969642-11843-1-git-send-email-yanhaishuang@cmss.chinamobile.com>

On Wed, Jul 24, 2019 at 08:00:42PM +0800, Haishuang Yan wrote:
> Since ip6_tnl_parse_tlv_enc_lim() can call pskb_may_pull()
> which may change skb->data, so we need to re-load ipv6h at
> the right place.
> 
> Fixes: 898b29798e36 ("ip6_gre: Refactor ip6gre xmit codes")
> Cc: William Tu <u9012063@gmail.com>
> Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>

LGTM, thanks for the fix
Acked-by: William Tu <u9012063@gmail.com>

> ---
>  net/ipv6/ip6_gre.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index c2049c7..dd2d0b96 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -660,12 +660,13 @@ static int prepare_ip6gre_xmit_ipv6(struct sk_buff *skb,
>  				    struct flowi6 *fl6, __u8 *dsfield,
>  				    int *encap_limit)
>  {
> -	struct ipv6hdr *ipv6h = ipv6_hdr(skb);
> +	struct ipv6hdr *ipv6h;
>  	struct ip6_tnl *t = netdev_priv(dev);
>  	__u16 offset;
>  
>  	offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb));
>  	/* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */
> +	ipv6h = ipv6_hdr(skb);
>  
>  	if (offset > 0) {
>  		struct ipv6_tlv_tnl_enc_lim *tel;
> -- 
> 1.8.3.1
> 
> 
> 

^ permalink raw reply

* Re: [PATCH net-next] selftests: mlxsw: Fix typo in qos_mc_aware.sh
From: Ido Schimmel @ 2019-07-24 15:39 UTC (permalink / raw)
  To: Masanari Iida
  Cc: shuah@kernel.org, linux-kernel@vger.kernel.org, Jiri Pirko,
	linux-kselftest@vger.kernel.org, rdunlap@infradead.org,
	netdev@vger.kernel.org
In-Reply-To: <20190724152951.4618-1-standby24x7@gmail.com>

On Thu, Jul 25, 2019 at 12:29:51AM +0900, Masanari Iida wrote:
> This patch fix some spelling typo in qos_mc_aware.sh
> 
> Signed-off-by: Masanari Iida <standby24x7@gmail.com>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>

Reviewed-by: Ido Schimmel <idosch@mellanox.com>

^ permalink raw reply

* Re: [PATCH -next] iwlwifi: dbg: work around clang bug by marking debug strings static
From: Sedat Dilek @ 2019-07-24 15:38 UTC (permalink / raw)
  To: Luca Coelho
  Cc: Nick Desaulniers, Joe Perches, Kalle Valo, Arnd Bergmann,
	Nathan Chancellor, Johannes Berg, Emmanuel Grumbach,
	Intel Linux Wireless, David S. Miller, Shahar S Matityahu,
	Sara Sharon, linux-wireless, netdev, LKML, clang-built-linux
In-Reply-To: <da053a97d771eff0ad8db37e644108ed2fad25a3.camel@coelho.fi>

On Tue, Jul 16, 2019 at 11:15 PM Luca Coelho <luca@coelho.fi> wrote:
>
> On Tue, 2019-07-16 at 10:28 -0700, Nick Desaulniers wrote:
> > On Thu, Jul 11, 2019 at 7:15 PM Joe Perches <joe@perches.com> wrote:
> > > On Thu, 2019-07-11 at 17:17 -0700, Nick Desaulniers wrote:
> > > > Commit r353569 in prerelease Clang-9 is producing a linkage failure:
> > > >
> > > > ld: drivers/net/wireless/intel/iwlwifi/fw/dbg.o:
> > > > in function `_iwl_fw_dbg_apply_point':
> > > > dbg.c:(.text+0x827a): undefined reference to `__compiletime_assert_2387'
> > > >
> > > > when the following configs are enabled:
> > > > - CONFIG_IWLWIFI
> > > > - CONFIG_IWLMVM
> > > > - CONFIG_KASAN
> > > >
> > > > Work around the issue for now by marking the debug strings as `static`,
> > > > which they probably should be any ways.
> > > >
> > > > Link: https://bugs.llvm.org/show_bug.cgi?id=42580
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/580
> > > > Reported-by: Arnd Bergmann <arnd@arndb.de>
> > > > Reported-by: Nathan Chancellor <natechancellor@gmail.com>
> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > ---
> > > >  drivers/net/wireless/intel/iwlwifi/fw/dbg.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
> > > > index e411ac98290d..f8c90ea4e9b4 100644
> > > > --- a/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
> > > > +++ b/drivers/net/wireless/intel/iwlwifi/fw/dbg.c
> > > > @@ -2438,7 +2438,7 @@ static void iwl_fw_dbg_info_apply(struct iwl_fw_runtime *fwrt,
> > > >  {
> > > >       u32 img_name_len = le32_to_cpu(dbg_info->img_name_len);
> > > >       u32 dbg_cfg_name_len = le32_to_cpu(dbg_info->dbg_cfg_name_len);
> > > > -     const char err_str[] =
> > > > +     static const char err_str[] =
> > > >               "WRT: ext=%d. Invalid %s name length %d, expected %d\n";
> > >
> > > Better still would be to use the format string directly
> > > in both locations instead of trying to deduplicate it
> > > via storing it into a separate pointer.
> > >
> > > Let the compiler/linker consolidate the format.
> > > It's smaller object code, allows format/argument verification,
> > > and is simpler for humans to understand.
> >
> > Whichever Kalle prefers, I just want my CI green again.
>
> We already changed this in a later internal patch, which will reach the
> mainline (-next) soon.  So let's skip this for now.
>

Do you have a link to your internal Git or patchwork for this?

I am interested in testing it.

- Sedat -

^ permalink raw reply

* Re: [PATCH v12 1/5] can: m_can: Create a m_can platform framework
From: Dan Murphy @ 2019-07-24 15:36 UTC (permalink / raw)
  To: Greg KH; +Cc: wg, mkl, davem, linux-can, netdev, linux-kernel
In-Reply-To: <20190724064754.GC22447@kroah.com>

Hello

On 7/24/19 1:47 AM, Greg KH wrote:
> On Tue, Jul 23, 2019 at 10:14:14AM -0500, Dan Murphy wrote:
>> Hello
>>
>> On 7/10/19 7:08 AM, Dan Murphy wrote:
>>> Hello
>>>
>>> On 6/17/19 10:09 AM, Dan Murphy wrote:
>>>> Marc
>>>>
>>>> On 6/10/19 11:35 AM, Dan Murphy wrote:
>>>>> Bump
>>>>>
>>>>> On 6/6/19 8:16 AM, Dan Murphy wrote:
>>>>>> Marc
>>>>>>
>>>>>> Bump
>>>>>>
>>>>>> On 5/31/19 6:51 AM, Dan Murphy wrote:
>>>>>>> Marc
>>>>>>>
>>>>>>> On 5/15/19 3:54 PM, Dan Murphy wrote:
>>>>>>>> Marc
>>>>>>>>
>>>>>>>> On 5/9/19 11:11 AM, Dan Murphy wrote:
>>>>>>>>> Create a m_can platform framework that peripheral
>>>>>>>>> devices can register to and use common code and register sets.
>>>>>>>>> The peripheral devices may provide read/write and configuration
>>>>>>>>> support of the IP.
>>>>>>>>>
>>>>>>>>> Acked-by: Wolfgang Grandegger <wg@grandegger.com>
>>>>>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> v12 - Update the m_can_read/write functions to
>>>>>>>>> create a backtrace if the callback
>>>>>>>>> pointer is NULL. - https://lore.kernel.org/patchwork/patch/1052302/
>>>>>>>>>
>>>>>>>> Is this able to be merged now?
>>>>>>> ping
>>>> Wondering if there is anything else we need to do?
>>>>
>>>> The part has officially shipped and we had hoped to have driver
>>>> support in Linux as part of the announcement.
>>>>
>>> Is this being sent in a PR for 5.3?
>>>
>>> Dan
>>>
>> Adding Greg to this thread as I have no idea what is going on with this.
> Why me?  What am I supposed to do here?  I see no patches at all to do
> anything with :(

I am not sure who to email. The maintainer seems to be on hiatus or 
super busy with other work.

So I added you to see if you know how to handle this.  Wolfgang Acked it 
but he said Marc needs to pull

it in.  We have quite a few users of this patchset. I have been hosting 
the patchset in a different tree.

These users keep pinging us for upstream status and all we can do is 
point them to the

LKML to show we are continuing to pursue inclusion.

https://lore.kernel.org/patchwork/project/lkml/list/?series=393454

Thanks
Dan


>
> thanks,
>
> greg "not a miracle worker" k-h

^ permalink raw reply

* [PATCH] net: phy: mscc: initialize stats array
From: Andreas Schwab @ 2019-07-24 15:32 UTC (permalink / raw)
  To: netdev
  Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
	linux-kernel

The memory allocated for the stats array may contain arbitrary data.

Signed-off-by: Andreas Schwab <schwab@suse.de>
---
 drivers/net/phy/mscc.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 28676af97b42..645d354ffb48 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -2226,8 +2226,8 @@ static int vsc8514_probe(struct phy_device *phydev)
 	vsc8531->supp_led_modes = VSC85XX_SUPP_LED_MODES;
 	vsc8531->hw_stats = vsc85xx_hw_stats;
 	vsc8531->nstats = ARRAY_SIZE(vsc85xx_hw_stats);
-	vsc8531->stats = devm_kmalloc_array(&phydev->mdio.dev, vsc8531->nstats,
-					    sizeof(u64), GFP_KERNEL);
+	vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats,
+				      sizeof(u64), GFP_KERNEL);
 	if (!vsc8531->stats)
 		return -ENOMEM;
 
@@ -2251,8 +2251,8 @@ static int vsc8574_probe(struct phy_device *phydev)
 	vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES;
 	vsc8531->hw_stats = vsc8584_hw_stats;
 	vsc8531->nstats = ARRAY_SIZE(vsc8584_hw_stats);
-	vsc8531->stats = devm_kmalloc_array(&phydev->mdio.dev, vsc8531->nstats,
-					    sizeof(u64), GFP_KERNEL);
+	vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats,
+				      sizeof(u64), GFP_KERNEL);
 	if (!vsc8531->stats)
 		return -ENOMEM;
 
@@ -2281,8 +2281,8 @@ static int vsc8584_probe(struct phy_device *phydev)
 	vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES;
 	vsc8531->hw_stats = vsc8584_hw_stats;
 	vsc8531->nstats = ARRAY_SIZE(vsc8584_hw_stats);
-	vsc8531->stats = devm_kmalloc_array(&phydev->mdio.dev, vsc8531->nstats,
-					    sizeof(u64), GFP_KERNEL);
+	vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats,
+				      sizeof(u64), GFP_KERNEL);
 	if (!vsc8531->stats)
 		return -ENOMEM;
 
@@ -2311,8 +2311,8 @@ static int vsc85xx_probe(struct phy_device *phydev)
 	vsc8531->supp_led_modes = VSC85XX_SUPP_LED_MODES;
 	vsc8531->hw_stats = vsc85xx_hw_stats;
 	vsc8531->nstats = ARRAY_SIZE(vsc85xx_hw_stats);
-	vsc8531->stats = devm_kmalloc_array(&phydev->mdio.dev, vsc8531->nstats,
-					    sizeof(u64), GFP_KERNEL);
+	vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats,
+				      sizeof(u64), GFP_KERNEL);
 	if (!vsc8531->stats)
 		return -ENOMEM;
 
-- 
2.22.0


-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply related

* [PATCH net-next] selftests: mlxsw: Fix typo in qos_mc_aware.sh
From: Masanari Iida @ 2019-07-24 15:29 UTC (permalink / raw)
  To: shuah, linux-kernel, jiri, idosch, linux-kselftest, rdunlap,
	netdev
  Cc: Masanari Iida

This patch fix some spelling typo in qos_mc_aware.sh

Signed-off-by: Masanari Iida <standby24x7@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
---
 tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh
index 71231ad2dbfb..47315fe48d5a 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh
@@ -262,7 +262,7 @@ test_mc_aware()
 
 	stop_traffic
 
-	log_test "UC performace under MC overload"
+	log_test "UC performance under MC overload"
 
 	echo "UC-only throughput  $(humanize $ucth1)"
 	echo "UC+MC throughput    $(humanize $ucth2)"
@@ -316,7 +316,7 @@ test_uc_aware()
 
 	stop_traffic
 
-	log_test "MC performace under UC overload"
+	log_test "MC performance under UC overload"
 	echo "    ingress UC throughput $(humanize ${uc_ir})"
 	echo "    egress UC throughput  $(humanize ${uc_er})"
 	echo "    sent $attempts BC ARPs, got $passes responses"
-- 
2.22.0.545.g9c9b961d7eb1


^ permalink raw reply related

* Re: [PATCH net-next v1 1/4] enetc: Clean up local mdio bus allocation
From: Andrew Lunn @ 2019-07-24 15:18 UTC (permalink / raw)
  To: Claudiu Manoil
  Cc: David S . Miller, Rob Herring, Li Yang, alexandru.marginean,
	netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1563979301-596-2-git-send-email-claudiu.manoil@nxp.com>

On Wed, Jul 24, 2019 at 05:41:38PM +0300, Claudiu Manoil wrote:
> Though it works, this is not how it should have been.
> What's needed is a pointer to the mdio registers.
> Store it properly inside bus->priv allocated space.
> Use devm_* variant to further clean up the init error /
> remove paths.
> 
> Fixes following sparse warning:
>  warning: incorrect type in assignment (different address spaces)
>     expected void *priv
>     got struct enetc_mdio_regs [noderef] <asn:2>*[assigned] regs
> 
> Fixes: ebfcb23d62ab ("enetc: Add ENETC PF level external MDIO support")
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> ---
> v1 - added this patch
> 
>  .../net/ethernet/freescale/enetc/enetc_mdio.c | 31 +++++++------------
>  1 file changed, 12 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
> index 77b9cd10ba2b..1e3cd21c13ee 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
> @@ -15,7 +15,8 @@ struct enetc_mdio_regs {
>  	u32	mdio_addr;	/* MDIO address */
>  };
>  
> -#define bus_to_enetc_regs(bus)	(struct enetc_mdio_regs __iomem *)((bus)->priv)
> +#define bus_to_enetc_regs(bus)	(*(struct enetc_mdio_regs __iomem **) \
> +				((bus)->priv))
>  
>  #define ENETC_MDIO_REG_OFFSET	0x1c00
>  #define ENETC_MDC_DIV		258
> @@ -146,12 +147,12 @@ static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
>  int enetc_mdio_probe(struct enetc_pf *pf)
>  {
>  	struct device *dev = &pf->si->pdev->dev;
> -	struct enetc_mdio_regs __iomem *regs;
> +	struct enetc_mdio_regs __iomem **regsp;
>  	struct device_node *np;
>  	struct mii_bus *bus;
> -	int ret;
> +	int err;
>  
> -	bus = mdiobus_alloc_size(sizeof(regs));
> +	bus = devm_mdiobus_alloc_size(dev, sizeof(*regsp));
>  	if (!bus)
>  		return -ENOMEM;
>  
> @@ -159,41 +160,33 @@ int enetc_mdio_probe(struct enetc_pf *pf)
>  	bus->read = enetc_mdio_read;
>  	bus->write = enetc_mdio_write;
>  	bus->parent = dev;
> +	regsp = bus->priv;
>  	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
>  
>  	/* store the enetc mdio base address for this bus */
> -	regs = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
> -	bus->priv = regs;
> +	*regsp = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;

This is all very odd and different to every other driver.

If i get the code write, there are 4 registers, each u32 in size,
starting at pf->si->hw.port + ENETC_MDIO_REG_OFFSET?

There are macros like enetc_port_wr() and enetc_global_wr(). It think
it would be much cleaner to add a macro enet_mdio_wr() which takes
hw, off, val.

#define enet_mdio_wr(hw, off, val) enet_port_wr(hw, off + ENETC_MDIO_REG_OFFSET, val)

struct enetc_mdio_priv {
       struct enetc_hw *hw;
}

	struct enetc_mdio_priv *mdio_priv;

	bus = devm_mdiobus_alloc_size(dev, sizeof(*mdio_priv));

	mdio_priv = bus->priv;
	mdio_priv->hw = pf->si->hw;


static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
                            u16 value)
{
	struct enetc_mdio_priv *mdio_priv = bus->priv;
...
	enet_mdio_wr(priv->hw, ENETC_MDIO_CFG, mdio_cfg);
}
			    	
All the horrible casts go away, the driver is structured like every
other driver, sparse is probably happy, etc.

      Andrew

^ permalink raw reply

* Re: [RFC PATCH net-next 00/12] drop_monitor: Capture dropped packets and metadata
From: Jiri Pirko @ 2019-07-24 15:15 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: netdev, davem, nhorman, dsahern, roopa, nikolay, jakub.kicinski,
	toke, andy, f.fainelli, andrew, vivien.didelot, mlxsw,
	Ido Schimmel
In-Reply-To: <20190722183134.14516-1-idosch@idosch.org>

Mon, Jul 22, 2019 at 08:31:22PM CEST, idosch@idosch.org wrote:
>From: Ido Schimmel <idosch@mellanox.com>
>
>So far drop monitor supported only one mode of operation in which a
>summary of recent packet drops is periodically sent to user space as a
>netlink event. The event only includes the drop location (program
>counter) and number of drops in the last interval.
>
>While this mode of operation allows one to understand if the system is
>dropping packets, it is not sufficient if a more detailed analysis is
>required. Both the packet itself and related metadata are missing.
>
>This patchset extends drop monitor with another mode of operation where
>the packet - potentially truncated - and metadata (e.g., drop location,
>timestamp, netdev) are sent to user space as a netlink event. Thanks to
>the extensible nature of netlink, more metadata can be added in the
>future.
>
>To avoid performing expensive operations in the context in which
>kfree_skb() is called, the dropped skbs are cloned and queued on per-CPU
>skb drop list. The list is then processed in process context (using a
>workqueue), where the netlink messages are allocated, prepared and
>finally sent to user space.
>
>As a follow-up, I plan to integrate drop monitor with devlink and allow
>the latter to call into drop monitor to report hardware drops. In the
>future, XDP drops can be added as well, thereby making drop monitor the
>go-to netlink channel for diagnosing all packet drops.
>
>Example usage with patched dropwatch [1] can be found here [2]. Example
>dissection of drop monitor netlink events with patched wireshark [3] can
>be found here [4]. I will submit both changes upstream after the kernel
>changes are accepted.
>
>Patches #1-#6 are just cleanups with no functional changes intended.
>
>Patches #7-#8 perform small refactoring before the actual changes are
>introduced in the last four patches.

In general, this looks very good to me. Thanks!

^ permalink raw reply

* [PATCH 0/2] Add the BroadMobi BM818 card
From: Angus Ainslie (Purism) @ 2019-07-24 14:52 UTC (permalink / raw)
  To: kernel
  Cc: Bjørn Mork, David S. Miller, Johan Hovold,
	Greg Kroah-Hartman, netdev, linux-usb, linux-kernel,
	Angus Ainslie (Purism)

Add qmi_wwan and option support for the BroadMobi BM818

Bob Ham (2):
  usb: serial: option: Add the BroadMobi BM818 card
  net: usb: qmi_wwan: Add the BroadMobi BM818 card

 drivers/net/usb/qmi_wwan.c  | 1 +
 drivers/usb/serial/option.c | 2 ++
 2 files changed, 3 insertions(+)

-- 
2.17.1


^ permalink raw reply

* [PATCH 1/2] usb: serial: option: Add the BroadMobi BM818 card
From: Angus Ainslie (Purism) @ 2019-07-24 14:52 UTC (permalink / raw)
  To: kernel
  Cc: Bjørn Mork, David S. Miller, Johan Hovold,
	Greg Kroah-Hartman, netdev, linux-usb, linux-kernel, Bob Ham,
	Angus Ainslie
In-Reply-To: <20190724145227.27169-1-angus@akkea.ca>

From: Bob Ham <bob.ham@puri.sm>

Add a VID:PID for the BroadModi BM818 M.2 card

Signed-off-by: Bob Ham <bob.ham@puri.sm>
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 drivers/usb/serial/option.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index c1582fbd1150..674a68ee9564 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1975,6 +1975,8 @@ static const struct usb_device_id option_ids[] = {
 	  .driver_info = RSVD(4) | RSVD(5) },
 	{ USB_DEVICE_INTERFACE_CLASS(0x2cb7, 0x0105, 0xff),			/* Fibocom NL678 series */
 	  .driver_info = RSVD(6) },
+	{ USB_DEVICE(0x2020, 0x2060),						/* BroadMobi  */
+	  .driver_info = RSVD(4) },
 	{ } /* Terminating entry */
 };
 MODULE_DEVICE_TABLE(usb, option_ids);
-- 
2.17.1


^ permalink raw reply related

* [PATCH 2/2] net: usb: qmi_wwan: Add the BroadMobi BM818 card
From: Angus Ainslie (Purism) @ 2019-07-24 14:52 UTC (permalink / raw)
  To: kernel
  Cc: Bjørn Mork, David S. Miller, Johan Hovold,
	Greg Kroah-Hartman, netdev, linux-usb, linux-kernel, Bob Ham,
	Angus Ainslie
In-Reply-To: <20190724145227.27169-1-angus@akkea.ca>

From: Bob Ham <bob.ham@puri.sm>

The BroadMobi BM818 M.2 card uses the QMI protocol

Signed-off-by: Bob Ham <bob.ham@puri.sm>
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
 drivers/net/usb/qmi_wwan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 69e0a2acfcb0..b6dc5d714b5e 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1295,6 +1295,7 @@ static const struct usb_device_id products[] = {
 	{QMI_FIXED_INTF(0x2001, 0x7e3d, 4)},	/* D-Link DWM-222 A2 */
 	{QMI_FIXED_INTF(0x2020, 0x2031, 4)},	/* Olicard 600 */
 	{QMI_FIXED_INTF(0x2020, 0x2033, 4)},	/* BroadMobi BM806U */
+	{QMI_FIXED_INTF(0x2020, 0x2060, 4)},	/* BroadMobi BM818 */
 	{QMI_FIXED_INTF(0x0f3d, 0x68a2, 8)},    /* Sierra Wireless MC7700 */
 	{QMI_FIXED_INTF(0x114f, 0x68a2, 8)},    /* Sierra Wireless MC7750 */
 	{QMI_FIXED_INTF(0x1199, 0x68a2, 8)},	/* Sierra Wireless MC7710 in QMI mode */
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v1 0/4] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Claudiu Manoil @ 2019-07-24 14:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
	devicetree, linux-arm-kernel, linux-kernel

Second patch just registers the PCIe endpoint device containing
the MDIO registers as a standalone MDIO bus driver, to allow
an alternative way to control the MDIO bus.  The same code used
by the ENETC ports (eth controllers) to manage MDIO via local
registers applies and is reused.

Bindings are provided for the new MDIO node, similarly to ENETC
port nodes bindings.

Last patch enables the ENETC port 1 and its RGMII PHY on the
LS1028A QDS board, where the MDIO muxing configuration relies
on the MDIO support provided in the first patch.

Claudiu Manoil (4):
  enetc: Clean up local mdio bus allocation
  enetc: Add mdio bus driver for the PCIe MDIO endpoint
  dt-bindings: net: fsl: enetc: Add bindings for the central MDIO PCIe
    endpoint
  arm64: dts: fsl: ls1028a: Enable eth port1 on the ls1028a QDS board

 .../devicetree/bindings/net/fsl-enetc.txt     |  42 ++++++-
 .../boot/dts/freescale/fsl-ls1028a-qds.dts    |  40 ++++++
 .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi |   6 +
 .../net/ethernet/freescale/enetc/enetc_mdio.c | 119 +++++++++++++++---
 .../net/ethernet/freescale/enetc/enetc_pf.c   |   5 +-
 5 files changed, 190 insertions(+), 22 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH net-next v1 3/4] dt-bindings: net: fsl: enetc: Add bindings for the central MDIO PCIe endpoint
From: Claudiu Manoil @ 2019-07-24 14:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
	devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1563979301-596-1-git-send-email-claudiu.manoil@nxp.com>

The on-chip PCIe root complex that integrates the ENETC ethernet
controllers also integrates a PCIe enpoint for the MDIO controller
provinding for cetralized control of the ENETC mdio bus.
Add bindings for this "central" MDIO Integrated PCIe Endpoit.

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v1 - none

 .../devicetree/bindings/net/fsl-enetc.txt     | 42 +++++++++++++++++--
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-enetc.txt b/Documentation/devicetree/bindings/net/fsl-enetc.txt
index 25fc687419db..c090f6df7a39 100644
--- a/Documentation/devicetree/bindings/net/fsl-enetc.txt
+++ b/Documentation/devicetree/bindings/net/fsl-enetc.txt
@@ -11,7 +11,9 @@ Required properties:
 		  to parent node bindings.
 - compatible	: Should be "fsl,enetc".
 
-1) The ENETC external port is connected to a MDIO configurable phy:
+1. The ENETC external port is connected to a MDIO configurable phy
+
+1.1. Using the local ENETC Port MDIO interface
 
 In this case, the ENETC node should include a "mdio" sub-node
 that in turn should contain the "ethernet-phy" node describing the
@@ -47,8 +49,42 @@ Example:
 		};
 	};
 
-2) The ENETC port is an internal port or has a fixed-link external
-connection:
+1.2. Using the central MDIO PCIe enpoint device
+
+In this case, the mdio node should be defined as another PCIe
+endpoint node, at the same level with the ENETC port nodes.
+
+Required properties:
+
+- reg		: Specifies PCIe Device Number and Function
+		  Number of the ENETC endpoint device, according
+		  to parent node bindings.
+- compatible	: Should be "fsl,enetc-mdio".
+
+The remaining required mdio bus properties are standard, their bindings
+already defined in Documentation/devicetree/bindings/net/mdio.txt.
+
+Example:
+
+	ethernet@0,0 {
+		compatible = "fsl,enetc";
+		reg = <0x000000 0 0 0 0>;
+		phy-handle = <&sgmii_phy0>;
+		phy-connection-type = "sgmii";
+	};
+
+	mdio@0,3 {
+		compatible = "fsl,enetc-mdio";
+		reg = <0x000300 0 0 0 0>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		sgmii_phy0: ethernet-phy@2 {
+			reg = <0x2>;
+		};
+	};
+
+2. The ENETC port is an internal port or has a fixed-link external
+connection
 
 In this case, the ENETC port node defines a fixed link connection,
 as specified by Documentation/devicetree/bindings/net/fixed-link.txt.
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v1 2/4] enetc: Add mdio bus driver for the PCIe MDIO endpoint
From: Claudiu Manoil @ 2019-07-24 14:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
	devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1563979301-596-1-git-send-email-claudiu.manoil@nxp.com>

ENETC ports can manage the MDIO bus via local register
interface.  However there's also a centralized way
to manage the MDIO bus, via the MDIO PCIe endpoint
device integrated by the same root complex that also
integrates the ENETC ports (eth controllers).

Depending on board design and use case, centralized
access to MDIO may be better than using local ENETC
port registers.  For instance, on the LS1028A QDS board
where MDIO muxing is requiered.  Also, the LS1028A on-chip
switch doesn't have a local MDIO register interface.

The current patch registers the above PCIe enpoint as a
separate MDIO bus and provides a driver for it by re-using
the code used for local MDIO access.  It also allows the
ENETC port PHYs to be managed by this driver if the local
"mdio" node is missing from the ENETC port node.

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v1 - fixed mdio bus allocation
   - requested only BAR0 region, as it's the only one used by the driver

 .../net/ethernet/freescale/enetc/enetc_mdio.c | 90 +++++++++++++++++++
 .../net/ethernet/freescale/enetc/enetc_pf.c   |  5 +-
 2 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index 1e3cd21c13ee..378cc8dd27f9 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -190,3 +190,93 @@ void enetc_mdio_remove(struct enetc_pf *pf)
 	if (pf->mdio)
 		mdiobus_unregister(pf->mdio);
 }
+
+#define ENETC_MDIO_DEV_ID	0xee01
+#define ENETC_MDIO_DEV_NAME	"FSL PCIe IE Central MDIO"
+#define ENETC_MDIO_BUS_NAME	ENETC_MDIO_DEV_NAME " Bus"
+#define ENETC_MDIO_DRV_NAME	ENETC_MDIO_DEV_NAME " driver"
+#define ENETC_MDIO_DRV_ID	"fsl_enetc_mdio"
+
+static int enetc_pci_mdio_probe(struct pci_dev *pdev,
+				const struct pci_device_id *ent)
+{
+	struct enetc_mdio_regs __iomem **regsp;
+	struct device *dev = &pdev->dev;
+	struct mii_bus *bus;
+	int err;
+
+	bus = devm_mdiobus_alloc_size(dev, sizeof(*regsp));
+	if (!bus)
+		return -ENOMEM;
+
+	bus->name = ENETC_MDIO_BUS_NAME;
+	bus->read = enetc_mdio_read;
+	bus->write = enetc_mdio_write;
+	bus->parent = dev;
+	regsp = bus->priv;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+
+	pcie_flr(pdev);
+	err = pci_enable_device_mem(pdev);
+	if (err) {
+		dev_err(dev, "device enable failed\n");
+		return err;
+	}
+
+	err = pci_request_region(pdev, 0, ENETC_MDIO_DRV_ID);
+	if (err) {
+		dev_err(dev, "pci_request_region failed\n");
+		goto err_pci_mem_reg;
+	}
+
+	*regsp = pci_iomap_range(pdev, 0, ENETC_MDIO_REG_OFFSET, 0);
+	if (!bus->priv) {
+		err = -ENXIO;
+		dev_err(dev, "iomap failed\n");
+		goto err_ioremap;
+	}
+
+	err = of_mdiobus_register(bus, dev->of_node);
+	if (err)
+		goto err_mdiobus_reg;
+
+	pci_set_drvdata(pdev, bus);
+
+	return 0;
+
+err_mdiobus_reg:
+	iounmap(*regsp);
+err_ioremap:
+	pci_release_mem_regions(pdev);
+err_pci_mem_reg:
+	pci_disable_device(pdev);
+
+	return err;
+}
+
+static void enetc_pci_mdio_remove(struct pci_dev *pdev)
+{
+	struct mii_bus *bus = pci_get_drvdata(pdev);
+
+	mdiobus_unregister(bus);
+	iounmap(bus_to_enetc_regs(bus));
+	pci_release_mem_regions(pdev);
+	pci_disable_device(pdev);
+}
+
+static const struct pci_device_id enetc_pci_mdio_id_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, ENETC_MDIO_DEV_ID) },
+	{ 0, } /* End of table. */
+};
+MODULE_DEVICE_TABLE(pci, enetc_mdio_id_table);
+
+static struct pci_driver enetc_pci_mdio_driver = {
+	.name = ENETC_MDIO_DRV_ID,
+	.id_table = enetc_pci_mdio_id_table,
+	.probe = enetc_pci_mdio_probe,
+	.remove = enetc_pci_mdio_remove,
+};
+module_pci_driver(enetc_pci_mdio_driver);
+
+MODULE_DESCRIPTION(ENETC_MDIO_DRV_NAME);
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 258b3cb38a6f..7d6513ff8507 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -750,6 +750,7 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
 {
 	struct enetc_pf *pf = enetc_si_priv(priv->si);
 	struct device_node *np = priv->dev->of_node;
+	struct device_node *mdio_np;
 	int err;
 
 	if (!np) {
@@ -773,7 +774,9 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
 		priv->phy_node = of_node_get(np);
 	}
 
-	if (!of_phy_is_fixed_link(np)) {
+	mdio_np = of_get_child_by_name(np, "mdio");
+	if (mdio_np) {
+		of_node_put(mdio_np);
 		err = enetc_mdio_probe(pf);
 		if (err) {
 			of_node_put(priv->phy_node);
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v1 1/4] enetc: Clean up local mdio bus allocation
From: Claudiu Manoil @ 2019-07-24 14:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
	devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1563979301-596-1-git-send-email-claudiu.manoil@nxp.com>

Though it works, this is not how it should have been.
What's needed is a pointer to the mdio registers.
Store it properly inside bus->priv allocated space.
Use devm_* variant to further clean up the init error /
remove paths.

Fixes following sparse warning:
 warning: incorrect type in assignment (different address spaces)
    expected void *priv
    got struct enetc_mdio_regs [noderef] <asn:2>*[assigned] regs

Fixes: ebfcb23d62ab ("enetc: Add ENETC PF level external MDIO support")

Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v1 - added this patch

 .../net/ethernet/freescale/enetc/enetc_mdio.c | 31 +++++++------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
index 77b9cd10ba2b..1e3cd21c13ee 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -15,7 +15,8 @@ struct enetc_mdio_regs {
 	u32	mdio_addr;	/* MDIO address */
 };
 
-#define bus_to_enetc_regs(bus)	(struct enetc_mdio_regs __iomem *)((bus)->priv)
+#define bus_to_enetc_regs(bus)	(*(struct enetc_mdio_regs __iomem **) \
+				((bus)->priv))
 
 #define ENETC_MDIO_REG_OFFSET	0x1c00
 #define ENETC_MDC_DIV		258
@@ -146,12 +147,12 @@ static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
 int enetc_mdio_probe(struct enetc_pf *pf)
 {
 	struct device *dev = &pf->si->pdev->dev;
-	struct enetc_mdio_regs __iomem *regs;
+	struct enetc_mdio_regs __iomem **regsp;
 	struct device_node *np;
 	struct mii_bus *bus;
-	int ret;
+	int err;
 
-	bus = mdiobus_alloc_size(sizeof(regs));
+	bus = devm_mdiobus_alloc_size(dev, sizeof(*regsp));
 	if (!bus)
 		return -ENOMEM;
 
@@ -159,41 +160,33 @@ int enetc_mdio_probe(struct enetc_pf *pf)
 	bus->read = enetc_mdio_read;
 	bus->write = enetc_mdio_write;
 	bus->parent = dev;
+	regsp = bus->priv;
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
 
 	/* store the enetc mdio base address for this bus */
-	regs = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
-	bus->priv = regs;
+	*regsp = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
 
 	np = of_get_child_by_name(dev->of_node, "mdio");
 	if (!np) {
 		dev_err(dev, "MDIO node missing\n");
-		ret = -EINVAL;
-		goto err_registration;
+		return -EINVAL;
 	}
 
-	ret = of_mdiobus_register(bus, np);
-	if (ret) {
+	err = of_mdiobus_register(bus, np);
+	if (err) {
 		of_node_put(np);
 		dev_err(dev, "cannot register MDIO bus\n");
-		goto err_registration;
+		return err;
 	}
 
 	of_node_put(np);
 	pf->mdio = bus;
 
 	return 0;
-
-err_registration:
-	mdiobus_free(bus);
-
-	return ret;
 }
 
 void enetc_mdio_remove(struct enetc_pf *pf)
 {
-	if (pf->mdio) {
+	if (pf->mdio)
 		mdiobus_unregister(pf->mdio);
-		mdiobus_free(pf->mdio);
-	}
 }
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v1 4/4] arm64: dts: fsl: ls1028a: Enable eth port1 on the ls1028a QDS board
From: Claudiu Manoil @ 2019-07-24 14:41 UTC (permalink / raw)
  To: David S . Miller
  Cc: andrew, Rob Herring, Li Yang, alexandru.marginean, netdev,
	devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <1563979301-596-1-git-send-email-claudiu.manoil@nxp.com>

LS1028a has one Ethernet management interface. On the QDS board, the
MDIO signals are multiplexed to either on-board AR8035 PHY device or
to 4 PCIe slots allowing for SGMII cards.
To enable the Ethernet ENETC Port 1, which can only be connected to a
RGMII PHY, the multiplexer needs to be configured to route the MDIO to
the AR8035 PHY.  The MDIO/MDC routing is controlled by bits 7:4 of FPGA
board config register 0x54, and value 0 selects the on-board RGMII PHY.
The FPGA board config registers are accessible on the i2c bus, at address
0x66.

The PF3 MDIO PCIe integrated endpoint device allows for centralized access
to the MDIO bus.  Add the corresponding devicetree node and set it to be
the MDIO bus parent.

Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v1 - none

 .../boot/dts/freescale/fsl-ls1028a-qds.dts    | 40 +++++++++++++++++++
 .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi |  6 +++
 2 files changed, 46 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index de6ef39f3118..663c4b728c07 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -85,6 +85,26 @@
 			system-clock-frequency = <25000000>;
 		};
 	};
+
+	mdio-mux {
+		compatible = "mdio-mux-multiplexer";
+		mux-controls = <&mux 0>;
+		mdio-parent-bus = <&enetc_mdio_pf3>;
+		#address-cells=<1>;
+		#size-cells = <0>;
+
+		/* on-board RGMII PHY */
+		mdio@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+
+			qds_phy1: ethernet-phy@5 {
+				/* Atheros 8035 */
+				reg = <5>;
+			};
+		};
+	};
 };
 
 &duart0 {
@@ -164,6 +184,26 @@
 			};
 		};
 	};
+
+	fpga@66 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "fsl,ls1028aqds-fpga", "fsl,fpga-qixis-i2c",
+			     "simple-mfd";
+		reg = <0x66>;
+
+		mux: mux-controller {
+			compatible = "reg-mux";
+			#mux-control-cells = <1>;
+			mux-reg-masks = <0x54 0xf0>; /* 0: reg 0x54, bits 7:4 */
+		};
+	};
+
+};
+
+&enetc_port1 {
+	phy-handle = <&qds_phy1>;
+	phy-connection-type = "rgmii-id";
 };
 
 &sai1 {
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 7975519b4f56..de71153fda00 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -536,6 +536,12 @@
 				compatible = "fsl,enetc";
 				reg = <0x000100 0 0 0 0>;
 			};
+			enetc_mdio_pf3: mdio@0,3 {
+				compatible = "fsl,enetc-mdio";
+				reg = <0x000300 0 0 0 0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
 			ethernet@0,4 {
 				compatible = "fsl,enetc-ptp";
 				reg = <0x000400 0 0 0 0>;
-- 
2.17.1


^ permalink raw reply related

* [PATCH] netfilter: trivial: remove extraneous space from message
From: Rui Salvaterra @ 2019-07-24 14:37 UTC (permalink / raw)
  To: pablo, davem, netfilter-devel, netdev, linux-kernel; +Cc: Rui Salvaterra

Pure ocd, but this one has been bugging me for a while.

Signed-off-by: Rui Salvaterra <rsalvaterra@gmail.com>
---
 net/netfilter/nf_conntrack_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 8d729e7c36ff..209123f35b4a 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -218,7 +218,7 @@ nf_ct_lookup_helper(struct nf_conn *ct, struct net *net)
 			return NULL;
 		pr_info("nf_conntrack: default automatic helper assignment "
 			"has been turned off for security reasons and CT-based "
-			" firewall rule not found. Use the iptables CT target "
+			"firewall rule not found. Use the iptables CT target "
 			"to attach helpers instead.\n");
 		net->ct.auto_assign_helper_warned = 1;
 		return NULL;
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH net-next 0/4] sctp: clean up __sctp_connect function
From: Marcelo Ricardo Leitner @ 2019-07-24 14:25 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
In-Reply-To: <cover.1563817029.git.lucien.xin@gmail.com>

On Tue, Jul 23, 2019 at 01:37:56AM +0800, Xin Long wrote:
> This patchset is to factor out some common code for
> sctp_sendmsg_new_asoc() and __sctp_connect() into 2
> new functioins.
> 
> Xin Long (4):
>   sctp: check addr_size with sa_family_t size in
>     __sctp_setsockopt_connectx
>   sctp: clean up __sctp_connect
>   sctp: factor out sctp_connect_new_asoc
>   sctp: factor out sctp_connect_add_peer

Nice cleanup! These patches LGTM. Hopefully for Neil as well.

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox