* Re: [PATCH 03/11] xdp: add offset param to zero_copy_allocator
From: Laatz, Kevin @ 2019-06-25 13:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, ast, daniel, bjorn.topel, magnus.karlsson, bpf,
intel-wired-lan, bruce.richardson, ciara.loftus
In-Reply-To: <20190624122342.26c6a9b4@cakuba.netronome.com>
On 24/06/2019 20:23, Jakub Kicinski wrote:
> On Thu, 20 Jun 2019 09:09:50 +0000, Kevin Laatz wrote:
>> diff --git a/include/net/xdp.h b/include/net/xdp.h
>> index 0f25b3675c5c..ea801fd2bf98 100644
>> --- a/include/net/xdp.h
>> +++ b/include/net/xdp.h
>> @@ -53,7 +53,8 @@ struct xdp_mem_info {
>> struct page_pool;
>>
>> struct zero_copy_allocator {
>> - void (*free)(struct zero_copy_allocator *zca, unsigned long
>> handle);
>> + void (*free)(struct zero_copy_allocator *zca, unsigned long handle,
>> + off_t off);
>> };
>
> Please run checkpatch --strict on all your changes. The code
> formatting is incorrect in many ways in this series.
>
Thanks, will fix in the v2.
> Please include performance measurements proving the slow down
> is negligible in the cover letter.
>
Good suggestion. Will add to the cover letter in the v2!
^ permalink raw reply
* Re: [PATCH 00/11] XDP unaligned chunk placement support
From: Laatz, Kevin @ 2019-06-25 13:12 UTC (permalink / raw)
To: Björn Töpel
Cc: Netdev, Alexei Starovoitov, Daniel Borkmann,
Björn Töpel, Karlsson, Magnus, bpf, intel-wired-lan,
Bruce Richardson, ciara.loftus
In-Reply-To: <CAJ+HfNijp8BgMgiOuohiuqDPz+spAutdG34gUqKzepYo2noE-w@mail.gmail.com>
On 24/06/2019 16:38, Björn Töpel wrote:
> On Thu, 20 Jun 2019 at 18:55, Kevin Laatz <kevin.laatz@intel.com> wrote:
>>
>> This patchset adds the ability to use unaligned chunks in the XDP umem.
>>
>> Currently, all chunk addresses passed to the umem are masked to be chunk
>> size aligned (default is 2k, max is PAGE_SIZE). This limits where we can
>> place chunks within the umem as well as limiting the packet sizes
>> that are
>> supported.
>>
>> The changes in this patchset removes these restrictions, allowing XDP
>> to be
>> more flexible in where it can place a chunk within a umem. By
>> relaxing where
>> the chunks can be placed, it allows us to use an arbitrary buffer
>> size and
>> place that wherever we have a free address in the umem. These changes
>> add the
>> ability to support jumboframes and make it easy to integrate with other
>> existing frameworks that have their own memory management systems,
>> such as
>> DPDK.
>>
>
> Thanks for working on this, Kevin and Ciara!
>
> I have some minor comments on the series, but in general I think it's
> in good shape!
>
> For some reason the series was submitted twice (at least on my side)?
Apologies for the confusion... The first set had a typo in the bpf
mailing list address (.com vs .org). Will fix for the v2.
>
>
> Thanks,
> Björn
Thanks for reviewing. Will address your comments in the v2.
>
>> Structure of the patchset:
>> Patch 1:
>> - Remove unnecessary masking and headroom addition during zero-copy Rx
>> buffer recycling in i40e. This change is required in order for the
>> buffer recycling to work in the unaligned chunk mode.
>>
>> Patch 2:
>> - Remove unnecessary masking and headroom addition during
>> zero-copy Rx buffer recycling in ixgbe. This change is required in
>> order for the buffer recycling to work in the unaligned chunk mode.
>>
>> Patch 3:
>> - Adds an offset parameter to zero_copy_allocator. This change will
>> enable us to calculate the original handle in zca_free. This will be
>> required for unaligned chunk mode since we can't easily mask back to
>> the original handle.
>>
>> Patch 4:
>> - Adds the offset parameter to i40e_zca_free. This change is needed
>> for
>> calculating the handle since we can't easily mask back to the
>> original
>> handle like we can in the aligned case.
>>
>> Patch 5:
>> - Adds the offset parameter to ixgbe_zca_free. This change is
>> needed for
>> calculating the handle since we can't easily mask back to the
>> original
>> handle like we can in the aligned case.
>>
>>
>> Patch 6:
>> - Add infrastructure for unaligned chunks. Since we are dealing
>> with unaligned chunks that could potentially cross a physical page
>> boundary, we add checks to keep track of that information. We can
>> later use this information to correctly handle buffers that are
>> placed at an address where they cross a page boundary.
>>
>> Patch 7:
>> - Add flags for umem configuration to libbpf
>>
>> Patch 8:
>> - Modify xdpsock application to add a command line option for
>> unaligned chunks
>>
>> Patch 9:
>> - Addition of command line argument to pass in a desired buffer size
>> and buffer recycling for unaligned mode. Passing in a buffer size
>> will
>> allow the application to use unaligned chunks with the unaligned
>> chunk
>> mode. Since we are now using unaligned chunks, we need to recycle
>> our
>> buffers in a slightly different way.
>>
>> Patch 10:
>> - Adds hugepage support to the xdpsock application
>>
>> Patch 11:
>> - Documentation update to include the unaligned chunk scenario. We
>> need
>> to explicitly state that the incoming addresses are only masked
>> in the
>> aligned chunk mode and not the unaligned chunk mode.
>>
>> Kevin Laatz (11):
>> i40e: simplify Rx buffer recycle
>> ixgbe: simplify Rx buffer recycle
>> xdp: add offset param to zero_copy_allocator
>> i40e: add offset to zca_free
>> ixgbe: add offset to zca_free
>> xsk: add support to allow unaligned chunk placement
>> libbpf: add flags to umem config
>> samples/bpf: add unaligned chunks mode support to xdpsock
>> samples/bpf: add buffer recycling for unaligned chunks to xdpsock
>> samples/bpf: use hugepages in xdpsock app
>> doc/af_xdp: include unaligned chunk case
>>
>> Documentation/networking/af_xdp.rst | 10 +-
>> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 21 ++--
>> drivers/net/ethernet/intel/i40e/i40e_xsk.h | 3 +-
>> .../ethernet/intel/ixgbe/ixgbe_txrx_common.h | 3 +-
>> drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 21 ++--
>> include/net/xdp.h | 3 +-
>> include/net/xdp_sock.h | 2 +
>> include/uapi/linux/if_xdp.h | 4 +
>> net/core/xdp.c | 11 ++-
>> net/xdp/xdp_umem.c | 17 ++--
>> net/xdp/xsk.c | 60 +++++++++--
>> net/xdp/xsk_queue.h | 60 +++++++++--
>> samples/bpf/xdpsock_user.c | 99 ++++++++++++++-----
>> tools/include/uapi/linux/if_xdp.h | 4 +
>> tools/lib/bpf/xsk.c | 7 ++
>> tools/lib/bpf/xsk.h | 2 +
>> 16 files changed, 241 insertions(+), 86 deletions(-)
>>
>> --
>> 2.17.1
>>
>
^ permalink raw reply
* RE: [PATCH net-next] net: stmmac: Fix the case when PHY handle is not present
From: Jose Abreu @ 2019-06-25 13:11 UTC (permalink / raw)
To: Jose Abreu, linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
Katsuhiro Suzuki
Cc: Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <351cce38d1c572d8b171044f2856c7fae9f89cbc.1561450696.git.joabreu@synopsys.com>
++ Katsuhiro
From: Jose Abreu <joabreu@synopsys.com>
> Some DT bindings do not have the PHY handle. Let's fallback to manually
> discovery in case phylink_of_phy_connect() fails.
>
> Reported-by: Katsuhiro Suzuki <katsuhiro@katsuster.net>
> Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> ---
> Hello Katsuhiro,
>
> Can you please test this patch ?
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index a48751989fa6..f4593d2d9d20 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -950,9 +950,12 @@ static int stmmac_init_phy(struct net_device *dev)
>
> node = priv->plat->phylink_node;
>
> - if (node) {
> + if (node)
> ret = phylink_of_phy_connect(priv->phylink, node, 0);
> - } else {
> +
> + /* Some DT bindings do not set-up the PHY handle. Let's try to
> + * manually parse it */
> + if (!node || ret) {
> int addr = priv->plat->phy_addr;
> struct phy_device *phydev;
>
> --
> 2.7.4
^ permalink raw reply
* Warnings generated from tcp_sacktag_write_queue.
From: Chinmay Agarwal @ 2019-06-25 13:07 UTC (permalink / raw)
To: netdev; +Cc: sharathv, kapandey
Dear All,
We are hitting the following WARN_ON condition:
WARN_ON((int)tcp_packets_in_flight(tp) < 0);
tcp_packets_in_flight = packets_out –( lost_out +
sacked_out ) + retrans_out (This value is coming -ve)
The tcp socket being used is in fin_wait_1 state.
The values for variables just before the crash:
packets_out = 0,
retrans_out = 28,
lost_out = 38,
sacked_out = 8
The only place I can find the packets_out value being set as 0 is:
void tcp_write_queue_purge(struct sock *sk)
{
...
tcp_sk(sk)->packets_out = 0;
inet_csk(sk)->icsk_backoff = 0;
}
Is there some code flow where packets_out can be set to 0 and other
values can remain unchanged?
If not, is there some scenario which may lead to "tcp_write_queue_purge"
called and not followed up by "tcp_clear_retrans"?
According to my understanding we should call "tcp_clear_retrans" after
setting packets_out to 0.
[ 1950.556150] Call trace:
[ 1950.558689] tcp_sacktag_write_queue+0x704/0x72c
[ 1950.561313] init: Untracked pid 10745 exited with status 0
[ 1950.563441] tcp_ack+0x3a4/0xd40
[ 1950.563447] tcp_rcv_state_process+0x1e8/0xbbc
[ 1950.563457] tcp_v4_do_rcv+0x18c/0x1cc
[ 1950.563461] tcp_v4_rcv+0x84c/0x8a8
[ 1950.563471] ip_protocol_deliver_rcu+0xdc/0x190
[ 1950.563474] ip_local_deliver_finish+0x64/0x80
[ 1950.563479] ip_local_deliver+0xc4/0xf8
[ 1950.563482] ip_rcv_finish+0x214/0x2e0
[ 1950.563486] ip_rcv+0x2fc/0x39c
[ 1950.563496] __netif_receive_skb_core+0x698/0x84c
[ 1950.563499] __netif_receive_skb+0x3c/0x7c
[ 1950.563503] process_backlog+0x98/0x148
[ 1950.563506] net_rx_action+0x128/0x388
[ 1950.563519] __do_softirq+0x20c/0x3f0
[ 1950.563528] irq_exit+0x9c/0xa8
[ 1950.563536] handle_IPI+0x174/0x278
[ 1950.563540] gic_handle_irq+0x124/0x1c0
[ 1950.563544] el1_irq+0xb4/0x12c
[ 1950.563556] lpm_cpuidle_enter+0x3f4/0x430
[ 1950.563561] cpuidle_enter_state+0x124/0x25c
[ 1950.563565] cpuidle_enter+0x30/0x40
[ 1950.563575] call_cpuidle+0x3c/0x60
[ 1950.563579] do_idle+0x190/0x228
[ 1950.563583] cpu_startup_entry+0x24/0x28
[ 1950.563588] secondary_start_kernel+0x12c/0x138
Thanks and Regards
Chinmay Agarwal
^ permalink raw reply
* Re: iwl_mvm_add_new_dqa_stream_wk BUG in lib/list_debug.c:56
From: Marc Haber @ 2019-06-25 13:03 UTC (permalink / raw)
To: linux-kernel, linux-wireless, netdev
In-Reply-To: <20190602134842.GC3249@torres.zugschlus.de>
On Sun, Jun 02, 2019 at 03:48:42PM +0200, Marc Haber wrote:
> On Thu, May 30, 2019 at 10:12:57AM +0200, Marc Haber wrote:
> > on my primary notebook, a Lenovo X260, with an Intel Wireless 8260
> > (8086:24f3), running Debian unstable, I have started to see network
> > hangs since upgrading to kernel 5.1. In this situation, I cannot
> > restart Network-Manager (the call just hangs), I can log out of X, but
> > the system does not cleanly shut down and I need to Magic SysRq myself
> > out of the running system. This happens about once every two days.
>
> The issue is also present in 5.1.5 and 5.1.6.
Almost a month later, 5.1.15 still crashes about twice a day on my
Notebook. The error message seems pretty clear to me, how can I go on
from there and may be identify a line number outside of a library?
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Leimen, Germany | lose things." Winona Ryder | Fon: *49 6224 1600402
Nordisch by Nature | How to make an American Quilt | Fax: *49 6224 1600421
^ permalink raw reply
* Re: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely
From: Guilherme Piccoli @ 2019-06-25 12:55 UTC (permalink / raw)
To: Sudarsana Reddy Kalluru
Cc: GR-everest-linux-l2, netdev@vger.kernel.org, Ariel Elior,
jay.vosburgh@canonical.com
In-Reply-To: <MN2PR18MB2528BCB89AC93EB791446BABD3E30@MN2PR18MB2528.namprd18.prod.outlook.com>
On Tue, Jun 25, 2019 at 1:02 AM Sudarsana Reddy Kalluru
<skalluru@marvell.com> wrote:
>
> Thanks for your changes and time on this. In general time-latching happens in couple or more milliseconds (even in some 100s of usec) under the normal traffic conditions. With this approach, there's a possibility that every packet has to wait for atleast 50ms for the timestamping. This in turn affects the wait-queue (of packets to be timestamped) at hardware as next TS recording happens only after the register is freed/read. And also, it incurs some latency for the ptp applications.
>
> PTP thread is consuming time may be due to the debug messages in this error path, which you are planning address already (thanks!!).
> "Also, I've dropped the PTP "outstanding, etc" messages to debug-level, they're quite flooding my log.
> Do you see cpu hog even after removing this message? In such case we may need to think of other alternatives such as sleep for 1 ms.
> Just for the info, the approach continuous-poll-for-timestamp() is used ixgbe driver (ixgbe_ptp_tx_hwtstamp_work()) as well.
>
Thanks again for the good insights Sudarsana! I'll do some experiments
dropping all messages and checking
if the ptp thread is still consuming a lot of CPU (I believe so). In
this case, I'll rework the approach by starting
the delays in 1ms to avoid impacting the HW wait-queue and causing
delays in ptp applications.
Cheers,
Guilherme
^ permalink raw reply
* [PATCH] can: mcp251x: add error check when wq alloc failed
From: Weitao Hou @ 2019-06-25 12:50 UTC (permalink / raw)
To: wg, mkl, davem, gregkh, allison, houweitaoo, tglx, sean
Cc: linux-can, netdev, linux-kernel
add error check when workqueue alloc failed, and remove
redundant code to make it clear
Signed-off-by: Weitao Hou <houweitaoo@gmail.com>
---
drivers/net/can/spi/mcp251x.c | 49 ++++++++++++++++-------------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
index 44e99e3d7134..2aec934fab0c 100644
--- a/drivers/net/can/spi/mcp251x.c
+++ b/drivers/net/can/spi/mcp251x.c
@@ -664,17 +664,6 @@ static int mcp251x_power_enable(struct regulator *reg, int enable)
return regulator_disable(reg);
}
-static void mcp251x_open_clean(struct net_device *net)
-{
- struct mcp251x_priv *priv = netdev_priv(net);
- struct spi_device *spi = priv->spi;
-
- free_irq(spi->irq, priv);
- mcp251x_hw_sleep(spi);
- mcp251x_power_enable(priv->transceiver, 0);
- close_candev(net);
-}
-
static int mcp251x_stop(struct net_device *net)
{
struct mcp251x_priv *priv = netdev_priv(net);
@@ -940,37 +929,43 @@ static int mcp251x_open(struct net_device *net)
flags | IRQF_ONESHOT, DEVICE_NAME, priv);
if (ret) {
dev_err(&spi->dev, "failed to acquire irq %d\n", spi->irq);
- mcp251x_power_enable(priv->transceiver, 0);
- close_candev(net);
- goto open_unlock;
+ goto out_close;
}
priv->wq = alloc_workqueue("mcp251x_wq", WQ_FREEZABLE | WQ_MEM_RECLAIM,
0);
+ if (!priv->wq) {
+ ret = -ENOMEM;
+ goto out_clean;
+ }
INIT_WORK(&priv->tx_work, mcp251x_tx_work_handler);
INIT_WORK(&priv->restart_work, mcp251x_restart_work_handler);
ret = mcp251x_hw_reset(spi);
- if (ret) {
- mcp251x_open_clean(net);
- goto open_unlock;
- }
+ if (ret)
+ goto out_free_wq;
ret = mcp251x_setup(net, spi);
- if (ret) {
- mcp251x_open_clean(net);
- goto open_unlock;
- }
+ if (ret)
+ goto out_free_wq;
ret = mcp251x_set_normal_mode(spi);
- if (ret) {
- mcp251x_open_clean(net);
- goto open_unlock;
- }
+ if (ret)
+ goto out_free_wq;
can_led_event(net, CAN_LED_EVENT_OPEN);
netif_wake_queue(net);
+ mutex_unlock(&priv->mcp_lock);
-open_unlock:
+ return 0;
+
+out_free_wq:
+ destroy_workqueue(priv->wq);
+out_clean:
+ free_irq(spi->irq, priv);
+ mcp251x_hw_sleep(spi);
+out_close:
+ mcp251x_power_enable(priv->transceiver, 0);
+ close_candev(net);
mutex_unlock(&priv->mcp_lock);
return ret;
}
--
2.18.0
^ permalink raw reply related
* Re: memory leak in sctp_get_port_local
From: Xin Long @ 2019-06-25 12:49 UTC (permalink / raw)
To: syzbot
Cc: davem, LKML, linux-sctp, Marcelo Ricardo Leitner, network dev,
Neil Horman, syzkaller-bugs, Vlad Yasevich
In-Reply-To: <00000000000069c3140589f6d3b7@google.com>
On Wed, May 29, 2019 at 2:28 AM syzbot
<syzbot+079bf326b38072f849d9@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: cd6c84d8 Linux 5.2-rc2
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=101a184aa00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=64479170dcaf0e11
> dashboard link: https://syzkaller.appspot.com/bug?extid=079bf326b38072f849d9
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13b5dbbca00000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1038444aa00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+079bf326b38072f849d9@syzkaller.appspotmail.com
>
> : Permanently added '10.128.0.127' (ECDSA) to the list of known hosts.
> executing program
> executing program
> BUG: memory leak
> unreferenced object 0xffff8881288ca380 (size 64):
> comm "softirq", pid 0, jiffies 4294944468 (age 13.410s)
> hex dump (first 32 bytes):
> 21 4e 00 00 00 00 00 00 00 00 00 00 00 00 00 00 !N..............
> 28 ae 85 23 81 88 ff ff 00 00 00 00 00 00 00 00 (..#............
> backtrace:
> [<0000000054ece54d>] kmemleak_alloc_recursive
> include/linux/kmemleak.h:55 [inline]
> [<0000000054ece54d>] slab_post_alloc_hook mm/slab.h:439 [inline]
> [<0000000054ece54d>] slab_alloc mm/slab.c:3326 [inline]
> [<0000000054ece54d>] kmem_cache_alloc+0x134/0x270 mm/slab.c:3488
> [<00000000d992ea84>] sctp_bucket_create net/sctp/socket.c:8395 [inline]
> [<00000000d992ea84>] sctp_get_port_local+0x189/0x5b0
> net/sctp/socket.c:8142
> [<0000000099206d90>] sctp_do_bind+0xcc/0x1e0 net/sctp/socket.c:402
> [<00000000b8795757>] sctp_bind+0x44/0x70 net/sctp/socket.c:302
> [<00000000672a44aa>] inet6_bind+0x40/0xb7 net/ipv6/af_inet6.c:445
> [<0000000001400e1c>] __sys_bind+0x11c/0x140 net/socket.c:1659
> [<00000000e69e8036>] __do_sys_bind net/socket.c:1670 [inline]
> [<00000000e69e8036>] __se_sys_bind net/socket.c:1668 [inline]
> [<00000000e69e8036>] __x64_sys_bind+0x1e/0x30 net/socket.c:1668
> [<000000001644bb1f>] do_syscall_64+0x76/0x1a0
> arch/x86/entry/common.c:301
> [<00000000199a1ea2>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
will post a fix for this:
@@ -4816,6 +4816,7 @@ static int sctp_setsockopt(struct sock *sk, int
level, int optname,
static int sctp_connect(struct sock *sk, struct sockaddr *addr,
int addr_len, int flags)
{
+ struct sctp_bind_addr *bp = &sctp_sk(sk)->ep->base.bind_addr;
struct inet_sock *inet = inet_sk(sk);
struct sctp_af *af;
int err = 0;
@@ -4826,12 +4827,13 @@ static int sctp_connect(struct sock *sk,
struct sockaddr *addr,
addr, addr_len);
/* We may need to bind the socket. */
- if (!inet->inet_num) {
+ if (!bp->port) {
if (sk->sk_prot->get_port(sk, 0)) {
release_sock(sk);
return -EAGAIN;
}
inet->inet_sport = htons(inet->inet_num);
+ bp->port = inet->inet_sport;
}
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support
From: Vladimir Oltean @ 2019-06-25 12:40 UTC (permalink / raw)
To: Marek Vasut
Cc: netdev, Andrew Lunn, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <f1dfe749-4bfa-17f7-ede7-f9bc38afa0d4@denx.de>
On Tue, 25 Jun 2019 at 15:06, Marek Vasut <marex@denx.de> wrote:
>
> On 6/25/19 1:59 AM, Vladimir Oltean wrote:
> > On Tue, 25 Jun 2019 at 01:17, Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 6/24/19 12:35 AM, Marek Vasut wrote:
> >>> Add basic SPI regmap support into the driver.
> >>>
> >>> Previous patches unconver that ksz_spi_write() is always ever called
> >>> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
> >>> check and we can also drop the allocation of the txbuf which is part
> >>> of the driver data and wastes 256 bytes for no reason. Regmap covers
> >>> the whole thing now.
> >>>
> >>> Signed-off-by: Marek Vasut <marex@denx.de>
> >>> Cc: Andrew Lunn <andrew@lunn.ch>
> >>> Cc: Florian Fainelli <f.fainelli@gmail.com>
> >>> Cc: Tristram Ha <Tristram.Ha@microchip.com>
> >>> Cc: Woojung Huh <Woojung.Huh@microchip.com>
> >>
> >> [...]
> >>
> >>> +#define KS_SPIOP_FLAG_MASK(opcode) \
> >>> + cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
> >>
> >> So the robot is complaining about this. I believe this is correct, as
> >> the mask should be in native endianness on the register and NOT the
> >> native endianness of the CPU.
> >>
> >> I think a cast would help here, e.g.:
> >> - cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
> >> - (__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
> >> SPI_TURNAROUND_SHIFT))
> >>
> >> Does this make sense ?
> >>
> >>> +#define KSZ_REGMAP_COMMON(width) \
> >>> + { \
> >>> + .val_bits = (width), \
> >>> + .reg_stride = (width) / 8, \
> >>> + .reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN, \
> >>> + .pad_bits = SPI_TURNAROUND_SHIFT, \
> >>> + .max_register = BIT(SPI_ADDR_SHIFT) - 1, \
> >>> + .cache_type = REGCACHE_NONE, \
> >>> + .read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD), \
> >>> + .write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR), \
> >>
> >> [...]
> >>
> >> --
> >> Best regards,
> >> Marek Vasut
> >
> > Hi Marek,
> >
> > I saw SPI buffers and endianness and got triggered :)
> > Would it make sense to take a look at CONFIG_PACKING for the KSZ9477 driver?
> > I don't know how bad the field alignment issue is on that hardware,
> > but on SJA1105 it was such a disaster that I couldn't have managed it
> > any other way.
>
> How does that help me here ? All I really need is a static constant mask
> for the register/flags , 32bit for KSZ9xxx and 16bit for KSZ87xx. I
> don't need any dynamic stuff, luckily.
>
Ok. I was thinking *instead of* regmap.
On the SJA1105 I couldn't use a spi regmap because:
* the enum regmap_endian wasn't enough to describe the hardware's bit
layout (it is big endian, but high-order and low-order 32 bit words
are swapped)
* it doesn't really expose a well-defined register map, but rather,
you're supposed to dynamically construct a TLV-type buffer and write
it starting with a fixed address.
I just thought you were facing some of these problems as well with
regmap. If not, that's perfectly fine!
> But I'm glad to see TJA1105 driver mainline :)
>
> --
> Best regards,
> Marek Vasut
Regards,
-Vladimir
^ permalink raw reply
* Re: [PATCH v3 2/2] bpf: Add selftests for bpf_perf_event_output
From: Daniel Borkmann @ 2019-06-25 12:40 UTC (permalink / raw)
To: allanzhang, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
In-Reply-To: <20190624235720.167067-3-allanzhang@google.com>
On 06/25/2019 01:57 AM, allanzhang wrote:
> Software event output is only enabled by a few prog types.
> This test is to ensure that all supported types are enbled for
Nit, typo: enbled
> bpf_perf_event_output sucessfully.
Nit, typo: sucessfully
> Signed-off-by: allanzhang <allanzhang@google.com>
For SOB, could you add proper formatted name before the email?
> ---
> tools/testing/selftests/bpf/test_verifier.c | 33 ++++++-
> .../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
> 2 files changed, 126 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index c5514daf8865..901a188e1eea 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -50,7 +50,7 @@
> #define MAX_INSNS BPF_MAXINSNS
> #define MAX_TEST_INSNS 1000000
> #define MAX_FIXUPS 8
> -#define MAX_NR_MAPS 18
> +#define MAX_NR_MAPS 19
> #define MAX_TEST_RUNS 8
> #define POINTER_VALUE 0xcafe4all
> #define TEST_DATA_LEN 64
> @@ -84,6 +84,7 @@ struct bpf_test {
> int fixup_map_array_wo[MAX_FIXUPS];
> int fixup_map_array_small[MAX_FIXUPS];
> int fixup_sk_storage_map[MAX_FIXUPS];
> + int fixup_map_event_output[MAX_FIXUPS];
> const char *errstr;
> const char *errstr_unpriv;
> uint32_t retval, retval_unpriv, insn_processed;
> @@ -604,6 +605,28 @@ static int create_sk_storage_map(void)
> return fd;
> }
>
> +static int create_event_output_map(void)
> +{
> + struct bpf_create_map_attr attr = {
> + .name = "test_map",
> + .map_type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> + .key_size = 4,
> + .value_size = 4,
> + .max_entries = 1,
> + };
> + int fd, btf_fd;
> +
> + btf_fd = load_btf();
> + if (btf_fd < 0)
> + return -1;
> + attr.btf_fd = btf_fd;
> + fd = bpf_create_map_xattr(&attr);
This does not look correct, BTF for spinlock does not belong to perf event array.
> + close(attr.btf_fd);
> + if (fd < 0)
> + printf("Failed to create event_output\n");
> + return fd;
> +}
> +
> static char bpf_vlog[UINT_MAX >> 8];
>
> static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
> @@ -627,6 +650,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
> int *fixup_map_array_wo = test->fixup_map_array_wo;
> int *fixup_map_array_small = test->fixup_map_array_small;
> int *fixup_sk_storage_map = test->fixup_sk_storage_map;
> + int *fixup_map_event_output = test->fixup_map_event_output;
>
> if (test->fill_helper) {
> test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
> @@ -788,6 +812,13 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
> fixup_sk_storage_map++;
> } while (*fixup_sk_storage_map);
> }
> + if (*fixup_map_event_output) {
> + map_fds[18] = create_event_output_map();
> + do {
> + prog[*fixup_map_event_output].imm = map_fds[18];
> + fixup_map_event_output++;
> + } while (*fixup_map_event_output);
> + }
> }
>
> static int set_admin(bool admin)
> diff --git a/tools/testing/selftests/bpf/verifier/event_output.c b/tools/testing/selftests/bpf/verifier/event_output.c
> new file mode 100644
> index 000000000000..b25eabcfaa56
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/verifier/event_output.c
> @@ -0,0 +1,94 @@
> +/* instructions used to output a skb based software event, produced
> + * from code snippet:
> +struct TMP {
> + uint64_t tmp;
> +} tt;
> +tt.tmp = 5;
> +bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
> + &tt, sizeof(tt));
> +return 1;
[...]
^ permalink raw reply
* Re: [PATCH] net: phylink: further documentation clarifications
From: Andrew Lunn @ 2019-06-25 12:38 UTC (permalink / raw)
To: Russell King; +Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <E1hfi09-0007Zs-Vb@rmk-PC.armlinux.org.uk>
On Tue, Jun 25, 2019 at 10:44:33AM +0100, Russell King wrote:
> Clarify the validate() behaviour in a few cases which weren't mentioned
> in the documentation, but which are necessary for users to get the
> correct behaviour.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* DMA API usage in au1000_eth.c
From: Christoph Hellwig @ 2019-06-25 12:34 UTC (permalink / raw)
To: BjoernRiemerriemer, Matt Porter, Herbert Valerio Riedel
Cc: linux-mips, netdev, linux-kernel
Hi all,
you are the persons that have their names listed in the driver,
hope you all remember what you did 15 years ago :)
The au1000_eth driver uses the DMA API somewhat oddly. For one
it uses the DMA_ATTR_NON_CONSISTENT flag to allocate memory that
is not DMA coherent, accompanied by a comment say:
/* Allocate the data buffers
* Snooping works fine with eth on all au1xxx
*/
which suggests that it actually is DMA coherent. As far as I can
tell many amd mips platforms are DMA coherent, in which case
DMA_ATTR_NON_CONSISTENT is no-op and everything is fine here. But
it seems some are not, in which case DMA_ATTR_NON_CONSISTENT will
give us not coherent memory, in which case something would be
broken? Removing DMA_ATTR_NON_CONSISTENT would be no-op on
coherent platforms, but return an address in the cache segement
on those that are non-coherent. Do you know if the hardware
event cares?
The next issue is that it calls virt_to_phys on the return value
from dma_alloc_attrs. Why would the hardware care about the physical
address and not the DMA address (in general those are the same in
mips)?
Last but not least it stores the kernel address return from
dma_alloc_attrs in a u32 instead of a pointer, which is a little
odd and not type safe, but not otherwise dramatic.
I can prepare a patch to fix these up, but I'd like to confirm my
theory about coherent of the platform and device first.
^ permalink raw reply
* Re: [PATCH v3 bpf-next 0/2] veth: Bulk XDP_TX
From: Daniel Borkmann @ 2019-06-25 12:29 UTC (permalink / raw)
To: Toshiaki Makita, Alexei Starovoitov, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend
Cc: netdev, xdp-newbies, bpf, Toke Høiland-Jørgensen,
Jason Wang
In-Reply-To: <20190613093959.2796-1-toshiaki.makita1@gmail.com>
On 06/13/2019 11:39 AM, Toshiaki Makita wrote:
> This introduces bulk XDP_TX in veth.
> Improves XDP_TX performance by approximately 9%. The detailed
> explanation and performance numbers are shown in patch 2.
>
> v2:
> - Use stack for bulk queue instead of a global variable.
>
> v3:
> - Add act field to xdp_bulk_tx tracepoint to be in line with other XDP
> tracepoints.
>
> Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
>
> Toshiaki Makita (2):
> xdp: Add tracepoint for bulk XDP_TX
> veth: Support bulk XDP_TX
>
> drivers/net/veth.c | 60 ++++++++++++++++++++++++++++++++++++----------
> include/trace/events/xdp.h | 29 ++++++++++++++++++++++
> kernel/bpf/core.c | 1 +
> 3 files changed, 78 insertions(+), 12 deletions(-)
>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next v2 08/12] xdp: tracking page_pool resources and safe removal
From: Ivan Khoronzhuk @ 2019-06-25 12:28 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, Ilias Apalodimas, Toke Høiland-Jørgensen,
Tariq Toukan, toshiaki.makita1, grygorii.strashko, mcroce
In-Reply-To: <20190625115107.GB6485@khorivan>
On Tue, Jun 25, 2019 at 02:51:08PM +0300, Ivan Khoronzhuk wrote:
>On Tue, Jun 25, 2019 at 01:27:50PM +0200, Jesper Dangaard Brouer wrote:
>>On Tue, 25 Jun 2019 13:50:14 +0300
>>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>>
>>>Hi Jesper,
>>>
>>>Could you please clarify one question.
>>>
>>>On Tue, Jun 18, 2019 at 03:05:47PM +0200, Jesper Dangaard Brouer wrote:
>>>>This patch is needed before we can allow drivers to use page_pool for
>>>>DMA-mappings. Today with page_pool and XDP return API, it is possible to
>>>>remove the page_pool object (from rhashtable), while there are still
>>>>in-flight packet-pages. This is safely handled via RCU and failed lookups in
>>>>__xdp_return() fallback to call put_page(), when page_pool object is gone.
>>>>In-case page is still DMA mapped, this will result in page note getting
>>>>correctly DMA unmapped.
>>>>
>>>>To solve this, the page_pool is extended with tracking in-flight pages. And
>>>>XDP disconnect system queries page_pool and waits, via workqueue, for all
>>>>in-flight pages to be returned.
>>>>
>>>>To avoid killing performance when tracking in-flight pages, the implement
>>>>use two (unsigned) counters, that in placed on different cache-lines, and
>>>>can be used to deduct in-flight packets. This is done by mapping the
>>>>unsigned "sequence" counters onto signed Two's complement arithmetic
>>>>operations. This is e.g. used by kernel's time_after macros, described in
>>>>kernel commit 1ba3aab3033b and 5a581b367b5, and also explained in RFC1982.
>>>>
>>>>The trick is these two incrementing counters only need to be read and
>>>>compared, when checking if it's safe to free the page_pool structure. Which
>>>>will only happen when driver have disconnected RX/alloc side. Thus, on a
>>>>non-fast-path.
>>>>
>>>>It is chosen that page_pool tracking is also enabled for the non-DMA
>>>>use-case, as this can be used for statistics later.
>>>>
>>>>After this patch, using page_pool requires more strict resource "release",
>>>>e.g. via page_pool_release_page() that was introduced in this patchset, and
>>>>previous patches implement/fix this more strict requirement.
>>>>
>>>>Drivers no-longer call page_pool_destroy(). Drivers already call
>>>>xdp_rxq_info_unreg() which call xdp_rxq_info_unreg_mem_model(), which will
>>>>attempt to disconnect the mem id, and if attempt fails schedule the
>>>>disconnect for later via delayed workqueue.
>>>>
>>>>Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>>>>Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>>>>---
>>>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 -
>>>> include/net/page_pool.h | 41 ++++++++++---
>>>> net/core/page_pool.c | 62 +++++++++++++++-----
>>>> net/core/xdp.c | 65 +++++++++++++++++++--
>>>> 4 files changed, 136 insertions(+), 35 deletions(-)
>>>>
>>>>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>>>>index 2f647be292b6..6c9d4d7defbc 100644
>>>>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>>>>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>>>>@@ -643,9 +643,6 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
>>>> }
>>>>
>>>> xdp_rxq_info_unreg(&rq->xdp_rxq);
>>>>- if (rq->page_pool)
>>>>- page_pool_destroy(rq->page_pool);
>>>>-
>>>> mlx5_wq_destroy(&rq->wq_ctrl);
>>>> }
>>>>
>>>>diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>>>>index 754d980700df..f09b3f1994e6 100644
>>>>--- a/include/net/page_pool.h
>>>>+++ b/include/net/page_pool.h
>>>>@@ -16,14 +16,16 @@
>>>> * page_pool_alloc_pages() call. Drivers should likely use
>>>> * page_pool_dev_alloc_pages() replacing dev_alloc_pages().
>>>> *
>>>>- * If page_pool handles DMA mapping (use page->private), then API user
>>>>- * is responsible for invoking page_pool_put_page() once. In-case of
>>>>- * elevated refcnt, the DMA state is released, assuming other users of
>>>>- * the page will eventually call put_page().
>>>>+ * API keeps track of in-flight pages, in-order to let API user know
>>>>+ * when it is safe to dealloactor page_pool object. Thus, API users
>>>>+ * must make sure to call page_pool_release_page() when a page is
>>>>+ * "leaving" the page_pool. Or call page_pool_put_page() where
>>>>+ * appropiate. For maintaining correct accounting.
>>>> *
>>>>- * If no DMA mapping is done, then it can act as shim-layer that
>>>>- * fall-through to alloc_page. As no state is kept on the page, the
>>>>- * regular put_page() call is sufficient.
>>>>+ * API user must only call page_pool_put_page() once on a page, as it
>>>>+ * will either recycle the page, or in case of elevated refcnt, it
>>>>+ * will release the DMA mapping and in-flight state accounting. We
>>>>+ * hope to lift this requirement in the future.
>>>> */
>>>> #ifndef _NET_PAGE_POOL_H
>>>> #define _NET_PAGE_POOL_H
>>>>@@ -66,9 +68,10 @@ struct page_pool_params {
>>>> };
>>>>
>>>> struct page_pool {
>>>>- struct rcu_head rcu;
>>>> struct page_pool_params p;
>>>>
>>>>+ u32 pages_state_hold_cnt;
>>>>+
>>>> /*
>>>> * Data structure for allocation side
>>>> *
>>>>@@ -96,6 +99,8 @@ struct page_pool {
>>>> * TODO: Implement bulk return pages into this structure.
>>>> */
>>>> struct ptr_ring ring;
>>>>+
>>>>+ atomic_t pages_state_release_cnt;
>>>> };
>>>>
>>>> struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
>>>>@@ -109,8 +114,6 @@ static inline struct page *page_pool_dev_alloc_pages(struct page_pool *pool)
>>>>
>>>> struct page_pool *page_pool_create(const struct page_pool_params *params);
>>>>
>>>>-void page_pool_destroy(struct page_pool *pool);
>>>>-
>>>> void __page_pool_free(struct page_pool *pool);
>>>> static inline void page_pool_free(struct page_pool *pool)
>>>> {
>>>>@@ -143,6 +146,24 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
>>>> __page_pool_put_page(pool, page, true);
>>>> }
>>>>
>>>>+/* API user MUST have disconnected alloc-side (not allowed to call
>>>>+ * page_pool_alloc_pages()) before calling this. The free-side can
>>>>+ * still run concurrently, to handle in-flight packet-pages.
>>>>+ *
>>>>+ * A request to shutdown can fail (with false) if there are still
>>>>+ * in-flight packet-pages.
>>>>+ */
>>>>+bool __page_pool_request_shutdown(struct page_pool *pool);
>>>>+static inline bool page_pool_request_shutdown(struct page_pool *pool)
>>>>+{
>>>>+ /* When page_pool isn't compiled-in, net/core/xdp.c doesn't
>>>>+ * allow registering MEM_TYPE_PAGE_POOL, but shield linker.
>>>>+ */
>>>>+#ifdef CONFIG_PAGE_POOL
>>>>+ return __page_pool_request_shutdown(pool);
>>>>+#endif
>>>>+}
>>>
>>>The free side can ran in softirq, that means fast cache recycle is accessed.
>>>And it increments not atomic pool->alloc.count.
>>>
>>>For instance While redirect, for remote interface, while .ndo_xdp_xmit the
>>>xdp_return_frame_rx_napi(xdpf) is called everywhere in error path ....
>^
>|
>
>>>
>>>In the same time, simultaneously, the work queue can try one more
>>>time to clear cash, calling __page_pool_request_shutdown()....
>>>
>>>Question, what prevents pool->alloc.count to be corrupted by race,
>>>causing to wrong array num and as result wrong page to be unmapped/put ....or
>>>even page leak. alloc.count usage is not protected,
>>>__page_pool_request_shutdown() is called not from same rx NAPI, even not from
>>>NAPI.
>>>
>>>Here, while alloc cache empty procedure in __page_pool_request_shutdown():
>>
>>You forgot to copy this comment, which explains:
>>
>> /* Empty alloc cache, assume caller made sure this is
>> * no-longer in use, and page_pool_alloc_pages() cannot be
>> * call concurrently.
>> */
>No I didn't. I'm talking about recycle, not allocation.
>To be more specific about this:
>__page_pool_recycle_direct() while remote ndev .ndo_xdp_xmit
>
>About this:
>"
>/* API user MUST have disconnected alloc-side (not allowed to call
>* page_pool_alloc_pages()) before calling this. The free-side can
>* still run concurrently, to handle in-flight packet-pages.
>"
For me it's important to know only if it means that alloc.count is
freed at first call of __mem_id_disconnect() while shutdown.
The workqueue for the rest is connected only with ring cache protected
by ring lock and not supposed that alloc.count can be changed while
workqueue tries to shutdonwn the pool.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* [PATCH net] vrf: reset rt_iif for recirculated mcast out pkts
From: Stephen Suryaputra @ 2019-06-25 10:33 UTC (permalink / raw)
To: netdev; +Cc: dsahern, Stephen Suryaputra
Multicast egress packets has skb_rtable(skb)->rt_iif set to the oif.
Depending on the socket, these packets might be recirculated back as
input and raw sockets that are opened for them are bound to the VRF. But
since skb_rtable(skb) is set and its rt_iif is non-zero, inet_iif()
function returns rt_iif instead of skb_iif (the VRF netdev). Hence, the
socket lookup fails.
Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
---
include/net/route.h | 1 +
net/ipv4/ip_output.c | 25 ++++++++++++++++++++++++-
net/ipv4/route.c | 33 +++++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/include/net/route.h b/include/net/route.h
index 065b47754f05..55ff71ffb796 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -221,6 +221,7 @@ void ip_rt_get_source(u8 *src, struct sk_buff *skb, struct rtable *rt);
struct rtable *rt_dst_alloc(struct net_device *dev,
unsigned int flags, u16 type,
bool nopolicy, bool noxfrm, bool will_cache);
+struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt);
struct in_ifaddr;
void fib_add_ifaddr(struct in_ifaddr *);
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 16f9159234a2..a5e240bad3ce 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -329,6 +329,19 @@ static int ip_mc_finish_output(struct net *net, struct sock *sk,
return dev_loopback_xmit(net, sk, skb);
}
+static void ip_mc_reset_rt_iif(struct net *net, struct rtable *rt,
+ struct sk_buff *newskb)
+{
+ struct rtable *new_rt;
+
+ new_rt = rt_dst_clone(net->loopback_dev, rt);
+ if (new_rt) {
+ new_rt->rt_iif = 0;
+ skb_dst_drop(newskb);
+ skb_dst_set(newskb, &new_rt->dst);
+ }
+}
+
int ip_mc_output(struct net *net, struct sock *sk, struct sk_buff *skb)
{
struct rtable *rt = skb_rtable(skb);
@@ -363,10 +376,20 @@ int ip_mc_output(struct net *net, struct sock *sk, struct sk_buff *skb)
#endif
) {
struct sk_buff *newskb = skb_clone(skb, GFP_ATOMIC);
- if (newskb)
+ if (newskb) {
+ /* Reset rt_iif so that inet_iif() will return
+ * skb->dev->ifIndex which is the VRF device for
+ * socket lookup. Setting this to VRF ifindex
+ * causes ipi_ifindex in in_pktinfo to be
+ * overwritten, see ipv4_pktinfo_prepare().
+ */
+ if (netif_is_l3_slave(dev))
+ ip_mc_reset_rt_iif(net, rt, newskb);
+
NF_HOOK(NFPROTO_IPV4, NF_INET_POST_ROUTING,
net, sk, newskb, NULL, newskb->dev,
ip_mc_finish_output);
+ }
}
/* Multicasts with ttl 0 must not go beyond the host */
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 6cb7cff22db9..8ea0735a6754 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1647,6 +1647,39 @@ struct rtable *rt_dst_alloc(struct net_device *dev,
}
EXPORT_SYMBOL(rt_dst_alloc);
+struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt)
+{
+ struct rtable *new_rt;
+
+ new_rt = dst_alloc(&ipv4_dst_ops, dev, 1, DST_OBSOLETE_FORCE_CHK,
+ rt->dst.flags);
+
+ if (new_rt) {
+ new_rt->rt_genid = rt_genid_ipv4(dev_net(dev));
+ new_rt->rt_flags = rt->rt_flags;
+ new_rt->rt_type = rt->rt_type;
+ new_rt->rt_is_input = rt->rt_is_input;
+ new_rt->rt_iif = rt->rt_iif;
+ new_rt->rt_pmtu = rt->rt_pmtu;
+ new_rt->rt_mtu_locked = rt->rt_mtu_locked;
+ new_rt->rt_gw_family = rt->rt_gw_family;
+ if (rt->rt_gw_family == AF_INET)
+ new_rt->rt_gw4 = rt->rt_gw4;
+ else if (rt->rt_gw_family == AF_INET6)
+ new_rt->rt_gw6 = rt->rt_gw6;
+ INIT_LIST_HEAD(&new_rt->rt_uncached);
+
+ new_rt->dst.flags |= DST_HOST;
+ new_rt->dst.input = rt->dst.input;
+ new_rt->dst.output = rt->dst.output;
+ new_rt->dst.error = rt->dst.error;
+ new_rt->dst.lastuse = jiffies;
+ new_rt->dst.lwtstate = lwtstate_get(rt->dst.lwtstate);
+ }
+ return new_rt;
+}
+EXPORT_SYMBOL(rt_dst_clone);
+
/* called in rcu_read_lock() section */
int ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
u8 tos, struct net_device *dev,
--
2.17.1
^ permalink raw reply related
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Russell King - ARM Linux admin @ 2019-06-25 12:10 UTC (permalink / raw)
To: René van Dorst
Cc: sean.wang, f.fainelli, davem, matthias.bgg, andrew,
vivien.didelot, frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <20190625113158.Horde.pCaJOVUsgyhYLd5Diz5EZKI@www.vdorst.com>
On Tue, Jun 25, 2019 at 11:31:58AM +0000, René van Dorst wrote:
> > > + if (state->link || mode == MLO_AN_FIXED)
> > > + mcr |= PMCR_FORCE_LNK;
> >
> > This should be removed - state->link is not for use in mac_config.
> > Even in fixed mode, the link can be brought up/down by means of a
> > gpio, and this should be dealt with via the mac_link_* functions.
>
> Maybe I understand it wrong, but is it the intention that in
> phylink_mac_config with modes MLO_AN_FIXED and MLO_AN_PHY the MAC is always
> forces into a certain speed/mode/interface. So it never auto-negotiate because
> phylink select the best configuration for you?
You are not the only one who has recently tried to make use of
state->link in mac_config(), so I'm now preparing a set of patches
to split the current mac_config() method into two separate methods:
void (*mac_config_fixed)(struct net_device *ndev,
phy_interface_t iface, int speed, int duplex,
int pause);
void (*mac_config_inband)(struct net_device *ndev,
phy_interface_t iface, bool an_enabled,
unsigned long *advertising, int pause);
so that it's not possible to use members that should not be accessed
in various modes.
> Also the PMCR_FORCE_LNK is only set in phylink_link_up() or can I do it here
> and do nothing phylink_link_up()?
When the link comes up/down, mac_link_up() and mac_link_down() will be
called appropriately. In PHY mode, this is equivalent to phylink doing
this:
if (link_changed) {
if (phydev->link)
mac_link_up();
else
mac_link_down();
}
So the actions you would've done depending on phydev->link should be in
the mac_link_*() methods.
> Other question:
> Where does the MAC enable/disable TX and RX fits best? port_{enable,disable}?
> Or only mac_config() and port_disable?
mac_link_*().
> > > + if (state->pause || phylink_test(state->advertising, Pause))
> > > + mcr |= PMCR_TX_FC_EN | PMCR_RX_FC_EN;
> > > + if (state->pause & MLO_PAUSE_TX)
> > > + mcr |= PMCR_TX_FC_EN;
> > > + if (state->pause & MLO_PAUSE_RX)
> > > + mcr |= PMCR_RX_FC_EN;
> >
> > This is clearly wrong - if any bit in state->pause is set, then we
> > end up with both PMCR_TX_FC_EN | PMCR_RX_FC_EN set. If we have Pause
> > Pause set in the advertising mask, then both are set. This doesn't
> > seem right - are these bits setting the advertisement, or are they
> > telling the MAC to use flow control?
>
> Last one, tell the MAC to use flow control.
So the first if() statement is incorrect, and should be removed
entirely. You only want to enable the MAC to use flow control as a
result of the negotiation results.
> On the current driver both bits are set in a forced-link situation.
>
> If we always forces the MAC mode I think I always set these bits and don't
> anything with the Pause modes? Is that the right way to do it?
So what happens if your link partner (e.g. switch) does not support
flow control? What if your link partner floods such frames to all
ports? You end up transmitting flow control frames, which could be
sent to all stations on the network... seems not a good idea.
Implementing stuff properly and not taking short-cuts is always a
good idea for inter-operability.
> > > +
> > > +static void mt7530_phylink_validate(struct dsa_switch *ds, int port,
> > > + unsigned long *supported,
> > > + struct phylink_link_state *state)
> > > +{
> > > + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > > +
> > > + switch (port) {
> > > + case 0: /* Internal phy */
> > > + case 1:
> > > + case 2:
> > > + case 3:
> > > + case 4:
> > > + if (state->interface != PHY_INTERFACE_MODE_NA &&
> > > + state->interface != PHY_INTERFACE_MODE_GMII)
> > > + goto unsupported;
> > > + break;
> > > + /* case 5: Port 5 not supported! */
> > > + case 6: /* 1st cpu port */
> > > + if (state->interface != PHY_INTERFACE_MODE_RGMII &&
> > > + state->interface != PHY_INTERFACE_MODE_TRGMII)
> >
> > PHY_INTERFACE_MODE_NA ?
>
> You mean PHY_INTERFACE_MODE_NA is missing?
Yes. Please see the updated documentation in the patch I sent this
morning "net: phylink: further documentation clarifications".
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH] bpf: Allow bpf_skb_event_output for a few prog types
From: Daniel Borkmann @ 2019-06-25 12:06 UTC (permalink / raw)
To: allanzhang, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, netdev, bpf, linux-kernel
In-Reply-To: <20190625001326.172280-3-allanzhang@google.com>
Hello,
On 06/25/2019 02:13 AM, allanzhang wrote:
> Software event output is only enabled by a few prog types right now (TC,
> LWT out, XDP, sockops). Many other skb based prog types need
> bpf_skb_event_output to produce software event.
>
> Added socket_filter, cg_skb, sk_skb prog types to generate sw event.
>
> Test bpf code is generated from code snippet:
>
> struct TMP {
> uint64_t tmp;
> } tt;
> tt.tmp = 5;
> bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
> &tt, sizeof(tt));
> return 1;
>
> the bpf assembly from llvm is:
> 0: b7 02 00 00 05 00 00 00 r2 = 5
> 1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
> 2: bf a4 00 00 00 00 00 00 r4 = r10
> 3: 07 04 00 00 f8 ff ff ff r4 += -8
> 4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
> 6: b7 03 00 00 00 00 00 00 r3 = 0
> 7: b7 05 00 00 08 00 00 00 r5 = 8
> 8: 85 00 00 00 19 00 00 00 call 25
> 9: b7 00 00 00 01 00 00 00 r0 = 1
> 10: 95 00 00 00 00 00 00 00 exit
>
> Patch 1 is enabling code.
> Patch 2 is fullly covered selftest code.
>
> Signed-off-by: allanzhang <allanzhang@google.com>
Thanks for the contribution! I'm a bit confused given the many submissions,
some are versioned in the subject (which is the correct way), but this patch
here was sent after v3 (?) but without a version. Which is the right one to
consider for review, I presume v3?
> ---
> tools/testing/selftests/bpf/test_verifier.c | 38 +++++++-
> .../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
> 2 files changed, 130 insertions(+), 2 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index c5514daf8865..45ce9dd4323f 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -1,10 +1,13 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> /*
> * Testsuite for eBPF verifier
> *
> * Copyright (c) 2014 PLUMgrid, http://plumgrid.com
> * Copyright (c) 2017 Facebook
> * Copyright (c) 2018 Covalent IO, Inc. http://covalent.io
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
This removal of SPDX probably slipped in as well here ..
> */
>
> #include <endian.h>
> @@ -50,7 +53,7 @@
> #define MAX_INSNS BPF_MAXINSNS
> #define MAX_TEST_INSNS 1000000
> #define MAX_FIXUPS 8
> -#define MAX_NR_MAPS 18
> +#define MAX_NR_MAPS 19
> #define MAX_TEST_RUNS 8
> #define POINTER_VALUE 0xcafe4all
> #define TEST_DATA_LEN 64
> @@ -84,6 +87,7 @@ struct bpf_test {
> int fixup_map_array_wo[MAX_FIXUPS];
> int fixup_map_array_small[MAX_FIXUPS];
> int fixup_sk_storage_map[MAX_FIXUPS];
> + int fixup_map_event_output[MAX_FIXUPS];
> const char *errstr;
> const char *errstr_unpriv;
> uint32_t retval, retval_unpriv, insn_processed;
> @@ -604,6 +608,28 @@ static int create_sk_storage_map(void)
> return fd;
> }
>
> +static int create_event_output_map(void)
> +{
> + struct bpf_create_map_attr attr = {
> + .name = "test_map",
> + .map_type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> + .key_size = 4,
> + .value_size = 4,
> + .max_entries = 1,
> + };
> + int fd, btf_fd;
> +
> + btf_fd = load_btf();
> + if (btf_fd < 0)
> + return -1;
> + attr.btf_fd = btf_fd;
> + fd = bpf_create_map_xattr(&attr);
> + close(attr.btf_fd);
> + if (fd < 0)
> + printf("Failed to create event_output\n");
> + return fd;
> +}
> +
> static char bpf_vlog[UINT_MAX >> 8];
>
> static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
> @@ -627,6 +653,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
> int *fixup_map_array_wo = test->fixup_map_array_wo;
> int *fixup_map_array_small = test->fixup_map_array_small;
> int *fixup_sk_storage_map = test->fixup_sk_storage_map;
> + int *fixup_map_event_output = test->fixup_map_event_output;
>
> if (test->fill_helper) {
> test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
> @@ -788,6 +815,13 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
> fixup_sk_storage_map++;
> } while (*fixup_sk_storage_map);
> }
> + if (*fixup_map_event_output) {
> + map_fds[18] = create_event_output_map();
> + do {
> + prog[*fixup_map_event_output].imm = map_fds[18];
> + fixup_map_event_output++;
> + } while (*fixup_map_event_output);
> + }
> }
>
> static int set_admin(bool admin)
> diff --git a/tools/testing/selftests/bpf/verifier/event_output.c b/tools/testing/selftests/bpf/verifier/event_output.c
> new file mode 100644
> index 000000000000..b25eabcfaa56
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/verifier/event_output.c
> @@ -0,0 +1,94 @@
> +/* instructions used to output a skb based software event, produced
> + * from code snippet:
> +struct TMP {
> + uint64_t tmp;
> +} tt;
> +tt.tmp = 5;
> +bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
> + &tt, sizeof(tt));
> +return 1;
> +
> +the bpf assembly from llvm is:
> + 0: b7 02 00 00 05 00 00 00 r2 = 5
> + 1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
> + 2: bf a4 00 00 00 00 00 00 r4 = r10
> + 3: 07 04 00 00 f8 ff ff ff r4 += -8
> + 4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
> + 6: b7 03 00 00 00 00 00 00 r3 = 0
> + 7: b7 05 00 00 08 00 00 00 r5 = 8
> + 8: 85 00 00 00 19 00 00 00 call 25
> + 9: b7 00 00 00 01 00 00 00 r0 = 1
> + 10: 95 00 00 00 00 00 00 00 exit
> +
> + The reason I put the code here instead of fill_helpers is that map fixup is
> + against the insns, instead of filled prog.
> +*/
> +
> +#define __PERF_EVENT_INSNS__ \
> + BPF_MOV64_IMM(BPF_REG_2, 5), \
> + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8), \
> + BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), \
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8), \
> + BPF_LD_MAP_FD(BPF_REG_2, 0), \
> + BPF_MOV64_IMM(BPF_REG_3, 0), \
> + BPF_MOV64_IMM(BPF_REG_5, 8), \
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, \
> + BPF_FUNC_perf_event_output), \
> + BPF_MOV64_IMM(BPF_REG_0, 1), \
> + BPF_EXIT_INSN(),
> +{
> + "perfevent for sockops",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_SOCK_OPS,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for tc",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for lwt out",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_LWT_OUT,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for xdp",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_XDP,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for socket filter",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for sk_skb",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_SK_SKB,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for cgroup skb",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
>
^ permalink raw reply
* Re: [PATCH V3 07/10] net: dsa: microchip: Initial SPI regmap support
From: Marek Vasut @ 2019-06-25 12:06 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, Andrew Lunn, Florian Fainelli, Tristram Ha, Woojung Huh
In-Reply-To: <CA+h21hpgXxnnRS7Upc1R82ajLum4k-3O9EQDROonO6jtAD+NZw@mail.gmail.com>
On 6/25/19 1:59 AM, Vladimir Oltean wrote:
> On Tue, 25 Jun 2019 at 01:17, Marek Vasut <marex@denx.de> wrote:
>>
>> On 6/24/19 12:35 AM, Marek Vasut wrote:
>>> Add basic SPI regmap support into the driver.
>>>
>>> Previous patches unconver that ksz_spi_write() is always ever called
>>> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
>>> check and we can also drop the allocation of the txbuf which is part
>>> of the driver data and wastes 256 bytes for no reason. Regmap covers
>>> the whole thing now.
>>>
>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>> Cc: Andrew Lunn <andrew@lunn.ch>
>>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>>> Cc: Tristram Ha <Tristram.Ha@microchip.com>
>>> Cc: Woojung Huh <Woojung.Huh@microchip.com>
>>
>> [...]
>>
>>> +#define KS_SPIOP_FLAG_MASK(opcode) \
>>> + cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
>>
>> So the robot is complaining about this. I believe this is correct, as
>> the mask should be in native endianness on the register and NOT the
>> native endianness of the CPU.
>>
>> I think a cast would help here, e.g.:
>> - cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
>> - (__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
>> SPI_TURNAROUND_SHIFT))
>>
>> Does this make sense ?
>>
>>> +#define KSZ_REGMAP_COMMON(width) \
>>> + { \
>>> + .val_bits = (width), \
>>> + .reg_stride = (width) / 8, \
>>> + .reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN, \
>>> + .pad_bits = SPI_TURNAROUND_SHIFT, \
>>> + .max_register = BIT(SPI_ADDR_SHIFT) - 1, \
>>> + .cache_type = REGCACHE_NONE, \
>>> + .read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD), \
>>> + .write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR), \
>>
>> [...]
>>
>> --
>> Best regards,
>> Marek Vasut
>
> Hi Marek,
>
> I saw SPI buffers and endianness and got triggered :)
> Would it make sense to take a look at CONFIG_PACKING for the KSZ9477 driver?
> I don't know how bad the field alignment issue is on that hardware,
> but on SJA1105 it was such a disaster that I couldn't have managed it
> any other way.
How does that help me here ? All I really need is a static constant mask
for the register/flags , 32bit for KSZ9xxx and 16bit for KSZ87xx. I
don't need any dynamic stuff, luckily.
But I'm glad to see TJA1105 driver mainline :)
--
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [PATCH net v2] net: mvpp2: prs: Don't override the sign bit in SRAM parser shift
From: Maxime Chevallier @ 2019-06-25 12:04 UTC (permalink / raw)
To: davem
Cc: netdev, linux-kernel, Antoine Tenart, thomas.petazzoni,
gregory.clement, nadavh, stefanc, mw, Alan Winkowski
In-Reply-To: <20190620094245.10501-1-maxime.chevallier@bootlin.com>
Hello David,
On Thu, 20 Jun 2019 11:42:45 +0200
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
>The Header Parser allows identifying various fields in the packet
>headers, used for various kind of filtering and classification
>steps.
>
>This is a re-entrant process, where the offset in the packet header
>depends on the previous lookup results. This offset is represented in
>the SRAM results of the TCAM, as a shift to be operated.
>
>This shift can be negative in some cases, such as in IPv6 parsing.
>
>This commit prevents overriding the sign bit when setting the shift
>value, which could cause instabilities when parsing IPv6 flows.
>
>Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
>Suggested-by: Alan Winkowski <walan@marvell.com>
>Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
>---
>V2 : Fix a typo in the commit log, reported by Sergei.
I see that this patch was set as "Accepted" on patchwork, but hasn't
made it to -net, I was wondering if this patch slipped through the
cracks :)
https://patchwork.ozlabs.org/patch/1119311/
Thanks,
Maxime
^ permalink raw reply
* Re: [PATCH bpf-next] MAINTAINERS: add reviewer to maintainers entry
From: Björn Töpel @ 2019-06-25 12:02 UTC (permalink / raw)
To: Daniel Borkmann, Jonathan Lemon, Björn Töpel
Cc: ast, netdev, magnus.karlsson, bpf, linux-kernel
In-Reply-To: <e0654342-7110-c1e3-53bc-cb9c303252e9@iogearbox.net>
On 2019-06-25 14:00, Daniel Borkmann wrote:
> Given it's a doc update, I've applied it to bpf, thanks!
Perfect, thanks!
^ permalink raw reply
* Re: [PATCH bpf-next] MAINTAINERS: add reviewer to maintainers entry
From: Daniel Borkmann @ 2019-06-25 12:00 UTC (permalink / raw)
To: Jonathan Lemon, Björn Töpel
Cc: ast, netdev, Björn Töpel, magnus.karlsson, bpf,
linux-kernel
In-Reply-To: <B5DAC105-8F88-43F7-9F6F-6C0B436C4F06@gmail.com>
On 06/24/2019 10:54 PM, Jonathan Lemon wrote:
> On 23 Jun 2019, at 22:24, Björn Töpel wrote:
>> From: Björn Töpel <bjorn.topel@intel.com>
>>
>> Jonathan Lemon has volunteered as an official AF_XDP reviewer. Thank
>> you, Jonathan!
>>
>> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Given it's a doc update, I've applied it to bpf, thanks!
^ permalink raw reply
* Re: [PATCH net-next v2 08/12] xdp: tracking page_pool resources and safe removal
From: Ivan Khoronzhuk @ 2019-06-25 11:51 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, Ilias Apalodimas, Toke Høiland-Jørgensen,
Tariq Toukan, toshiaki.makita1, grygorii.strashko, mcroce
In-Reply-To: <20190625132750.06939133@carbon>
On Tue, Jun 25, 2019 at 01:27:50PM +0200, Jesper Dangaard Brouer wrote:
>On Tue, 25 Jun 2019 13:50:14 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> Hi Jesper,
>>
>> Could you please clarify one question.
>>
>> On Tue, Jun 18, 2019 at 03:05:47PM +0200, Jesper Dangaard Brouer wrote:
>> >This patch is needed before we can allow drivers to use page_pool for
>> >DMA-mappings. Today with page_pool and XDP return API, it is possible to
>> >remove the page_pool object (from rhashtable), while there are still
>> >in-flight packet-pages. This is safely handled via RCU and failed lookups in
>> >__xdp_return() fallback to call put_page(), when page_pool object is gone.
>> >In-case page is still DMA mapped, this will result in page note getting
>> >correctly DMA unmapped.
>> >
>> >To solve this, the page_pool is extended with tracking in-flight pages. And
>> >XDP disconnect system queries page_pool and waits, via workqueue, for all
>> >in-flight pages to be returned.
>> >
>> >To avoid killing performance when tracking in-flight pages, the implement
>> >use two (unsigned) counters, that in placed on different cache-lines, and
>> >can be used to deduct in-flight packets. This is done by mapping the
>> >unsigned "sequence" counters onto signed Two's complement arithmetic
>> >operations. This is e.g. used by kernel's time_after macros, described in
>> >kernel commit 1ba3aab3033b and 5a581b367b5, and also explained in RFC1982.
>> >
>> >The trick is these two incrementing counters only need to be read and
>> >compared, when checking if it's safe to free the page_pool structure. Which
>> >will only happen when driver have disconnected RX/alloc side. Thus, on a
>> >non-fast-path.
>> >
>> >It is chosen that page_pool tracking is also enabled for the non-DMA
>> >use-case, as this can be used for statistics later.
>> >
>> >After this patch, using page_pool requires more strict resource "release",
>> >e.g. via page_pool_release_page() that was introduced in this patchset, and
>> >previous patches implement/fix this more strict requirement.
>> >
>> >Drivers no-longer call page_pool_destroy(). Drivers already call
>> >xdp_rxq_info_unreg() which call xdp_rxq_info_unreg_mem_model(), which will
>> >attempt to disconnect the mem id, and if attempt fails schedule the
>> >disconnect for later via delayed workqueue.
>> >
>> >Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>> >Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> >---
>> > drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 -
>> > include/net/page_pool.h | 41 ++++++++++---
>> > net/core/page_pool.c | 62 +++++++++++++++-----
>> > net/core/xdp.c | 65 +++++++++++++++++++--
>> > 4 files changed, 136 insertions(+), 35 deletions(-)
>> >
>> >diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> >index 2f647be292b6..6c9d4d7defbc 100644
>> >--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> >+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> >@@ -643,9 +643,6 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
>> > }
>> >
>> > xdp_rxq_info_unreg(&rq->xdp_rxq);
>> >- if (rq->page_pool)
>> >- page_pool_destroy(rq->page_pool);
>> >-
>> > mlx5_wq_destroy(&rq->wq_ctrl);
>> > }
>> >
>> >diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>> >index 754d980700df..f09b3f1994e6 100644
>> >--- a/include/net/page_pool.h
>> >+++ b/include/net/page_pool.h
>> >@@ -16,14 +16,16 @@
>> > * page_pool_alloc_pages() call. Drivers should likely use
>> > * page_pool_dev_alloc_pages() replacing dev_alloc_pages().
>> > *
>> >- * If page_pool handles DMA mapping (use page->private), then API user
>> >- * is responsible for invoking page_pool_put_page() once. In-case of
>> >- * elevated refcnt, the DMA state is released, assuming other users of
>> >- * the page will eventually call put_page().
>> >+ * API keeps track of in-flight pages, in-order to let API user know
>> >+ * when it is safe to dealloactor page_pool object. Thus, API users
>> >+ * must make sure to call page_pool_release_page() when a page is
>> >+ * "leaving" the page_pool. Or call page_pool_put_page() where
>> >+ * appropiate. For maintaining correct accounting.
>> > *
>> >- * If no DMA mapping is done, then it can act as shim-layer that
>> >- * fall-through to alloc_page. As no state is kept on the page, the
>> >- * regular put_page() call is sufficient.
>> >+ * API user must only call page_pool_put_page() once on a page, as it
>> >+ * will either recycle the page, or in case of elevated refcnt, it
>> >+ * will release the DMA mapping and in-flight state accounting. We
>> >+ * hope to lift this requirement in the future.
>> > */
>> > #ifndef _NET_PAGE_POOL_H
>> > #define _NET_PAGE_POOL_H
>> >@@ -66,9 +68,10 @@ struct page_pool_params {
>> > };
>> >
>> > struct page_pool {
>> >- struct rcu_head rcu;
>> > struct page_pool_params p;
>> >
>> >+ u32 pages_state_hold_cnt;
>> >+
>> > /*
>> > * Data structure for allocation side
>> > *
>> >@@ -96,6 +99,8 @@ struct page_pool {
>> > * TODO: Implement bulk return pages into this structure.
>> > */
>> > struct ptr_ring ring;
>> >+
>> >+ atomic_t pages_state_release_cnt;
>> > };
>> >
>> > struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
>> >@@ -109,8 +114,6 @@ static inline struct page *page_pool_dev_alloc_pages(struct page_pool *pool)
>> >
>> > struct page_pool *page_pool_create(const struct page_pool_params *params);
>> >
>> >-void page_pool_destroy(struct page_pool *pool);
>> >-
>> > void __page_pool_free(struct page_pool *pool);
>> > static inline void page_pool_free(struct page_pool *pool)
>> > {
>> >@@ -143,6 +146,24 @@ static inline void page_pool_recycle_direct(struct page_pool *pool,
>> > __page_pool_put_page(pool, page, true);
>> > }
>> >
>> >+/* API user MUST have disconnected alloc-side (not allowed to call
>> >+ * page_pool_alloc_pages()) before calling this. The free-side can
>> >+ * still run concurrently, to handle in-flight packet-pages.
>> >+ *
>> >+ * A request to shutdown can fail (with false) if there are still
>> >+ * in-flight packet-pages.
>> >+ */
>> >+bool __page_pool_request_shutdown(struct page_pool *pool);
>> >+static inline bool page_pool_request_shutdown(struct page_pool *pool)
>> >+{
>> >+ /* When page_pool isn't compiled-in, net/core/xdp.c doesn't
>> >+ * allow registering MEM_TYPE_PAGE_POOL, but shield linker.
>> >+ */
>> >+#ifdef CONFIG_PAGE_POOL
>> >+ return __page_pool_request_shutdown(pool);
>> >+#endif
>> >+}
>>
>> The free side can ran in softirq, that means fast cache recycle is accessed.
>> And it increments not atomic pool->alloc.count.
>>
>> For instance While redirect, for remote interface, while .ndo_xdp_xmit the
>> xdp_return_frame_rx_napi(xdpf) is called everywhere in error path ....
^
|
>>
>> In the same time, simultaneously, the work queue can try one more
>> time to clear cash, calling __page_pool_request_shutdown()....
>>
>> Question, what prevents pool->alloc.count to be corrupted by race,
>> causing to wrong array num and as result wrong page to be unmapped/put ....or
>> even page leak. alloc.count usage is not protected,
>> __page_pool_request_shutdown() is called not from same rx NAPI, even not from
>> NAPI.
>>
>> Here, while alloc cache empty procedure in __page_pool_request_shutdown():
>
>You forgot to copy this comment, which explains:
>
> /* Empty alloc cache, assume caller made sure this is
> * no-longer in use, and page_pool_alloc_pages() cannot be
> * call concurrently.
> */
No I didn't. I'm talking about recycle, not allocation.
To be more specific about this:
__page_pool_recycle_direct() while remote ndev .ndo_xdp_xmit
About this:
"
/* API user MUST have disconnected alloc-side (not allowed to call
* page_pool_alloc_pages()) before calling this. The free-side can
* still run concurrently, to handle in-flight packet-pages.
"
>
>> while (pool->alloc.count) {
>> page = pool->alloc.cache[--pool->alloc.count];
>> __page_pool_return_page(pool, page);
>> }
>>
>> For me seems all works fine, but I can't find what have I missed?
>
>You have missed that, it is the drivers responsibility to "disconnect"
>the xdp_rxq_info before calling shutdown. Which means that it is not
>allowed to be used for RX, while the driver is shutting down a
>RX-queue. For drivers this is very natural, else other things will
>break.
>
>
>> ...
>>
>> Same question about how xdp frame should be returned for drivers running
>> tx napi exclusively, it can be still softirq but another CPU? What API
>> should be used to return xdp frame.
>
>You have to use the normal xdp_return_frame() which doesn't do "direct"
>return.
Oh, yes, sorry
/* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
page = virt_to_head_page(data);
if (likely(xa)) {
napi_direct &= !xdp_return_frame_no_direct();
page_pool_put_page(xa->page_pool, page, napi_direct);
just masked napi_direct on 1, but forgot it's zero from scratch......
>
>--
>Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* [PATCH iproute2 2/2] devlink: fix libc and kernel headers collision
From: Baruch Siach @ 2019-06-25 11:49 UTC (permalink / raw)
To: Stephen Hemminger, Jiri Pirko
Cc: netdev, Baruch Siach, Aya Levin, Moshe Shemesh
In-Reply-To: <016aabe2639668b4710b73157ea39e8f97f7d726.1561463345.git.baruch@tkos.co.il>
Since commit 2f1242efe9d ("devlink: Add devlink health show command") we
use the sys/sysinfo.h header for the sysinfo(2) system call. But since
iproute2 carries a local version of the kernel struct sysinfo, this
causes a collision with libc that do not rely on kernel defined sysinfo
like musl libc:
In file included from devlink.c:25:0:
.../sysroot/usr/include/sys/sysinfo.h:10:8: error: redefinition of 'struct sysinfo'
struct sysinfo {
^~~~~~~
In file included from ../include/uapi/linux/kernel.h:5:0,
from ../include/uapi/linux/netlink.h:5,
from ../include/uapi/linux/genetlink.h:6,
from devlink.c:21:
../include/uapi/linux/sysinfo.h:8:8: note: originally defined here
struct sysinfo {
^~~~~~~
Move the sys/sysinfo.h userspace header before kernel headers, and
suppress the indirect include of linux/sysinfo.h.
Cc: Aya Levin <ayal@mellanox.com>
Cc: Moshe Shemesh <moshe@mellanox.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
devlink/devlink.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index b400fab17578..2ea60d3556da 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -18,11 +18,16 @@
#include <limits.h>
#include <errno.h>
#include <inttypes.h>
+#include <sys/sysinfo.h>
+/* Suppress linux/sysinfo.h to avoid
+ * collision of struct sysinfo definition
+ * with musl libc headers
+ */
+#define _LINUX_SYSINFO_H
#include <linux/genetlink.h>
#include <linux/devlink.h>
#include <libmnl/libmnl.h>
#include <netinet/ether.h>
-#include <sys/sysinfo.h>
#include <sys/queue.h>
#include "SNAPSHOT.h"
--
2.20.1
^ permalink raw reply related
* [PATCH iproute2 1/2] devlink: fix format string warning for 32bit targets
From: Baruch Siach @ 2019-06-25 11:49 UTC (permalink / raw)
To: Stephen Hemminger, Jiri Pirko
Cc: netdev, Baruch Siach, Aya Levin, Moshe Shemesh
32bit targets define uint64_t as long long unsigned. This leads to the
following build warning:
devlink.c: In function ‘pr_out_u64’:
devlink.c:1729:11: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t {aka long long unsigned int}’ [-Wformat=]
pr_out("%s %lu", name, val);
^
devlink.c:59:21: note: in definition of macro ‘pr_out’
fprintf(stdout, ##args); \
^~~~
Use 'll' length modifier in the format string to fix that.
Cc: Aya Levin <ayal@mellanox.com>
Cc: Moshe Shemesh <moshe@mellanox.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
devlink/devlink.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 436935f88bda..b400fab17578 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -1726,9 +1726,9 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
jsonw_u64_field(dl->jw, name, val);
} else {
if (g_indent_newline)
- pr_out("%s %lu", name, val);
+ pr_out("%s %llu", name, val);
else
- pr_out(" %s %lu", name, val);
+ pr_out(" %s %llu", name, val);
}
}
@@ -1753,7 +1753,7 @@ static void pr_out_uint64_value(struct dl *dl, uint64_t value)
if (dl->json_output)
jsonw_u64(dl->jw, value);
else
- pr_out(" %lu", value);
+ pr_out(" %llu", value);
}
static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: René van Dorst @ 2019-06-25 11:43 UTC (permalink / raw)
To: Daniel Santos
Cc: sean.wang, f.fainelli, linux, davem, matthias.bgg, andrew,
vivien.didelot, frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <13c67cb7-b33e-f2b1-9d1e-d2882e0ff076@pobox.com>
Quoting Daniel Santos <daniel.santos@pobox.com>:
Hi Daniel,
> On 6/24/19 9:52 AM, René van Dorst wrote:
>> Convert mt7530 to PHYLINK API
>>
>> Signed-off-by: René van Dorst <opensource@vdorst.com>
>> ---
>> drivers/net/dsa/mt7530.c | 237 +++++++++++++++++++++++++++++----------
>> drivers/net/dsa/mt7530.h | 9 ++
>> 2 files changed, 187 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
>> index 3181e95586d6..9c5e4dd00826 100644
>> --- a/drivers/net/dsa/mt7530.c
>> +++ b/drivers/net/dsa/mt7530.c
>> @@ -13,7 +13,7 @@
>> #include <linux/of_mdio.h>
>> #include <linux/of_net.h>
>> #include <linux/of_platform.h>
>> -#include <linux/phy.h>
>> +#include <linux/phylink.h>
>> #include <linux/regmap.h>
>> #include <linux/regulator/consumer.h>
>> #include <linux/reset.h>
>> @@ -633,63 +633,6 @@ mt7530_get_sset_count(struct dsa_switch *ds,
>> int port, int sset)
>> return ARRAY_SIZE(mt7530_mib);
>> }
>>
>> -static void mt7530_adjust_link(struct dsa_switch *ds, int port,
>> - struct phy_device *phydev)
>> -{
>> - struct mt7530_priv *priv = ds->priv;
>> -
>> - if (phy_is_pseudo_fixed_link(phydev)) {
>> - dev_dbg(priv->dev, "phy-mode for master device = %x\n",
>> - phydev->interface);
>> -
>> - /* Setup TX circuit incluing relevant PAD and driving */
>> - mt7530_pad_clk_setup(ds, phydev->interface);
>> -
>> - if (priv->id == ID_MT7530) {
>> - /* Setup RX circuit, relevant PAD and driving on the
>> - * host which must be placed after the setup on the
>> - * device side is all finished.
>> - */
>> - mt7623_pad_clk_setup(ds);
>> - }
>> - } else {
>> - u16 lcl_adv = 0, rmt_adv = 0;
>> - u8 flowctrl;
>> - u32 mcr = PMCR_USERP_LINK | PMCR_FORCE_MODE;
>> -
>> - switch (phydev->speed) {
>> - case SPEED_1000:
>> - mcr |= PMCR_FORCE_SPEED_1000;
>> - break;
>> - case SPEED_100:
>> - mcr |= PMCR_FORCE_SPEED_100;
>> - break;
>> - }
>> -
>> - if (phydev->link)
>> - mcr |= PMCR_FORCE_LNK;
>> -
>> - if (phydev->duplex) {
>> - mcr |= PMCR_FORCE_FDX;
>> -
>> - if (phydev->pause)
>> - rmt_adv = LPA_PAUSE_CAP;
>> - if (phydev->asym_pause)
>> - rmt_adv |= LPA_PAUSE_ASYM;
>> -
>> - lcl_adv = linkmode_adv_to_lcl_adv_t(
>> - phydev->advertising);
>> - flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
>> -
>> - if (flowctrl & FLOW_CTRL_TX)
>> - mcr |= PMCR_TX_FC_EN;
>> - if (flowctrl & FLOW_CTRL_RX)
>> - mcr |= PMCR_RX_FC_EN;
>> - }
>> - mt7530_write(priv, MT7530_PMCR_P(port), mcr);
>> - }
>> -}
>> -
>> static int
>> mt7530_cpu_port_enable(struct mt7530_priv *priv,
>> int port)
>> @@ -1323,6 +1266,178 @@ mt7530_setup(struct dsa_switch *ds)
>> return 0;
>> }
>>
>> +static void mt7530_phylink_mac_config(struct dsa_switch *ds, int port,
>> + unsigned int mode,
>> + const struct phylink_link_state *state)
>> +{
>> + struct mt7530_priv *priv = ds->priv;
>> + u32 mcr = PMCR_IFG_XMIT(1) | PMCR_MAC_MODE | PMCR_BACKOFF_EN |
>> + PMCR_BACKPR_EN | PMCR_TX_EN | PMCR_RX_EN;
>> +
>> + switch (port) {
>> + case 0: /* Internal phy */
>> + case 1:
>> + case 2:
>> + case 3:
>> + case 4:
>> + if (state->interface != PHY_INTERFACE_MODE_GMII)
>> + goto unsupported;
>> + break;
>> + /* case 5: Port 5 is not supported! */
>> + case 6: /* 1st cpu port */
>> + if (state->interface != PHY_INTERFACE_MODE_RGMII &&
>> + state->interface != PHY_INTERFACE_MODE_TRGMII)
>> + goto unsupported;
>> +
>> + /* Setup TX circuit incluing relevant PAD and driving */
>> + mt7530_pad_clk_setup(ds, state->interface);
>> +
>> + if (priv->id == ID_MT7530) {
>> + /* Setup RX circuit, relevant PAD and driving on the
>> + * host which must be placed after the setup on the
>> + * device side is all finished.
>> + */
>> + mt7623_pad_clk_setup(ds);
>> + }
>> + break;
>> + default:
>> + dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
>> + return;
>> + }
>> +
>> + if (!state->an_enabled || mode == MLO_AN_FIXED) {
>> + mcr |= PMCR_FORCE_MODE;
>> +
>> + if (state->speed == SPEED_1000)
>> + mcr |= PMCR_FORCE_SPEED_1000;
>> + if (state->speed == SPEED_100)
>> + mcr |= PMCR_FORCE_SPEED_100;
>
> I would suggest using the defines
>
> #define PMCR_FORCE_SPEED 0x0000000c /* or PMCR_FORCE_SPEED_MASK */
> #define PMCR_FORCE_SPEED_10 0x00000000
> #define PMCR_FORCE_SPEED_100 0x00000004
> #define PMCR_FORCE_SPEED_1000 0x00000008
>
> and a switch statement such as
>
> switch (state->speed) {
> case SPEED_10:
> mcr |= PMCR_FORCE_SPEED_10;
> break;
> case SPEED_100:
> mcr |= PMCR_FORCE_SPEED_100;
> break;
> case SPEED_1000:
> mcr |= PMCR_FORCE_SPEED_1000;
> break;
> }
>
> This will compile the same (i.e, the mcr |= 0 will optimize away, etc.),
> while alleviating the need to intimately know the hardware in order to
> easily understand what the code is doing at a glance. It's also better
> form as we're treating the two bits as a composite value, rather than
> two separate bits.
I will change it based on your surgestion.
>
>
>> + if (state->duplex == DUPLEX_FULL)
>> + mcr |= PMCR_FORCE_FDX;
>> + if (state->link || mode == MLO_AN_FIXED)
>> + mcr |= PMCR_FORCE_LNK;
>> + if (state->pause || phylink_test(state->advertising, Pause))
>> + mcr |= PMCR_TX_FC_EN | PMCR_RX_FC_EN;
>> + if (state->pause & MLO_PAUSE_TX)
>> + mcr |= PMCR_TX_FC_EN;
>> + if (state->pause & MLO_PAUSE_RX)
>> + mcr |= PMCR_RX_FC_EN;
>> + }
>> +
>> + mt7530_write(priv, MT7530_PMCR_P(port), mcr);
>> +
>> + return;
>> +
>> +unsupported:
>> + dev_err(ds->dev, "%s: P%d: Unsupported phy_interface mode: %d (%s)\n",
>> + __func__, port, state->interface, phy_modes(state->interface));
>> +}
>> +
>> +static void mt7530_phylink_mac_link_down(struct dsa_switch *ds, int port,
>> + unsigned int mode,
>> + phy_interface_t interface)
>> +{
>> + /* Do nothing */
>> +}
>> +
>> +static void mt7530_phylink_mac_link_up(struct dsa_switch *ds, int port,
>> + unsigned int mode,
>> + phy_interface_t interface,
>> + struct phy_device *phydev)
>> +{
>> + /* Do nothing */
>> +}
>> +
>> +static void mt7530_phylink_validate(struct dsa_switch *ds, int port,
>> + unsigned long *supported,
>> + struct phylink_link_state *state)
>> +{
>> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>> +
>> + switch (port) {
>> + case 0: /* Internal phy */
>> + case 1:
>> + case 2:
>> + case 3:
>> + case 4:
>> + if (state->interface != PHY_INTERFACE_MODE_NA &&
>> + state->interface != PHY_INTERFACE_MODE_GMII)
>> + goto unsupported;
>> + break;
>> + /* case 5: Port 5 not supported! */
>> + case 6: /* 1st cpu port */
>> + if (state->interface != PHY_INTERFACE_MODE_RGMII &&
>> + state->interface != PHY_INTERFACE_MODE_TRGMII)
>> + goto unsupported;
>> + break;
>> + default:
>> + linkmode_zero(supported);
>> + dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
>> + return;
>> + }
>> +
>> + phylink_set(mask, Autoneg);
>> + phylink_set(mask, Pause);
>> + phylink_set(mask, Asym_Pause);
>> + phylink_set(mask, MII);
>> +
>> + phylink_set(mask, 10baseT_Half);
>> + phylink_set(mask, 10baseT_Full);
>> + phylink_set(mask, 100baseT_Half);
>> + phylink_set(mask, 100baseT_Full);
>> + phylink_set(mask, 1000baseT_Full);
>> + phylink_set(mask, 1000baseT_Half);
>> +
>> + linkmode_and(supported, supported, mask);
>> + linkmode_and(state->advertising, state->advertising, mask);
>> + return;
>> +
>> +unsupported:
>> + linkmode_zero(supported);
>> + dev_err(ds->dev, "%s: unsupported interface mode: [0x%x] %s\n",
>> + __func__, state->interface, phy_modes(state->interface));
>> +}
>> +
>> +static int
>> +mt7530_phylink_mac_link_state(struct dsa_switch *ds, int port,
>> + struct phylink_link_state *state)
>> +{
>> + struct mt7530_priv *priv = ds->priv;
>> + u32 pmsr;
>> +
>> + if (port < 0 || port >= MT7530_NUM_PORTS)
>> + return -EINVAL;
>> +
>> + pmsr = mt7530_read(priv, MT7530_PMSR_P(port));
>> +
>> + state->link = (pmsr & PMSR_LINK);
>> + state->an_complete = state->link;
>> + state->duplex = (pmsr & PMSR_DPX) >> 1;
>> +
>> + switch (pmsr & (PMSR_SPEED_1000 | PMSR_SPEED_100)) {
>> + case 0:
>> + state->speed = SPEED_10;
>> + break;
>> + case PMSR_SPEED_100:
>> + state->speed = SPEED_100;
>> + break;
>> + case PMSR_SPEED_1000:
>> + state->speed = SPEED_1000;
>> + break;
>> + default:
>> + state->speed = SPEED_UNKNOWN;
>> + break;
>> + }
>> +
>
> Same as above: add PMSR_SPEED_10, and and with PMSR_SPEED (or
> PMSR_SPEED_MASK if you prefer).
Same as above.
>
>> + state->pause = 0;
>> + if (pmsr & PMSR_RX_FC)
>> + state->pause |= MLO_PAUSE_RX;
>> + if (pmsr & PMSR_TX_FC)
>> + state->pause |= MLO_PAUSE_TX;
>> +
>> + return 1;
>> +}
>> +
>> static const struct dsa_switch_ops mt7530_switch_ops = {
>> .get_tag_protocol = mtk_get_tag_protocol,
>> .setup = mt7530_setup,
>> @@ -1331,7 +1446,6 @@ static const struct dsa_switch_ops
>> mt7530_switch_ops = {
>> .phy_write = mt7530_phy_write,
>> .get_ethtool_stats = mt7530_get_ethtool_stats,
>> .get_sset_count = mt7530_get_sset_count,
>> - .adjust_link = mt7530_adjust_link,
>> .port_enable = mt7530_port_enable,
>> .port_disable = mt7530_port_disable,
>> .port_stp_state_set = mt7530_stp_state_set,
>> @@ -1344,6 +1458,11 @@ static const struct dsa_switch_ops
>> mt7530_switch_ops = {
>> .port_vlan_prepare = mt7530_port_vlan_prepare,
>> .port_vlan_add = mt7530_port_vlan_add,
>> .port_vlan_del = mt7530_port_vlan_del,
>> + .phylink_validate = mt7530_phylink_validate,
>> + .phylink_mac_link_state = mt7530_phylink_mac_link_state,
>> + .phylink_mac_config = mt7530_phylink_mac_config,
>> + .phylink_mac_link_down = mt7530_phylink_mac_link_down,
>> + .phylink_mac_link_up = mt7530_phylink_mac_link_up,
>> };
>>
>> static const struct of_device_id mt7530_of_match[] = {
>> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
>> index bfac90f48102..41d9a132ac70 100644
>> --- a/drivers/net/dsa/mt7530.h
>> +++ b/drivers/net/dsa/mt7530.h
>> @@ -198,6 +198,7 @@ enum mt7530_vlan_port_attr {
>> #define PMCR_FORCE_SPEED_100 BIT(2)
>> #define PMCR_FORCE_FDX BIT(1)
>> #define PMCR_FORCE_LNK BIT(0)
>> +#define PMCR_FORCE_LNK_DOWN PMCR_FORCE_MODE
>> #define PMCR_COMMON_LINK (PMCR_IFG_XMIT(1) | PMCR_MAC_MODE | \
>> PMCR_BACKOFF_EN | PMCR_BACKPR_EN | \
>> PMCR_TX_EN | PMCR_RX_EN | \
>> @@ -218,6 +219,14 @@ enum mt7530_vlan_port_attr {
>> PMCR_TX_FC_EN | PMCR_RX_FC_EN)
>>
>> #define MT7530_PMSR_P(x) (0x3008 + (x) * 0x100)
>> +#define PMSR_EEE1G BIT(7)
>> +#define PMSR_EEE100M BIT(6)
>> +#define PMSR_RX_FC BIT(5)
>> +#define PMSR_TX_FC BIT(4)
>> +#define PMSR_SPEED_1000 BIT(3)
>> +#define PMSR_SPEED_100 BIT(2)
>> +#define PMSR_DPX BIT(1)
>> +#define PMSR_LINK BIT(0)
>>
>> /* Register for MIB */
>> #define MT7530_PORT_MIB_COUNTER(x) (0x4000 + (x) * 0x100)
>
> Cheers,
> Daniel
Greats,
René
^ 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