* net: suspicious RCU usage in nf_hook
From: Dmitry Vyukov @ 2017-01-27 21:15 UTC (permalink / raw)
To: David Miller, Alexey Kuznetsov, Eric Dumazet, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML,
Pablo Neira Ayuso, netfilter-devel
Cc: syzkaller
Hello,
I've got the following report while running syzkaller fuzzer on
fd694aaa46c7ed811b72eb47d5eb11ce7ab3f7f1:
[ INFO: suspicious RCU usage. ]
4.10.0-rc5+ #192 Not tainted
-------------------------------
./include/linux/rcupdate.h:561 Illegal context switch in RCU read-side
critical section!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 0
2 locks held by syz-executor14/23111:
#0: (sk_lock-AF_INET6){+.+.+.}, at: [<ffffffff83a35c35>] lock_sock
include/net/sock.h:1454 [inline]
#0: (sk_lock-AF_INET6){+.+.+.}, at: [<ffffffff83a35c35>]
rawv6_sendmsg+0x1e65/0x3ec0 net/ipv6/raw.c:919
#1: (rcu_read_lock){......}, at: [<ffffffff83ae2678>] nf_hook
include/linux/netfilter.h:201 [inline]
#1: (rcu_read_lock){......}, at: [<ffffffff83ae2678>]
__ip6_local_out+0x258/0x840 net/ipv6/output_core.c:160
stack backtrace:
CPU: 2 PID: 23111 Comm: syz-executor14 Not tainted 4.10.0-rc5+ #192
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:15 [inline]
dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
lockdep_rcu_suspicious+0x139/0x180 kernel/locking/lockdep.c:4452
rcu_preempt_sleep_check include/linux/rcupdate.h:560 [inline]
___might_sleep+0x560/0x650 kernel/sched/core.c:7748
__might_sleep+0x95/0x1a0 kernel/sched/core.c:7739
mutex_lock_nested+0x24f/0x1730 kernel/locking/mutex.c:752
atomic_dec_and_mutex_lock+0x119/0x160 kernel/locking/mutex.c:1060
__static_key_slow_dec+0x7a/0x1e0 kernel/jump_label.c:149
static_key_slow_dec+0x51/0x90 kernel/jump_label.c:174
net_disable_timestamp+0x3b/0x50 net/core/dev.c:1728
sock_disable_timestamp+0x98/0xc0 net/core/sock.c:403
__sk_destruct+0x27d/0x6b0 net/core/sock.c:1441
sk_destruct+0x47/0x80 net/core/sock.c:1460
__sk_free+0x57/0x230 net/core/sock.c:1468
sock_wfree+0xae/0x120 net/core/sock.c:1645
skb_release_head_state+0xfc/0x200 net/core/skbuff.c:655
skb_release_all+0x15/0x60 net/core/skbuff.c:668
__kfree_skb+0x15/0x20 net/core/skbuff.c:684
kfree_skb+0x16e/0x4c0 net/core/skbuff.c:705
inet_frag_destroy+0x121/0x290 net/ipv4/inet_fragment.c:304
inet_frag_put include/net/inet_frag.h:133 [inline]
nf_ct_frag6_gather+0x1106/0x3840 net/ipv6/netfilter/nf_conntrack_reasm.c:617
ipv6_defrag+0x1be/0x2b0 net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68
nf_hook_entry_hookfn include/linux/netfilter.h:102 [inline]
nf_hook_slow+0xc3/0x290 net/netfilter/core.c:310
nf_hook include/linux/netfilter.h:212 [inline]
__ip6_local_out+0x489/0x840 net/ipv6/output_core.c:160
ip6_local_out+0x2d/0x170 net/ipv6/output_core.c:170
ip6_send_skb+0xa1/0x340 net/ipv6/ip6_output.c:1722
ip6_push_pending_frames+0xb3/0xe0 net/ipv6/ip6_output.c:1742
rawv6_push_pending_frames net/ipv6/raw.c:613 [inline]
rawv6_sendmsg+0x2d1a/0x3ec0 net/ipv6/raw.c:927
inet_sendmsg+0x164/0x5b0 net/ipv4/af_inet.c:744
sock_sendmsg_nosec net/socket.c:635 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:645
sock_write_iter+0x326/0x600 net/socket.c:848
do_iter_readv_writev+0x2e3/0x5b0 fs/read_write.c:695
do_readv_writev+0x42c/0x9b0 fs/read_write.c:872
vfs_writev+0x87/0xc0 fs/read_write.c:911
do_writev+0x110/0x2c0 fs/read_write.c:944
SYSC_writev fs/read_write.c:1017 [inline]
SyS_writev+0x27/0x30 fs/read_write.c:1014
entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x445559
RSP: 002b:00007f6f46fceb58 EFLAGS: 00000292 ORIG_RAX: 0000000000000014
RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 0000000000445559
RDX: 0000000000000001 RSI: 0000000020f1eff0 RDI: 0000000000000005
RBP: 00000000006e19c0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000292 R12: 0000000000700000
R13: 0000000020f59000 R14: 0000000000000015 R15: 0000000000020400
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:752
in_atomic(): 1, irqs_disabled(): 0, pid: 23111, name: syz-executor14
INFO: lockdep is turned off.
CPU: 2 PID: 23111 Comm: syz-executor14 Not tainted 4.10.0-rc5+ #192
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:15 [inline]
dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
___might_sleep+0x47e/0x650 kernel/sched/core.c:7780
__might_sleep+0x95/0x1a0 kernel/sched/core.c:7739
mutex_lock_nested+0x24f/0x1730 kernel/locking/mutex.c:752
atomic_dec_and_mutex_lock+0x119/0x160 kernel/locking/mutex.c:1060
__static_key_slow_dec+0x7a/0x1e0 kernel/jump_label.c:149
static_key_slow_dec+0x51/0x90 kernel/jump_label.c:174
net_disable_timestamp+0x3b/0x50 net/core/dev.c:1728
sock_disable_timestamp+0x98/0xc0 net/core/sock.c:403
__sk_destruct+0x27d/0x6b0 net/core/sock.c:1441
sk_destruct+0x47/0x80 net/core/sock.c:1460
__sk_free+0x57/0x230 net/core/sock.c:1468
sock_wfree+0xae/0x120 net/core/sock.c:1645
skb_release_head_state+0xfc/0x200 net/core/skbuff.c:655
skb_release_all+0x15/0x60 net/core/skbuff.c:668
__kfree_skb+0x15/0x20 net/core/skbuff.c:684
kfree_skb+0x16e/0x4c0 net/core/skbuff.c:705
inet_frag_destroy+0x121/0x290 net/ipv4/inet_fragment.c:304
inet_frag_put include/net/inet_frag.h:133 [inline]
nf_ct_frag6_gather+0x1106/0x3840 net/ipv6/netfilter/nf_conntrack_reasm.c:617
ipv6_defrag+0x1be/0x2b0 net/ipv6/netfilter/nf_defrag_ipv6_hooks.c:68
nf_hook_entry_hookfn include/linux/netfilter.h:102 [inline]
nf_hook_slow+0xc3/0x290 net/netfilter/core.c:310
nf_hook include/linux/netfilter.h:212 [inline]
__ip6_local_out+0x489/0x840 net/ipv6/output_core.c:160
ip6_local_out+0x2d/0x170 net/ipv6/output_core.c:170
ip6_send_skb+0xa1/0x340 net/ipv6/ip6_output.c:1722
ip6_push_pending_frames+0xb3/0xe0 net/ipv6/ip6_output.c:1742
rawv6_push_pending_frames net/ipv6/raw.c:613 [inline]
rawv6_sendmsg+0x2d1a/0x3ec0 net/ipv6/raw.c:927
inet_sendmsg+0x164/0x5b0 net/ipv4/af_inet.c:744
sock_sendmsg_nosec net/socket.c:635 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:645
sock_write_iter+0x326/0x600 net/socket.c:848
do_iter_readv_writev+0x2e3/0x5b0 fs/read_write.c:695
do_readv_writev+0x42c/0x9b0 fs/read_write.c:872
vfs_writev+0x87/0xc0 fs/read_write.c:911
do_writev+0x110/0x2c0 fs/read_write.c:944
SYSC_writev fs/read_write.c:1017 [inline]
SyS_writev+0x27/0x30 fs/read_write.c:1014
entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x445559
RSP: 002b:00007f6f46fceb58 EFLAGS: 00000292 ORIG_RAX: 0000000000000014
RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 0000000000445559
RDX: 0000000000000001 RSI: 0000000020f1eff0 RDI: 0000000000000005
RBP: 00000000006e19c0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000292 R12: 0000000000700000
R13: 0000000020f59000 R14: 0000000000000015 R15: 0000000000020400
BUG: scheduling while atomic: syz-executor14/23111/0x00000002
INFO: lockdep is turned off.
Modules linked in:
Kernel panic - not syncing: scheduling while atomic
^ permalink raw reply
* Re: [net-next] openvswitch: Simplify do_execute_actions().
From: Andy Zhou @ 2017-01-27 21:25 UTC (permalink / raw)
To: Pravin Shelar; +Cc: David S. Miller, Linux Kernel Network Developers
In-Reply-To: <CAOrHB_C3C_pcPpS=TSvKD0pF8eKRZMVgCK6eNKfbJnT_-xs1xw@mail.gmail.com>
On Fri, Jan 27, 2017 at 12:42 PM, Pravin Shelar <pshelar@ovn.org> wrote:
> On Wed, Jan 25, 2017 at 9:24 PM, Andy Zhou <azhou@ovn.org> wrote:
>> do_execute_actions() implements a worthwhile optimization: in case
>> an output action is the last action in an action list, skb_clone()
>> can be avoided by outputing the current skb. However, the
>> implementation is more complicated than necessary. This patch
>> simplify this logic.
>>
>> Signed-off-by: Andy Zhou <azhou@ovn.org>
>> ---
>> net/openvswitch/actions.c | 40 +++++++++++++++++++---------------------
>> 1 file changed, 19 insertions(+), 21 deletions(-)
>>
>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>> index 514f7bc..3866608 100644
>> --- a/net/openvswitch/actions.c
>> +++ b/net/openvswitch/actions.c
>> @@ -830,6 +830,9 @@ static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port,
>> {
>> struct vport *vport = ovs_vport_rcu(dp, out_port);
>>
>> + if (unlikely(!skb))
>> + return;
>> +
> Patch looks good to me. But I wanted to know if you considered moving
> this check to do_execute_actions() in case skb-clone is done? This way
> we can avoid this unlikely check from likely case :)
>
Good point. O.K. I will repost a version without this check. Thanks
for the review and comment.
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: dsa: bcm_sf2: CFP support
From: Chris Healy @ 2017-01-27 21:24 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, Vivien Didelot, Pablo Neira Ayuso,
netdev@vger.kernel.org, David S. Miller
In-Reply-To: <20170127210516.16704-1-f.fainelli@gmail.com>
Hi Florian,
In saying the below, I may just be showing my naivety but here goes:
If I understand this correctly, what you are using is similar to the
TCAM hardware present in the newer Marvell switches. I think Pablo is
doing some work with nftables and HW offload using TCAM HW. Is there
overlap here? It seems that one or the other API should be used but
not both.
Regards,
Chris
On Fri, Jan 27, 2017 at 1:05 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Hi all,
>
> This patch series adds support for the Broadcom Compact Field Processor (CFP)
> which is a classification and matching engine built into most Broadcom switches.
>
> We support that using ethtool::rxnfc because it allows all known uses cases from
> the users I support to work, and more importantly, it allows the selection of a
> target rule index, which is later used by e.g: offloading hardware, this is an
> essential feature that I could not find being supported with cls_* for instance.
>
> Thanks
>
> Florian Fainelli (4):
> net: dsa: Hook {get,set}_rxnfc ethtool operations
> net: dsa: bcm_sf2: Configure traffic classes to queue mapping
> net: dsa: bcm_sf2: Add CFP registers definitions
> net: dsa: bcm_sf2: Add support for ethtool::rxnfc
>
> drivers/net/dsa/Makefile | 2 +-
> drivers/net/dsa/bcm_sf2.c | 23 ++
> drivers/net/dsa/bcm_sf2.h | 17 ++
> drivers/net/dsa/bcm_sf2_cfp.c | 613 +++++++++++++++++++++++++++++++++++++++++
> drivers/net/dsa/bcm_sf2_regs.h | 150 ++++++++++
> include/net/dsa.h | 8 +
> net/dsa/slave.c | 26 ++
> 7 files changed, 838 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/dsa/bcm_sf2_cfp.c
>
> --
> 2.9.3
>
^ permalink raw reply
* [RFC PATCH 0/2] rx zero copy interface for af_packet
From: John Fastabend @ 2017-01-27 21:33 UTC (permalink / raw)
To: bjorn.topel, jasowang, ast, alexander.duyck, brouer
Cc: john.r.fastabend, netdev, john.fastabend
This is an experimental implementation of rx zero copy for af_packet.
Its a bit rough and likely has errors but the plan is to clean it up
over the next few months.
And seeing I said I would post it in another thread a few days back
here it is.
Comments welcome and use at your own risk.
Thanks,
John
---
John Fastabend (2):
af_packet: direct dma for packet ineterface
ixgbe: add af_packet direct copy support
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 3
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 255 +++++++++++++++++++++++++
include/linux/netdevice.h | 8 +
include/net/af_packet.h | 64 ++++++
include/uapi/linux/if_packet.h | 1
net/packet/af_packet.c | 37 ++++
net/packet/internal.h | 60 ------
tools/testing/selftests/net/psock_tpacket.c | 51 ++++-
8 files changed, 410 insertions(+), 69 deletions(-)
create mode 100644 include/net/af_packet.h
^ permalink raw reply
* [RFC PATCH 1/2] af_packet: direct dma for packet ineterface
From: John Fastabend @ 2017-01-27 21:33 UTC (permalink / raw)
To: bjorn.topel, jasowang, ast, alexander.duyck, brouer
Cc: john.r.fastabend, netdev, john.fastabend
In-Reply-To: <20170127213132.14162.82951.stgit@john-Precision-Tower-5810>
This adds ndo ops for upper layer objects to request direct DMA from
the network interface into memory "slots". The slots must be DMA'able
memory given by a page/offset/size vector in a packet_ring_buffer
structure.
The PF_PACKET socket interface can use these ndo_ops to do zerocopy
RX from the network device into memory mapped userspace memory. For
this to work drivers encode the correct descriptor blocks and headers
so that existing PF_PACKET applications work without any modification.
This only supports the V2 header formats for now. And works by mapping
a ring of the network device to these slots. Originally I used V2
header formats but this does complicate the driver a bit.
V3 header formats added bulk polling via socket calls and timers
used in the polling interface to return every n milliseconds. Currently,
I don't see any way to support this in hardware because we can't
know if the hardware is in the middle of a DMA operation or not
on a slot. So when a timer fires I don't know how to advance the
descriptor ring leaving empty descriptors similar to how the software
ring works. The easiest (best?) route is to simply not support this.
It might be worth creating a new v4 header that is simple for drivers
to support direct DMA ops with. I can imagine using the xdp_buff
structure as a header for example. Thoughts?
The ndo operations and new socket option PACKET_RX_DIRECT work by
giving a queue_index to run the direct dma operations over. Once
setsockopt returns successfully the indicated queue is mapped
directly to the requesting application and can not be used for
other purposes. Also any kernel layers such as tc will be bypassed
and need to be implemented in the hardware via some other mechanism
such as tc offload or other offload interfaces.
Users steer traffic to the selected queue using flow director,
tc offload infrastructure or via macvlan offload.
The new socket option added to PF_PACKET is called PACKET_RX_DIRECT.
It takes a single unsigned int value specifying the queue index,
setsockopt(sock, SOL_PACKET, PACKET_RX_DIRECT,
&queue_index, sizeof(queue_index));
Implementing busy_poll support will allow userspace to kick the
drivers receive routine if needed. This work is TBD.
To test this I hacked a hardcoded test into the tool psock_tpacket
in the selftests kernel directory here:
./tools/testing/selftests/net/psock_tpacket.c
Running this tool opens a socket and listens for packets over
the PACKET_RX_DIRECT enabled socket. Obviously it needs to be
reworked to enable all the older tests and not hardcode my
interface before it actually gets released.
In general this is a rough patch to explore the interface and
put something concrete up for debate. The patch does not handle
all the error cases correctly and needs to be cleaned up.
Known Limitations (TBD):
(1) Users are required to match the number of rx ring
slots with ethtool to the number requested by the
setsockopt PF_PACKET layout. In the future we could
possibly do this automatically.
(2) Users need to configure Flow director or setup_tc
to steer traffic to the correct queues. I don't believe
this needs to be changed it seems to be a good mechanism
for driving directed dma.
(3) Not supporting timestamps or priv space yet, pushing
a v4 packet header would resolve this nicely.
(5) Only RX supported so far. TX already supports direct DMA
interface but uses skbs which is really not needed. In
the TX_RING case we can optimize this path as well.
To support TX case we can do a similar "slots" mechanism and
kick operation. The kick could be a busy_poll like operation
but on the TX side. The flow would be user space loads up
n number of slots with packets, kicks tx busy poll bit, the
driver sends packets, and finally when xmit is complete
clears header bits to give slots back. When we have qdisc
bypass set today we already bypass the entire stack so no
paticular reason to use skb's in this case. Using xdp_buff
as a v4 packet header would also allow us to consolidate
driver code.
To be done:
(1) More testing and performance analysis
(2) Busy polling sockets
(3) Implement v4 xdp_buff headers for analysis
(4) performance testing :/ hopefully it looks good.
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
include/linux/netdevice.h | 8 +++
include/net/af_packet.h | 64 +++++++++++++++++++++++++++
include/uapi/linux/if_packet.h | 1
net/packet/af_packet.c | 37 ++++++++++++++++
net/packet/internal.h | 60 -------------------------
tools/testing/selftests/net/psock_tpacket.c | 51 +++++++++++++++++++---
6 files changed, 154 insertions(+), 67 deletions(-)
create mode 100644 include/net/af_packet.h
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9bde955..a64a333 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -54,6 +54,8 @@
#include <uapi/linux/pkt_cls.h>
#include <linux/hashtable.h>
+#include <net/af_packet.h>
+
struct netpoll_info;
struct device;
struct phy_device;
@@ -1324,6 +1326,12 @@ struct net_device_ops {
int needed_headroom);
int (*ndo_xdp)(struct net_device *dev,
struct netdev_xdp *xdp);
+ int (*ndo_ddma_map)(struct net_device *dev,
+ unsigned int rindex,
+ struct sock *sk,
+ struct packet_ring_buffer *rb);
+ void (*ndo_ddma_unmap)(struct net_device *dev,
+ unsigned int rindex);
};
/**
diff --git a/include/net/af_packet.h b/include/net/af_packet.h
new file mode 100644
index 0000000..9e82ba1
--- /dev/null
+++ b/include/net/af_packet.h
@@ -0,0 +1,64 @@
+#include <linux/timer.h>
+
+struct pgv {
+ char *buffer;
+};
+
+/* kbdq - kernel block descriptor queue */
+struct tpacket_kbdq_core {
+ struct pgv *pkbdq;
+ unsigned int feature_req_word;
+ unsigned int hdrlen;
+ unsigned char reset_pending_on_curr_blk;
+ unsigned char delete_blk_timer;
+ unsigned short kactive_blk_num;
+ unsigned short blk_sizeof_priv;
+
+
+ /* last_kactive_blk_num:
+ * trick to see if user-space has caught up
+ * in order to avoid refreshing timer when every single pkt arrives.
+ */
+ unsigned short last_kactive_blk_num;
+
+ char *pkblk_start;
+ char *pkblk_end;
+ int kblk_size;
+ unsigned int max_frame_len;
+ unsigned int knum_blocks;
+ uint64_t knxt_seq_num;
+ char *prev;
+ char *nxt_offset;
+ struct sk_buff *skb;
+
+ atomic_t blk_fill_in_prog;
+
+ /* Default is set to 8ms */
+#define DEFAULT_PRB_RETIRE_TOV (8)
+
+ unsigned short retire_blk_tov;
+ unsigned short version;
+ unsigned long tov_in_jiffies;
+
+ /* timer to retire an outstanding block */
+ struct timer_list retire_blk_timer;
+};
+
+struct packet_ring_buffer {
+ struct pgv *pg_vec;
+
+ unsigned int head;
+ unsigned int frames_per_block;
+ unsigned int frame_size;
+ unsigned int frame_max;
+
+ unsigned int pg_vec_order;
+ unsigned int pg_vec_pages;
+ unsigned int pg_vec_len;
+
+ unsigned int __percpu *pending_refcnt;
+
+ bool ddma;
+
+ struct tpacket_kbdq_core prb_bdqc;
+};
diff --git a/include/uapi/linux/if_packet.h b/include/uapi/linux/if_packet.h
index 9e7edfd..04b069a 100644
--- a/include/uapi/linux/if_packet.h
+++ b/include/uapi/linux/if_packet.h
@@ -56,6 +56,7 @@ struct sockaddr_ll {
#define PACKET_QDISC_BYPASS 20
#define PACKET_ROLLOVER_STATS 21
#define PACKET_FANOUT_DATA 22
+#define PACKET_RX_DIRECT 23
#define PACKET_FANOUT_HASH 0
#define PACKET_FANOUT_LB 1
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 3d555c7..180666f 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -3731,6 +3731,34 @@ static void packet_flush_mclist(struct sock *sk)
po->xmit = val ? packet_direct_xmit : dev_queue_xmit;
return 0;
}
+ case PACKET_RX_DIRECT:
+ {
+ struct packet_ring_buffer *rb = &po->rx_ring;
+ struct net_device *dev;
+ unsigned int index;
+ int err;
+
+ if (optlen != sizeof(index))
+ return -EINVAL;
+ if (copy_from_user(&index, optval, sizeof(index)))
+ return -EFAULT;
+
+ /* This call only works after a bind call which calls a dev_hold
+ * operation so we do not need to increment dev ref counter
+ */
+ dev = __dev_get_by_index(sock_net(sk), po->ifindex);
+ if (!dev)
+ return -EINVAL;
+ if (!dev->netdev_ops->ndo_ddma_map)
+ return -EOPNOTSUPP;
+ if (!atomic_read(&po->mapped))
+ return -EINVAL;
+
+ err = dev->netdev_ops->ndo_ddma_map(dev, index, sk, rb);
+ if (!err)
+ rb->ddma = true;
+ return err;
+ }
default:
return -ENOPROTOOPT;
}
@@ -4228,6 +4256,15 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
if (atomic_read(&po->mapped))
pr_err("packet_mmap: vma is busy: %d\n",
atomic_read(&po->mapped));
+
+ if (rb->ddma) {
+ struct net_device *dev =
+ __dev_get_by_index(sock_net(sk), po->ifindex);
+
+ if (dev && dev->netdev_ops->ndo_ddma_map)
+ dev->netdev_ops->ndo_ddma_unmap(dev, 0);
+ rb->ddma = false;
+ }
}
mutex_unlock(&po->pg_vec_lock);
diff --git a/net/packet/internal.h b/net/packet/internal.h
index 9ee4631..4eec79e 100644
--- a/net/packet/internal.h
+++ b/net/packet/internal.h
@@ -10,66 +10,6 @@ struct packet_mclist {
unsigned char addr[MAX_ADDR_LEN];
};
-/* kbdq - kernel block descriptor queue */
-struct tpacket_kbdq_core {
- struct pgv *pkbdq;
- unsigned int feature_req_word;
- unsigned int hdrlen;
- unsigned char reset_pending_on_curr_blk;
- unsigned char delete_blk_timer;
- unsigned short kactive_blk_num;
- unsigned short blk_sizeof_priv;
-
- /* last_kactive_blk_num:
- * trick to see if user-space has caught up
- * in order to avoid refreshing timer when every single pkt arrives.
- */
- unsigned short last_kactive_blk_num;
-
- char *pkblk_start;
- char *pkblk_end;
- int kblk_size;
- unsigned int max_frame_len;
- unsigned int knum_blocks;
- uint64_t knxt_seq_num;
- char *prev;
- char *nxt_offset;
- struct sk_buff *skb;
-
- atomic_t blk_fill_in_prog;
-
- /* Default is set to 8ms */
-#define DEFAULT_PRB_RETIRE_TOV (8)
-
- unsigned short retire_blk_tov;
- unsigned short version;
- unsigned long tov_in_jiffies;
-
- /* timer to retire an outstanding block */
- struct timer_list retire_blk_timer;
-};
-
-struct pgv {
- char *buffer;
-};
-
-struct packet_ring_buffer {
- struct pgv *pg_vec;
-
- unsigned int head;
- unsigned int frames_per_block;
- unsigned int frame_size;
- unsigned int frame_max;
-
- unsigned int pg_vec_order;
- unsigned int pg_vec_pages;
- unsigned int pg_vec_len;
-
- unsigned int __percpu *pending_refcnt;
-
- struct tpacket_kbdq_core prb_bdqc;
-};
-
extern struct mutex fanout_mutex;
#define PACKET_FANOUT_MAX 256
diff --git a/tools/testing/selftests/net/psock_tpacket.c b/tools/testing/selftests/net/psock_tpacket.c
index 24adf70..32514f3 100644
--- a/tools/testing/selftests/net/psock_tpacket.c
+++ b/tools/testing/selftests/net/psock_tpacket.c
@@ -133,6 +133,20 @@ static void status_bar_update(void)
}
}
+static void print_payload(void *pay, size_t len)
+{
+ unsigned char *payload = pay;
+ int i;
+
+ printf("payload (bytes %lu): ", len);
+ for (i = 0; i < len; i++) {
+ if ((i % 32) == 0)
+ printf("\n");
+ printf("0x%02x ", payload[i]);
+ }
+ printf("\n");
+}
+
static void test_payload(void *pay, size_t len)
{
struct ethhdr *eth = pay;
@@ -148,6 +162,7 @@ static void test_payload(void *pay, size_t len)
"type: 0x%x!\n", ntohs(eth->h_proto));
exit(1);
}
+ print_payload(pay, len);
}
static void create_payload(void *pay, size_t *len)
@@ -232,21 +247,21 @@ static inline void __v1_v2_rx_user_ready(void *base, int version)
static void walk_v1_v2_rx(int sock, struct ring *ring)
{
struct pollfd pfd;
- int udp_sock[2];
+ //int udp_sock[2];
union frame_map ppd;
unsigned int frame_num = 0;
bug_on(ring->type != PACKET_RX_RING);
- pair_udp_open(udp_sock, PORT_BASE);
- pair_udp_setfilter(sock);
+ //pair_udp_open(udp_sock, PORT_BASE);
+ //pair_udp_setfilter(sock);
memset(&pfd, 0, sizeof(pfd));
pfd.fd = sock;
pfd.events = POLLIN | POLLERR;
pfd.revents = 0;
- pair_udp_send(udp_sock, NUM_PACKETS);
+ //pair_udp_send(udp_sock, NUM_PACKETS);
while (total_packets < NUM_PACKETS * 2) {
while (__v1_v2_rx_kernel_ready(ring->rd[frame_num].iov_base,
@@ -257,6 +272,9 @@ static void walk_v1_v2_rx(int sock, struct ring *ring)
case TPACKET_V1:
test_payload((uint8_t *) ppd.raw + ppd.v1->tp_h.tp_mac,
ppd.v1->tp_h.tp_snaplen);
+ print_payload((uint8_t *) ppd.raw +
+ ppd.v2->tp_h.tp_mac,
+ ppd.v2->tp_h.tp_snaplen);
total_bytes += ppd.v1->tp_h.tp_snaplen;
break;
@@ -278,7 +296,7 @@ static void walk_v1_v2_rx(int sock, struct ring *ring)
poll(&pfd, 1, 1);
}
- pair_udp_close(udp_sock);
+ //pair_udp_close(udp_sock);
if (total_packets != 2 * NUM_PACKETS) {
fprintf(stderr, "walk_v%d_rx: received %u out of %u pkts\n",
@@ -372,7 +390,8 @@ static void walk_v1_v2_tx(int sock, struct ring *ring)
pair_udp_setfilter(rcv_sock);
- ll.sll_ifindex = if_nametoindex("lo");
+ /* hacking my test up */
+ ll.sll_ifindex = if_nametoindex("eth3");
ret = bind(rcv_sock, (struct sockaddr *) &ll, sizeof(ll));
if (ret == -1) {
perror("bind");
@@ -687,7 +706,7 @@ static void bind_ring(int sock, struct ring *ring)
ring->ll.sll_family = PF_PACKET;
ring->ll.sll_protocol = htons(ETH_P_ALL);
- ring->ll.sll_ifindex = if_nametoindex("lo");
+ ring->ll.sll_ifindex = if_nametoindex("eth3");
ring->ll.sll_hatype = 0;
ring->ll.sll_pkttype = 0;
ring->ll.sll_halen = 0;
@@ -755,6 +774,19 @@ static int test_user_bit_width(void)
[PACKET_TX_RING] = "PACKET_TX_RING",
};
+void direct_dma_ring(int sock)
+{
+ int ret;
+ int index = 1;
+
+ ret = setsockopt(sock, SOL_PACKET, PACKET_RX_DIRECT,
+ &index, sizeof(index));
+ if (ret == -10)
+ printf("Failed direct dma socket with %i\n", ret);
+ else
+ printf("Configured a direct dma socket!\n");
+}
+
static int test_tpacket(int version, int type)
{
int sock;
@@ -777,6 +809,7 @@ static int test_tpacket(int version, int type)
setup_ring(sock, &ring, version, type);
mmap_ring(sock, &ring);
bind_ring(sock, &ring);
+ direct_dma_ring(sock);
walk_ring(sock, &ring);
unmap_ring(sock, &ring);
close(sock);
@@ -789,13 +822,17 @@ int main(void)
{
int ret = 0;
+#if 0
ret |= test_tpacket(TPACKET_V1, PACKET_RX_RING);
ret |= test_tpacket(TPACKET_V1, PACKET_TX_RING);
+#endif
ret |= test_tpacket(TPACKET_V2, PACKET_RX_RING);
+#if 0
ret |= test_tpacket(TPACKET_V2, PACKET_TX_RING);
ret |= test_tpacket(TPACKET_V3, PACKET_RX_RING);
+#endif
if (ret)
return 1;
^ permalink raw reply related
* [RFC PATCH 2/2] ixgbe: add af_packet direct copy support
From: John Fastabend @ 2017-01-27 21:34 UTC (permalink / raw)
To: bjorn.topel, jasowang, ast, alexander.duyck, brouer
Cc: john.r.fastabend, netdev, john.fastabend
In-Reply-To: <20170127213132.14162.82951.stgit@john-Precision-Tower-5810>
This implements the ndo ops for direct dma socket option. This is
to start looking at the interface and driver work needed to enable
it.
Note error paths are not handled and I'm aware of a few bugs.
For example interface must be up before attaching socket or else
it will fail silently. TBD fix all these things.
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 3
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 255 +++++++++++++++++++++++++
2 files changed, 256 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index ef81c3d..198f90c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -266,6 +266,7 @@ struct ixgbe_ring {
struct ixgbe_tx_buffer *tx_buffer_info;
struct ixgbe_rx_buffer *rx_buffer_info;
};
+ unsigned int buffer_size;
unsigned long state;
u8 __iomem *tail;
dma_addr_t dma; /* phys. address of descriptor ring */
@@ -299,6 +300,8 @@ struct ixgbe_ring {
struct ixgbe_tx_queue_stats tx_stats;
struct ixgbe_rx_queue_stats rx_stats;
};
+ bool ddma; /* ring data buffers mapped to userspace */
+ struct sock *rx_kick; /* rx kick userspace */
} ____cacheline_internodealigned_in_smp;
enum ixgbe_ring_f_enum {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 1e2f39e..c5ab44a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1608,8 +1608,19 @@ void ixgbe_alloc_rx_buffers(struct ixgbe_ring *rx_ring, u16 cleaned_count)
i -= rx_ring->count;
do {
- if (!ixgbe_alloc_mapped_page(rx_ring, bi))
- break;
+ /* If direct dma has not released packet yet stop */
+ if (rx_ring->ddma) {
+ int align = TPACKET_ALIGN(TPACKET2_HDRLEN);
+ int hdrlen = ALIGN(align, L1_CACHE_BYTES);
+ struct tpacket2_hdr *hdr;
+
+ hdr = page_address(bi->page) + bi->page_offset - hdrlen;
+ if (unlikely(TP_STATUS_USER & hdr->tp_status))
+ break;
+ } else {
+ if (!ixgbe_alloc_mapped_page(rx_ring, bi))
+ break;
+ }
/*
* Refresh the desc even if buffer_addrs didn't change
@@ -2005,6 +2016,97 @@ static bool ixgbe_add_rx_frag(struct ixgbe_ring *rx_ring,
return true;
}
+/* ixgbe_do_ddma - direct dma routine to populate PACKET_RX_RING mmap region
+ *
+ * The packet socket interface builds a shared memory region using mmap after
+ * it is specified by the PACKET_RX_RING socket option. This will create a
+ * circular ring of memory slots. Typical software usage case copies the skb
+ * into these pages via tpacket_rcv() routine.
+ *
+ * Here we do direct DMA from the hardware (82599 in this case) into the
+ * mmap regions and populate the uhdr (think user space descriptor). This
+ * requires the hardware to support Scatter Gather and HighDMA which should
+ * be standard on most (all?) 10/40 Gbps devices.
+ *
+ * The buffer mapping should have already been done so that rx_buffer pages
+ * are handed to the driver from the mmap setup done at the socket layer.
+ *
+ * See ./include/uapi/linux/if_packet.h for details on packet layout here
+ * we can only use tpacket2_hdr type. v3 of the header type introduced bulk
+ * polling modes which do not work correctly with hardware DMA engine. The
+ * primary issue is we can not stop a DMA transaction from occurring after it
+ * has been configured. What results is the software timer advances the
+ * ring ahead of the hardware and the ring state is lost. Maybe there is
+ * a clever way to resolve this by I haven't thought it up yet.
+ */
+static int ixgbe_do_ddma(struct ixgbe_ring *rx_ring,
+ union ixgbe_adv_rx_desc *rx_desc)
+{
+ int hdrlen = ALIGN(TPACKET_ALIGN(TPACKET2_HDRLEN), L1_CACHE_BYTES);
+ struct ixgbe_adapter *adapter = netdev_priv(rx_ring->netdev);
+ struct ixgbe_rx_buffer *rx_buffer;
+ struct tpacket2_hdr *h2; /* userspace descriptor */
+ struct sockaddr_ll *sll;
+ struct ethhdr *eth;
+ int len = 0;
+ u64 ns = 0;
+ s32 rem;
+
+ rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
+ if (!rx_buffer->dma)
+ return -EBUSY;
+
+ prefetchw(rx_buffer->page);
+
+ /* test for any known error cases */
+ WARN_ON(ixgbe_test_staterr(rx_desc,
+ IXGBE_RXDADV_ERR_FRAME_ERR_MASK) &&
+ !(rx_ring->netdev->features & NETIF_F_RXALL));
+
+ dma_sync_single_range_for_cpu(rx_ring->dev,
+ rx_buffer->dma,
+ rx_buffer->page_offset,
+ rx_ring->buffer_size,
+ DMA_FROM_DEVICE);
+
+ /* Update the last_rx_timestamp timer in order to enable watchdog check
+ * for error case of latched timestamp on a dropped packet.
+ */
+ adapter->last_rx_timestamp = jiffies;
+ h2 = page_address(rx_buffer->page) + rx_buffer->page_offset - hdrlen;
+ eth = page_address(rx_buffer->page) + rx_buffer->page_offset,
+
+ /* This indicates a bug in ixgbe leaving for testing purposes */
+ WARN_ON(TP_STATUS_USER & h2->tp_status);
+ len = le16_to_cpu(rx_desc->wb.upper.length);
+ h2->tp_len = len;
+ h2->tp_snaplen = len;
+ h2->tp_mac = ALIGN(TPACKET_ALIGN(TPACKET2_HDRLEN), L1_CACHE_BYTES);
+ h2->tp_net = h2->tp_mac + ETH_HLEN;
+ h2->tp_sec = div_s64_rem(ns, NSEC_PER_SEC, &rem);
+ h2->tp_nsec = rem;
+
+ sll = (void *)h2 + TPACKET_ALIGN(sizeof(struct tpacket2_hdr));
+ sll->sll_halen = ETH_HLEN;
+ memcpy(sll->sll_addr, eth->h_source, ETH_ALEN);
+ sll->sll_family = AF_PACKET;
+ sll->sll_hatype = rx_ring->netdev->type;
+ sll->sll_protocol = eth->h_proto;
+ sll->sll_pkttype = PACKET_HOST;
+ sll->sll_ifindex = rx_ring->netdev->ifindex;
+
+ smp_mb();
+ h2->tp_status = TP_STATUS_USER;
+ rx_ring->rx_kick->sk_data_ready(rx_ring->rx_kick);
+
+ /* TBD handle non-EOP frames? - I think this is an invalid case
+ * assuming ring slots are setup correctly.
+ */
+ if (ixgbe_is_non_eop(rx_ring, rx_desc, NULL))
+ e_warn(drv, "Direct DMA received non-eop descriptor!");
+ return len;
+}
+
static struct sk_buff *ixgbe_fetch_rx_buffer(struct ixgbe_ring *rx_ring,
union ixgbe_adv_rx_desc *rx_desc)
{
@@ -2134,6 +2236,21 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
*/
dma_rmb();
+ /* If we use direct DMA to shmem then we do not need SKBs
+ * because user space descriptors are populated directly.
+ */
+ if (rx_ring->ddma) {
+ int len = ixgbe_do_ddma(rx_ring, rx_desc);
+
+ if (len) {
+ total_rx_packets++;
+ total_rx_bytes += len;
+ cleaned_count++;
+ continue;
+ }
+ break;
+ }
+
/* retrieve a buffer from the ring */
skb = ixgbe_fetch_rx_buffer(rx_ring, rx_desc);
@@ -4887,6 +5004,12 @@ static void ixgbe_clean_rx_ring(struct ixgbe_ring *rx_ring)
if (!rx_ring->rx_buffer_info)
return;
+ /* Buffers are owned by direct dma endpoint so we should not free
+ * them here and instead wait for endpoint to claim buffers.
+ */
+ if (rx_ring->ddma)
+ return;
+
/* Free all the Rx ring sk_buffs */
for (i = 0; i < rx_ring->count; i++) {
struct ixgbe_rx_buffer *rx_buffer = &rx_ring->rx_buffer_info[i];
@@ -9264,6 +9387,132 @@ static void ixgbe_fwd_del(struct net_device *pdev, void *priv)
return features;
}
+static int ixgbe_ddma_map(struct net_device *dev, unsigned int ring,
+ struct sock *sk, struct packet_ring_buffer *prb)
+{
+ struct ixgbe_adapter *adapter = netdev_priv(dev);
+ struct ixgbe_ring *rx_ring = adapter->rx_ring[ring];
+ unsigned int frames_per_blk = prb->frames_per_block;
+ unsigned int blk_nr = prb->pg_vec_len;
+ struct ixgbe_rx_buffer *bi;
+ int i, j, err = 0;
+
+ /* Verify we are given a valid ring */
+ if (ring >= adapter->num_rx_queues)
+ return -EINVAL;
+ /* Verify we have enough descriptors to support user space ring */
+ if (!frames_per_blk || ((blk_nr * frames_per_blk) != rx_ring->count)) {
+ e_warn(drv, "ddma map requires %i ring slots\n",
+ blk_nr * frames_per_blk);
+ return -EBUSY;
+ }
+
+ /* Bring the queue down to point buffers at user space */
+ ixgbe_disable_rx_queue(adapter, rx_ring);
+ usleep_range(10000, 20000);
+ ixgbe_irq_disable_queues(adapter, ((u64)1 << ring));
+ ixgbe_clean_rx_ring(rx_ring);
+
+ rx_ring->buffer_size = prb->frame_size;
+
+ /* In Direct DMA mode the descriptor block and tpacket headers are
+ * held in fixed locations so we can pre-populate most of the fields.
+ * Similarly the pages, offsets, and sizes for DMA are pre-calculated
+ * to align with the user space ring and do not need to change.
+ *
+ * The layout is fixed to match the PF_PACKET layer in /net/packet/
+ * which also invokes this routine via ndo_ddma_map().
+ *
+ * Couple design comments: Should we create v4 protocol rather than
+ * trying to leverage v2 for compat. Perhaps. And similarly should
+ * we generalize the packet_ring_buffer onto something that can be
+ * generalized for other users?
+ */
+ for (i = 0; i < blk_nr; i++) {
+ char *kaddr = prb->pg_vec[i].buffer;
+ unsigned int blk_size;
+ struct page *page;
+ size_t offset = 0;
+ dma_addr_t dma;
+
+ /* For DMA usage we can't use vmalloc */
+ if (is_vmalloc_addr(kaddr)) {
+ e_warn(drv, "vmalloc pages not supported in ddma\n");
+ err = -EINVAL;
+ goto unwind_map;
+ }
+
+ /* map page for use */
+ page = virt_to_page(kaddr);
+ blk_size = PAGE_SIZE << prb->pg_vec_order;
+ dma = dma_map_page(rx_ring->dev, page, 0,
+ blk_size, DMA_FROM_DEVICE);
+ if (dma_mapping_error(rx_ring->dev, dma)) {
+ e_warn(drv, "ddma mapping error DMA_FROM_DEVICE\n");
+ rx_ring->rx_stats.alloc_rx_page_failed++;
+ err = -EBUSY;
+ goto unwind_map;
+ }
+
+ /* We may be able to push multiple frames per block in this case
+ * set offset correctly to set pkt_addr correctly in descriptor.
+ */
+ for (j = 0; j < frames_per_blk; j++) {
+ int hdrlen = ALIGN(TPACKET_ALIGN(TPACKET2_HDRLEN),
+ L1_CACHE_BYTES);
+ int b = ((i * frames_per_blk) + j);
+
+ bi = &rx_ring->rx_buffer_info[b];
+ if (!bi) // TBD abort here this just stops a panic
+ continue;
+ bi->page = page;
+ bi->dma = dma;
+
+ /* ignore priv for now */
+ bi->page_offset = offset + hdrlen;
+ offset += rx_ring->buffer_size;
+ }
+ }
+
+ rx_ring->ddma = true;
+ rx_ring->rx_kick = sk;
+unwind_map:
+ //tbd error path handling
+ ixgbe_configure_rx_ring(adapter, rx_ring);
+ return err;
+}
+
+static void ixgbe_ddma_unmap(struct net_device *dev, unsigned int ring)
+{
+ struct ixgbe_adapter *adapter = netdev_priv(dev);
+ struct ixgbe_ring *rx_ring = adapter->rx_ring[ring];
+ int i;
+
+ rx_ring->ddma = false;
+ // tbd live traffic here(?) need to stop queue
+
+ /* Free all the Rx ring sk_buffs */
+ for (i = 0; i < rx_ring->count; i++) {
+ struct ixgbe_rx_buffer *rx_buffer;
+
+ rx_buffer = &rx_ring->rx_buffer_info[i];
+ if (!rx_buffer)
+ continue;
+ if (rx_buffer->dma)
+ dma_unmap_page(rx_ring->dev, rx_buffer->dma,
+ rx_buffer->page_offset,
+ DMA_FROM_DEVICE);
+
+ rx_buffer->dma = 0;
+ rx_buffer->page_offset = 0;
+ rx_buffer->page = NULL;
+ }
+
+ rtnl_lock();
+ ixgbe_setup_tc(dev, netdev_get_num_tc(dev));
+ rtnl_unlock();
+}
+
static const struct net_device_ops ixgbe_netdev_ops = {
.ndo_open = ixgbe_open,
.ndo_stop = ixgbe_close,
@@ -9312,6 +9561,8 @@ static void ixgbe_fwd_del(struct net_device *pdev, void *priv)
.ndo_udp_tunnel_add = ixgbe_add_udp_tunnel_port,
.ndo_udp_tunnel_del = ixgbe_del_udp_tunnel_port,
.ndo_features_check = ixgbe_features_check,
+ .ndo_ddma_map = ixgbe_ddma_map,
+ .ndo_ddma_unmap = ixgbe_ddma_unmap,
};
/**
^ permalink raw reply related
* [net-next v2] openvswitch: Simplify do_execute_actions().
From: Andy Zhou @ 2017-01-27 21:45 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou
do_execute_actions() implements a worthwhile optimization: in case
an output action is the last action in an action list, skb_clone()
can be avoided by outputing the current skb. However, the
implementation is more complicated than necessary. This patch
simplify this logic.
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
v1->v2: drop skb NULL check in do_output()
---
net/openvswitch/actions.c | 42 ++++++++++++++++++++----------------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 514f7bc..efa9a88 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1141,12 +1141,6 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
struct sw_flow_key *key,
const struct nlattr *attr, int len)
{
- /* Every output action needs a separate clone of 'skb', but the common
- * case is just a single output action, so that doing a clone and
- * then freeing the original skbuff is wasteful. So the following code
- * is slightly obscure just to avoid that.
- */
- int prev_port = -1;
const struct nlattr *a;
int rem;
@@ -1154,20 +1148,28 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
a = nla_next(a, &rem)) {
int err = 0;
- if (unlikely(prev_port != -1)) {
- struct sk_buff *out_skb = skb_clone(skb, GFP_ATOMIC);
-
- if (out_skb)
- do_output(dp, out_skb, prev_port, key);
+ switch (nla_type(a)) {
+ case OVS_ACTION_ATTR_OUTPUT: {
+ int port = nla_get_u32(a);
+ struct sk_buff *clone;
+
+ /* Every output action needs a separate clone
+ * of 'skb', In case the output action is the
+ * last action, cloning can be avoided.
+ */
+ if (nla_is_last(a, rem)) {
+ do_output(dp, skb, port, key);
+ /* 'skb' has been used for output.
+ */
+ return 0;
+ }
+ clone = skb_clone(skb, GFP_ATOMIC);
+ if (clone)
+ do_output(dp, clone, port, key);
OVS_CB(skb)->cutlen = 0;
- prev_port = -1;
- }
-
- switch (nla_type(a)) {
- case OVS_ACTION_ATTR_OUTPUT:
- prev_port = nla_get_u32(a);
break;
+ }
case OVS_ACTION_ATTR_TRUNC: {
struct ovs_action_trunc *trunc = nla_data(a);
@@ -1257,11 +1259,7 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
}
}
- if (prev_port != -1)
- do_output(dp, skb, prev_port, key);
- else
- consume_skb(skb);
-
+ consume_skb(skb);
return 0;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next 1/4] mlx5: Make building eswitch configurable
From: Saeed Mahameed @ 2017-01-27 21:15 UTC (permalink / raw)
To: Tom Herbert
Cc: Or Gerlitz, Saeed Mahameed, David Miller, Linux Netdev List,
Kernel Team
In-Reply-To: <CALx6S35-ecvO0k6-yqdLpO6mXA4Y6mycMD3HqHPCaUtMZd8GYA@mail.gmail.com>
On Fri, Jan 27, 2017 at 8:42 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Fri, Jan 27, 2017 at 10:28 AM, Saeed Mahameed
> <saeedm@dev.mellanox.co.il> wrote:
>> On Fri, Jan 27, 2017 at 8:16 PM, Tom Herbert <tom@herbertland.com> wrote:
>>> On Fri, Jan 27, 2017 at 10:05 AM, Saeed Mahameed
>>> <saeedm@dev.mellanox.co.il> wrote:
>>>> On Fri, Jan 27, 2017 at 7:50 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>>> On Fri, Jan 27, 2017 at 9:38 AM, Saeed Mahameed
>>>>> <saeedm@dev.mellanox.co.il> wrote:
>>>>>> On Fri, Jan 27, 2017 at 7:34 AM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>>>>> On Fri, Jan 27, 2017 at 1:32 AM, Tom Herbert <tom@herbertland.com> wrote:
>>>>>>>> Add a configuration option (CONFIG_MLX5_CORE_ESWITCH) for controlling
>>>>>>>> whether the eswitch code is built. Change Kconfig and Makefile
>>>>>>>> accordingly.
>>>>>>>
>>>>>>> Tom, FWIW, please note that the basic e-switch functionality is needed
>>>>>>> also when SRIOV isn't of use, this is for a multi host configuration.
>>>>>>>
>>>>>>
>>>>>> Right, set_l2_table_entry@eswitch.c need to be called by PF for any UC
>>>>>> MAC address wanted by VF or PF.
>>>>>> To keep one flow in the code, the implementation is done as part of eswitch.
>>>>>>
>>>>>> so in multi-host configuration (where there are 4 PFs) each PF should
>>>>>> invoke set_l2_table_entry_cmd for each one of its own UC MACs.
>>>>>>
>>>>>> populating the l2 table is done using the whole eswitch event driven
>>>>>> mechanisms, it is not easy and IMH not right to separate eswitch
>>>>>> tables from l2 table (same management logic, different tables).
>>>>>>
>>>>>> Anyways as Or stated this is just an FYI, eswitch needs to be enabled
>>>>>> on Multi-host configuration.
>>>>>>
>>>>> What indicate a multi-host configuration?
>>>>
>>>> nothing in the driver, it is transparent.
>>>>
>>> So then we always need the eswitch code to be built even if someone
>>> never uses any of it?
>>>
>>
>> yes.
>> but for your convenience all you need is to compile eswitch.c.
>> esiwtch_offoalds.c and en_rep.c can be compiled out for basic ethernet.
>>
> Well eswitch.c is 2200 LOC. en_rep.c and eswitch_offloads.c are 1600
> LOC. If we _must_ have eswitch.c then there's probably not much point
> then. But I am still finding it hard to fathom that eswitch has now
> become a mandatory component of Ethernet drivers/devices.
>
It is only mandatory for configurations that needs eswitch, where the
driver has no way to know about them, for a good old bare metal box,
eswitch is not needed.
we can do some work to strip the l2 table logic - needed for PFs to
work on multi-host - out of eswitch but again that would further
complicate the driver code since eswitch will still need to update l2
tables for VFs.
> Tom
>
>>>>>
>>>>>>> Or.
>>>>>>>
>>>>>>> My WW (and same for the rest of the IL team..) has ended so I will be
>>>>>>> able to further look on this series and comment on Sunday.
^ permalink raw reply
* Re: [net 7/8] net/mlx5e: Fix update of hash function/key via ethtool
From: Tom Herbert @ 2017-01-27 21:50 UTC (permalink / raw)
To: Saeed Mahameed
Cc: David S. Miller, Linux Kernel Network Developers, Gal Pressman
In-Reply-To: <20170127203843.3206-8-saeedm@mellanox.com>
On Fri, Jan 27, 2017 at 12:38 PM, Saeed Mahameed <saeedm@mellanox.com> wrote:
> From: Gal Pressman <galp@mellanox.com>
>
> Modifying TIR hash should change selected fields bitmask in addition to
> the function and key.
> Formerly, we would not set this field resulting in zeroing of its value,
> which means no packet fields are used for RX RSS hash calculation thus
> causing all traffic to arrive in RQ[0].
>
This commit log is rather scant in details. Does this mean that RSS is
somehow broken in mlx5? What is exact test that demonstrates bad
behavior? Did you verify that this doesn't break IPv4 or IPv6?
> Fixes: bdfc028de1b3 ("net/mlx5e: Fix ethtool RX hash func configuration change")
> Signed-off-by: Gal Pressman <galp@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 3 +-
> .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 13 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 198 ++++++++++-----------
> 3 files changed, 109 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 1619147a63e8..d5ecb8f53fd4 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -791,7 +791,8 @@ void mlx5e_disable_vlan_filter(struct mlx5e_priv *priv);
> int mlx5e_modify_rqs_vsd(struct mlx5e_priv *priv, bool vsd);
>
> int mlx5e_redirect_rqt(struct mlx5e_priv *priv, u32 rqtn, int sz, int ix);
> -void mlx5e_build_tir_ctx_hash(void *tirc, struct mlx5e_priv *priv);
> +void mlx5e_build_indir_tir_ctx_hash(struct mlx5e_priv *priv, void *tirc,
> + enum mlx5e_traffic_types tt);
>
> int mlx5e_open_locked(struct net_device *netdev);
> int mlx5e_close_locked(struct net_device *netdev);
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index 6f4eb34259f0..bb67863aa361 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> @@ -980,15 +980,18 @@ static int mlx5e_get_rxfh(struct net_device *netdev, u32 *indir, u8 *key,
>
> static void mlx5e_modify_tirs_hash(struct mlx5e_priv *priv, void *in, int inlen)
> {
> - struct mlx5_core_dev *mdev = priv->mdev;
> void *tirc = MLX5_ADDR_OF(modify_tir_in, in, ctx);
> - int i;
> + struct mlx5_core_dev *mdev = priv->mdev;
> + int ctxlen = MLX5_ST_SZ_BYTES(tirc);
> + int tt;
>
> MLX5_SET(modify_tir_in, in, bitmask.hash, 1);
> - mlx5e_build_tir_ctx_hash(tirc, priv);
>
> - for (i = 0; i < MLX5E_NUM_INDIR_TIRS; i++)
> - mlx5_core_modify_tir(mdev, priv->indir_tir[i].tirn, in, inlen);
> + for (tt = 0; tt < MLX5E_NUM_INDIR_TIRS; tt++) {
> + memset(tirc, 0, ctxlen);
> + mlx5e_build_indir_tir_ctx_hash(priv, tirc, tt);
> + mlx5_core_modify_tir(mdev, priv->indir_tir[tt].tirn, in, inlen);
> + }
> }
>
> static int mlx5e_set_rxfh(struct net_device *dev, const u32 *indir,
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 948351ae5bd2..f14ca3385fdd 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -2022,8 +2022,23 @@ static void mlx5e_build_tir_ctx_lro(void *tirc, struct mlx5e_priv *priv)
> MLX5_SET(tirc, tirc, lro_timeout_period_usecs, priv->params.lro_timeout);
> }
>
> -void mlx5e_build_tir_ctx_hash(void *tirc, struct mlx5e_priv *priv)
> +void mlx5e_build_indir_tir_ctx_hash(struct mlx5e_priv *priv, void *tirc,
> + enum mlx5e_traffic_types tt)
> {
> + void *hfso = MLX5_ADDR_OF(tirc, tirc, rx_hash_field_selector_outer);
> +
> +#define MLX5_HASH_IP (MLX5_HASH_FIELD_SEL_SRC_IP |\
> + MLX5_HASH_FIELD_SEL_DST_IP)
> +
> +#define MLX5_HASH_IP_L4PORTS (MLX5_HASH_FIELD_SEL_SRC_IP |\
> + MLX5_HASH_FIELD_SEL_DST_IP |\
> + MLX5_HASH_FIELD_SEL_L4_SPORT |\
> + MLX5_HASH_FIELD_SEL_L4_DPORT)
> +
> +#define MLX5_HASH_IP_IPSEC_SPI (MLX5_HASH_FIELD_SEL_SRC_IP |\
> + MLX5_HASH_FIELD_SEL_DST_IP |\
> + MLX5_HASH_FIELD_SEL_IPSEC_SPI)
> +
> MLX5_SET(tirc, tirc, rx_hash_fn,
> mlx5e_rx_hash_fn(priv->params.rss_hfunc));
> if (priv->params.rss_hfunc == ETH_RSS_HASH_TOP) {
> @@ -2035,6 +2050,88 @@ void mlx5e_build_tir_ctx_hash(void *tirc, struct mlx5e_priv *priv)
> MLX5_SET(tirc, tirc, rx_hash_symmetric, 1);
> memcpy(rss_key, priv->params.toeplitz_hash_key, len);
> }
> +
> + switch (tt) {
> + case MLX5E_TT_IPV4_TCP:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV4);
> + MLX5_SET(rx_hash_field_select, hfso, l4_prot_type,
> + MLX5_L4_PROT_TYPE_TCP);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP_L4PORTS);
> + break;
> +
> + case MLX5E_TT_IPV6_TCP:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV6);
> + MLX5_SET(rx_hash_field_select, hfso, l4_prot_type,
> + MLX5_L4_PROT_TYPE_TCP);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP_L4PORTS);
> + break;
> +
> + case MLX5E_TT_IPV4_UDP:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV4);
> + MLX5_SET(rx_hash_field_select, hfso, l4_prot_type,
> + MLX5_L4_PROT_TYPE_UDP);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP_L4PORTS);
> + break;
> +
> + case MLX5E_TT_IPV6_UDP:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV6);
> + MLX5_SET(rx_hash_field_select, hfso, l4_prot_type,
> + MLX5_L4_PROT_TYPE_UDP);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP_L4PORTS);
> + break;
> +
> + case MLX5E_TT_IPV4_IPSEC_AH:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV4);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP_IPSEC_SPI);
> + break;
> +
> + case MLX5E_TT_IPV6_IPSEC_AH:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV6);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP_IPSEC_SPI);
> + break;
> +
> + case MLX5E_TT_IPV4_IPSEC_ESP:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV4);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP_IPSEC_SPI);
> + break;
> +
> + case MLX5E_TT_IPV6_IPSEC_ESP:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV6);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP_IPSEC_SPI);
> + break;
> +
> + case MLX5E_TT_IPV4:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV4);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP);
> + break;
> +
> + case MLX5E_TT_IPV6:
> + MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> + MLX5_L3_PROT_TYPE_IPV6);
> + MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> + MLX5_HASH_IP);
> + break;
> + default:
> + WARN_ONCE(true, "%s: bad traffic type!\n", __func__);
> + }
> }
>
> static int mlx5e_modify_tirs_lro(struct mlx5e_priv *priv)
> @@ -2404,110 +2501,13 @@ void mlx5e_cleanup_nic_tx(struct mlx5e_priv *priv)
> static void mlx5e_build_indir_tir_ctx(struct mlx5e_priv *priv, u32 *tirc,
> enum mlx5e_traffic_types tt)
> {
> - void *hfso = MLX5_ADDR_OF(tirc, tirc, rx_hash_field_selector_outer);
> -
> MLX5_SET(tirc, tirc, transport_domain, priv->mdev->mlx5e_res.td.tdn);
>
> -#define MLX5_HASH_IP (MLX5_HASH_FIELD_SEL_SRC_IP |\
> - MLX5_HASH_FIELD_SEL_DST_IP)
> -
> -#define MLX5_HASH_IP_L4PORTS (MLX5_HASH_FIELD_SEL_SRC_IP |\
> - MLX5_HASH_FIELD_SEL_DST_IP |\
> - MLX5_HASH_FIELD_SEL_L4_SPORT |\
> - MLX5_HASH_FIELD_SEL_L4_DPORT)
> -
> -#define MLX5_HASH_IP_IPSEC_SPI (MLX5_HASH_FIELD_SEL_SRC_IP |\
> - MLX5_HASH_FIELD_SEL_DST_IP |\
> - MLX5_HASH_FIELD_SEL_IPSEC_SPI)
> -
> mlx5e_build_tir_ctx_lro(tirc, priv);
>
> MLX5_SET(tirc, tirc, disp_type, MLX5_TIRC_DISP_TYPE_INDIRECT);
> MLX5_SET(tirc, tirc, indirect_table, priv->indir_rqt.rqtn);
> - mlx5e_build_tir_ctx_hash(tirc, priv);
> -
> - switch (tt) {
> - case MLX5E_TT_IPV4_TCP:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV4);
> - MLX5_SET(rx_hash_field_select, hfso, l4_prot_type,
> - MLX5_L4_PROT_TYPE_TCP);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP_L4PORTS);
> - break;
> -
> - case MLX5E_TT_IPV6_TCP:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV6);
> - MLX5_SET(rx_hash_field_select, hfso, l4_prot_type,
> - MLX5_L4_PROT_TYPE_TCP);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP_L4PORTS);
> - break;
> -
> - case MLX5E_TT_IPV4_UDP:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV4);
> - MLX5_SET(rx_hash_field_select, hfso, l4_prot_type,
> - MLX5_L4_PROT_TYPE_UDP);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP_L4PORTS);
> - break;
> -
> - case MLX5E_TT_IPV6_UDP:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV6);
> - MLX5_SET(rx_hash_field_select, hfso, l4_prot_type,
> - MLX5_L4_PROT_TYPE_UDP);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP_L4PORTS);
> - break;
> -
> - case MLX5E_TT_IPV4_IPSEC_AH:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV4);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP_IPSEC_SPI);
> - break;
> -
> - case MLX5E_TT_IPV6_IPSEC_AH:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV6);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP_IPSEC_SPI);
> - break;
> -
> - case MLX5E_TT_IPV4_IPSEC_ESP:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV4);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP_IPSEC_SPI);
> - break;
> -
> - case MLX5E_TT_IPV6_IPSEC_ESP:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV6);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP_IPSEC_SPI);
> - break;
> -
> - case MLX5E_TT_IPV4:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV4);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP);
> - break;
> -
> - case MLX5E_TT_IPV6:
> - MLX5_SET(rx_hash_field_select, hfso, l3_prot_type,
> - MLX5_L3_PROT_TYPE_IPV6);
> - MLX5_SET(rx_hash_field_select, hfso, selected_fields,
> - MLX5_HASH_IP);
> - break;
> - default:
> - WARN_ONCE(true,
> - "mlx5e_build_indir_tir_ctx: bad traffic type!\n");
> - }
> + mlx5e_build_indir_tir_ctx_hash(priv, tirc, tt);
> }
>
> static void mlx5e_build_direct_tir_ctx(struct mlx5e_priv *priv, u32 *tirc,
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH RFC net-next] packet: always ensure that we pass hard_header_len bytes in skb_headlen() to the driver
From: Sowmini Varadhan @ 2017-01-27 21:58 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: David Miller, Network Development
In-Reply-To: <CAF=yD-+GGAdvgXoZeMshyQXXh_3m9kjDGNFf9FQ43rxJgk__vw@mail.gmail.com>
On (01/27/17 15:51), Willem de Bruijn wrote:
:
> - limit capable() check to drivers with with .validate callback
(aka second option below)
:
> - let privileged applications shoot themselves in the foot (change nothing).
> The second will break variable length header protocols unless
> you exhaustively search for all variable length protocols and add
> validate callbacks first.
other than ax25, are there variable length header protocols out there
without ->validate, and which need the CAP_RAW_SYSIO branch?
I realize that, to an extent, even ethernet is a protocol whose
header is > 14 with vlan, but from the google search, seems like it
was mostly ax25 that really triggered a large part of the check.
If we think that there are a large number of these (that dont have a
->validate, to fix up things as desired) I'd just go for the "change
nothing in pf_packet" option.
As I found out many drivers like ixgbe and sunvnet have defensive checks
in the Tx path anyway, and xen_netfront can also join that group with
a few simple checks.
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: dsa: bcm_sf2: CFP support
From: Florian Fainelli @ 2017-01-27 21:58 UTC (permalink / raw)
To: Chris Healy
Cc: Andrew Lunn, Vivien Didelot, Pablo Neira Ayuso,
netdev@vger.kernel.org, David S. Miller
In-Reply-To: <CAFXsbZohZy3ybi6Pe5hryGfuBPnstUAQpB0HM4++XFmwbYi+3w@mail.gmail.com>
Hi Chris,
On 01/27/2017 01:24 PM, Chris Healy wrote:
> Hi Florian,
>
> In saying the below, I may just be showing my naivety but here goes:
>
> If I understand this correctly, what you are using is similar to the
> TCAM hardware present in the newer Marvell switches. I think Pablo is
> doing some work with nftables and HW offload using TCAM HW. Is there
> overlap here? It seems that one or the other API should be used but
> not both.
Well, the problem is that there is overlap with 3 different unrelated
subsystems accessing the same HW here: tc, ethtool, and netfilter, all
(two at least) with different ways of formatting input parameters, as I
pointed out a while back in this thread:
https://www.mail-archive.com/netdev@vger.kernel.org/msg126321.html
My angle on this submission is the following, purely based on pragmatism:
- I have real users behind this feature who are currently very happy
with how this works using ethtool, switching them to netlink, tc,
netfilter is not trivial, but could be done in the long run, not just
now. At the very least, this serves as reference code for people who are
curious to see how Broadcom's CFP works
- cls_flower was looked at, it is missing a critical feature IMHO which
is the ability to specify a rule index, and the amount of code necessary
to validate input parameters is just totally insane, just like the fact
that there is not a common intermediate input representation (ala
ethtool_rx_flow_spec) makes it impractical
- I have heard about the work Pablo is doing, but until it is publicly
submitted and reviewed, it's hard to project what it is going to look like
Thanks!
^ permalink raw reply
* [PATCH v2] net: adaptec: starfire: add checks for dma mapping errors
From: Alexey Khoroshilov @ 2017-01-27 22:07 UTC (permalink / raw)
To: David S. Miller
Cc: Alexey Khoroshilov, Ion Badulescu, netdev, linux-kernel,
ldv-project
In-Reply-To: <20170123.125317.799337053139253880.davem@redhat.com>
init_ring(), refill_rx_ring() and start_tx() don't check
if mapping dma memory succeed.
The patch adds the checks and failure handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/net/ethernet/adaptec/starfire.c | 45 +++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index c12d2618eebf..3872ab96b80a 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -1152,6 +1152,12 @@ static void init_ring(struct net_device *dev)
if (skb == NULL)
break;
np->rx_info[i].mapping = pci_map_single(np->pci_dev, skb->data, np->rx_buf_sz, PCI_DMA_FROMDEVICE);
+ if (pci_dma_mapping_error(np->pci_dev,
+ np->rx_info[i].mapping)) {
+ dev_kfree_skb(skb);
+ np->rx_info[i].skb = NULL;
+ break;
+ }
/* Grrr, we cannot offset to correctly align the IP header. */
np->rx_ring[i].rxaddr = cpu_to_dma(np->rx_info[i].mapping | RxDescValid);
}
@@ -1182,8 +1188,9 @@ static netdev_tx_t start_tx(struct sk_buff *skb, struct net_device *dev)
{
struct netdev_private *np = netdev_priv(dev);
unsigned int entry;
+ unsigned int prev_tx;
u32 status;
- int i;
+ int i, j;
/*
* be cautious here, wrapping the queue has weird semantics
@@ -1201,6 +1208,7 @@ static netdev_tx_t start_tx(struct sk_buff *skb, struct net_device *dev)
}
#endif /* ZEROCOPY && HAS_BROKEN_FIRMWARE */
+ prev_tx = np->cur_tx;
entry = np->cur_tx % TX_RING_SIZE;
for (i = 0; i < skb_num_frags(skb); i++) {
int wrap_ring = 0;
@@ -1234,6 +1242,11 @@ static netdev_tx_t start_tx(struct sk_buff *skb, struct net_device *dev)
skb_frag_size(this_frag),
PCI_DMA_TODEVICE);
}
+ if (pci_dma_mapping_error(np->pci_dev,
+ np->tx_info[entry].mapping)) {
+ dev->stats.tx_dropped++;
+ goto err_out;
+ }
np->tx_ring[entry].addr = cpu_to_dma(np->tx_info[entry].mapping);
np->tx_ring[entry].status = cpu_to_le32(status);
@@ -1268,8 +1281,30 @@ static netdev_tx_t start_tx(struct sk_buff *skb, struct net_device *dev)
netif_stop_queue(dev);
return NETDEV_TX_OK;
-}
+err_out:
+ entry = prev_tx % TX_RING_SIZE;
+ np->tx_info[entry].skb = NULL;
+ if (i > 0) {
+ pci_unmap_single(np->pci_dev,
+ np->tx_info[entry].mapping,
+ skb_first_frag_len(skb),
+ PCI_DMA_TODEVICE);
+ np->tx_info[entry].mapping = 0;
+ entry = (entry + np->tx_info[entry].used_slots) % TX_RING_SIZE;
+ for (j = 1; j < i; j++) {
+ pci_unmap_single(np->pci_dev,
+ np->tx_info[entry].mapping,
+ skb_frag_size(
+ &skb_shinfo(skb)->frags[j-1]),
+ PCI_DMA_TODEVICE);
+ entry++;
+ }
+ }
+ dev_kfree_skb_any(skb);
+ np->cur_tx = prev_tx;
+ return NETDEV_TX_OK;
+}
/* The interrupt handler does all of the Rx thread work and cleans up
after the Tx thread. */
@@ -1569,6 +1604,12 @@ static void refill_rx_ring(struct net_device *dev)
break; /* Better luck next round. */
np->rx_info[entry].mapping =
pci_map_single(np->pci_dev, skb->data, np->rx_buf_sz, PCI_DMA_FROMDEVICE);
+ if (pci_dma_mapping_error(np->pci_dev,
+ np->rx_info[entry].mapping)) {
+ dev_kfree_skb(skb);
+ np->rx_info[entry].skb = NULL;
+ break;
+ }
np->rx_ring[entry].rxaddr =
cpu_to_dma(np->rx_info[entry].mapping | RxDescValid);
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v4] net: ethernet: faraday: To support device tree usage.
From: Rob Herring @ 2017-01-27 22:17 UTC (permalink / raw)
To: Arnd Bergmann, green.hu-Re5JQEeQqe8AvxtiuMwx3w
Cc: David Miller, Networking, devicetree-u79uwXL29TY76Z2rM5mHXA,
andrew-g2DYL2Zd6BY, jiri-rHqAuBHg3fBzbRFIqnYvSA,
Linux Kernel Mailing List, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
Mark Rutland
In-Reply-To: <CAK8P3a2Zr6=4x8JRa313DQ2ND0V=8eL0yKkrd93wbdxkiOtmSw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Jan 25, 2017 at 10:09:20PM +0100, Arnd Bergmann wrote:
> On Wed, Jan 25, 2017 at 6:34 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> > From: Greentime Hu <green.hu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Date: Tue, 24 Jan 2017 16:46:14 +0800
> >> We also use the same binding document to describe the same faraday ethernet
> >> controller and add faraday to vendor-prefixes.txt.
> >
> > Why are you renaming the MOXA binding file instead of adding a completely new one
> > for faraday? The MOXA one should stick around, I don't see a justification for
> > removing it.
>
> This was my suggestion, basically fixing the name of the existing
> binding, which was
> accidentally named after one of the users rather than the company that did the
> hardware.
>
> We can't change the compatible string, but I'd much prefer having only
> one binding
> file for this device rather than two separate ones that could possibly become
> incompatible in case we add new properties to them. If there is only
> one of them,
> naming it according to the hardware design is the general policy.
>
> Note that we currently have two separate device drivers, but that is more a
> historic artifact, and if we ever get around to merging them into one driver,
> that should not impact the binding.
The change is fine with me, but the subject and commit message need some
work. I'm guessing faraday licensed this to MOXA or something? Why is
the new name preferred or better?
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] Documentation: devicetree: change the mediatek ethernet compatible string
From: Rob Herring @ 2017-01-27 22:41 UTC (permalink / raw)
To: John Crispin
Cc: Matthias Brugger, David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
Sean Wang, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1485332455-34743-1-git-send-email-john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>
On Wed, Jan 25, 2017 at 09:20:54AM +0100, John Crispin wrote:
> When the binding was defined, I was not aware that mt2701 was an earlier
> version of the SoC. For sake of consistency, the ethernet driver should
> use mt2701 inside the compat string as this is the earliest SoC with the
> ethernet core.
>
> The ethernet driver is currently of no real use until we finish and
> upstream the DSA driver. There are no users of this binding yet. It should
> be safe to fix this now before it is too late and we need to provide
> backward compatibility for the mt7623-eth compat string.
Thanks for the explanation.
> Reported-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: John Crispin <john-Pj+rj9U5foFAfugRpC6u6w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/net/mediatek-net.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
> index c010faf..c7194e8 100644
> --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
> +++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
> @@ -7,7 +7,7 @@ have dual GMAC each represented by a child node..
> * Ethernet controller node
>
> Required properties:
> -- compatible: Should be "mediatek,mt7623-eth"
> +- compatible: Should be "mediatek,mt2701-eth"
You should have both strings with 2701 being last. That way if you ever
find a difference in the 7623, you don't need a DT update to fix it.
> - reg: Address and length of the register set for the device
> - interrupts: Should contain the three frame engines interrupts in numeric
> order. These are fe_int0, fe_int1 and fe_int2.
> --
> 1.7.10.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 5/5] [net-next] net: qcom/emac: add an error interrupt handler for the sgmii
From: Timur Tabi @ 2017-01-27 22:43 UTC (permalink / raw)
To: David Miller, netdev
In-Reply-To: <1485557028-32510-1-git-send-email-timur@codeaurora.org>
The SGMII (internal PHY) can report decode errors via an interrupt. It
can also report autonegotiation status changes, but we don't need to track
those. The SGMII can recover automatically from most decode errors, so
we only reset the interface if we get multiple consecutive errors.
It's possible for bogus decode errors to be reported while the link is
being brought up. The interrupt is registered when the interface is
opened, and it's enabled after the link is up.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 8 +-
drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 126 +++++++++++++++++++++++-
drivers/net/ethernet/qualcomm/emac/emac-sgmii.h | 16 ++-
drivers/net/ethernet/qualcomm/emac/emac.c | 10 ++
4 files changed, 153 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index 33d7ff1..b991219 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -951,12 +951,16 @@ static void emac_mac_rx_descs_refill(struct emac_adapter *adpt,
static void emac_adjust_link(struct net_device *netdev)
{
struct emac_adapter *adpt = netdev_priv(netdev);
+ struct emac_sgmii *sgmii = &adpt->phy;
struct phy_device *phydev = netdev->phydev;
- if (phydev->link)
+ if (phydev->link) {
emac_mac_start(adpt);
- else
+ sgmii->link_up(adpt);
+ } else {
+ sgmii->link_down(adpt);
emac_mac_stop(adpt);
+ }
phy_print_status(phydev);
}
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
index b5269c4..040b289 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
@@ -25,7 +25,9 @@
#define EMAC_SGMII_PHY_SPEED_CFG1 0x0074
#define EMAC_SGMII_PHY_IRQ_CMD 0x00ac
#define EMAC_SGMII_PHY_INTERRUPT_CLEAR 0x00b0
+#define EMAC_SGMII_PHY_INTERRUPT_MASK 0x00b4
#define EMAC_SGMII_PHY_INTERRUPT_STATUS 0x00b8
+#define EMAC_SGMII_PHY_RX_CHK_STATUS 0x00d4
#define FORCE_AN_TX_CFG BIT(5)
#define FORCE_AN_RX_CFG BIT(4)
@@ -36,6 +38,8 @@
#define SPDMODE_100 BIT(0)
#define SPDMODE_10 0
+#define CDR_ALIGN_DET BIT(6)
+
#define IRQ_GLOBAL_CLEAR BIT(0)
#define DECODE_CODE_ERR BIT(7)
@@ -44,6 +48,7 @@
#define SGMII_PHY_IRQ_CLR_WAIT_TIME 10
#define SGMII_PHY_INTERRUPT_ERR (DECODE_CODE_ERR | DECODE_DISP_ERR)
+#define SGMII_ISR_MASK (SGMII_PHY_INTERRUPT_ERR)
#define SERDES_START_WAIT_TIMES 100
@@ -96,6 +101,51 @@ static int emac_sgmii_irq_clear(struct emac_adapter *adpt, u32 irq_bits)
return 0;
}
+/* The number of decode errors that triggers a reset */
+#define DECODE_ERROR_LIMIT 2
+
+static irqreturn_t emac_sgmii_interrupt(int irq, void *data)
+{
+ struct emac_adapter *adpt = data;
+ struct emac_sgmii *phy = &adpt->phy;
+ u32 status;
+
+ status = readl(phy->base + EMAC_SGMII_PHY_INTERRUPT_STATUS);
+ status &= SGMII_ISR_MASK;
+ if (!status)
+ return IRQ_HANDLED;
+
+ /* If we get a decoding error and CDR is not locked, then try
+ * resetting the internal PHY. The internal PHY uses an embedded
+ * clock with Clock and Data Recovery (CDR) to recover the
+ * clock and data.
+ */
+ if (status & SGMII_PHY_INTERRUPT_ERR) {
+ int count;
+
+ /* The SGMII is capable of recovering from some decode
+ * errors automatically. However, if we get multiple
+ * decode errors in a row, then assume that something
+ * is wrong and reset the interface.
+ */
+ count = atomic_inc_return(&phy->decode_error_count);
+ if (count == DECODE_ERROR_LIMIT) {
+ schedule_work(&adpt->work_thread);
+ atomic_set(&phy->decode_error_count, 0);
+ }
+ } else {
+ /* We only care about consecutive decode errors. */
+ atomic_set(&phy->decode_error_count, 0);
+ }
+
+ if (emac_sgmii_irq_clear(adpt, status)) {
+ netdev_warn(adpt->netdev, "failed to clear SGMII interrupt\n");
+ schedule_work(&adpt->work_thread);
+ }
+
+ return IRQ_HANDLED;
+}
+
static void emac_sgmii_reset_prepare(struct emac_adapter *adpt)
{
struct emac_sgmii *phy = &adpt->phy;
@@ -129,6 +179,68 @@ void emac_sgmii_reset(struct emac_adapter *adpt)
ret);
}
+static int emac_sgmii_open(struct emac_adapter *adpt)
+{
+ struct emac_sgmii *sgmii = &adpt->phy;
+ int ret;
+
+ if (sgmii->irq) {
+ /* Make sure interrupts are cleared and disabled first */
+ ret = emac_sgmii_irq_clear(adpt, 0xff);
+ if (ret)
+ return ret;
+ writel(0, sgmii->base + EMAC_SGMII_PHY_INTERRUPT_MASK);
+
+ ret = request_irq(sgmii->irq, emac_sgmii_interrupt, 0,
+ "emac-sgmii", adpt);
+ if (ret) {
+ netdev_err(adpt->netdev,
+ "could not register handler for internal PHY\n");
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int emac_sgmii_close(struct emac_adapter *adpt)
+{
+ struct emac_sgmii *sgmii = &adpt->phy;
+
+ /* Make sure interrupts are disabled */
+ writel(0, sgmii->base + EMAC_SGMII_PHY_INTERRUPT_MASK);
+ free_irq(sgmii->irq, adpt);
+
+ return 0;
+}
+
+/* The error interrupts are only valid after the link is up */
+static int emac_sgmii_link_up(struct emac_adapter *adpt)
+{
+ struct emac_sgmii *sgmii = &adpt->phy;
+ int ret;
+
+ /* Clear and enable interrupts */
+ ret = emac_sgmii_irq_clear(adpt, 0xff);
+ if (ret)
+ return ret;
+
+ writel(SGMII_ISR_MASK, sgmii->base + EMAC_SGMII_PHY_INTERRUPT_MASK);
+
+ return 0;
+}
+
+static int emac_sgmii_link_down(struct emac_adapter *adpt)
+{
+ struct emac_sgmii *sgmii = &adpt->phy;
+
+ /* Disable interrupts */
+ writel(0, sgmii->base + EMAC_SGMII_PHY_INTERRUPT_MASK);
+ synchronize_irq(sgmii->irq);
+
+ return 0;
+}
+
static int emac_sgmii_acpi_match(struct device *dev, void *data)
{
#ifdef CONFIG_ACPI
@@ -139,7 +251,7 @@ static int emac_sgmii_acpi_match(struct device *dev, void *data)
{}
};
const struct acpi_device_id *id = acpi_match_device(match_table, dev);
- emac_sgmii_initialize *initialize = data;
+ emac_sgmii_function *initialize = data;
if (id) {
acpi_handle handle = ACPI_HANDLE(dev);
@@ -226,9 +338,14 @@ int emac_sgmii_config(struct platform_device *pdev, struct emac_adapter *adpt)
goto error_put_device;
}
- phy->initialize = (emac_sgmii_initialize)match->data;
+ phy->initialize = (emac_sgmii_function)match->data;
}
+ phy->open = emac_sgmii_open;
+ phy->close = emac_sgmii_close;
+ phy->link_up = emac_sgmii_link_up;
+ phy->link_down = emac_sgmii_link_down;
+
/* Base address is the first address */
res = platform_get_resource(sgmii_pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -256,9 +373,12 @@ int emac_sgmii_config(struct platform_device *pdev, struct emac_adapter *adpt)
if (ret)
goto error;
- emac_sgmii_irq_clear(adpt, SGMII_PHY_INTERRUPT_ERR);
emac_sgmii_link_init(adpt);
+ ret = platform_get_irq(sgmii_pdev, 0);
+ if (ret > 0)
+ phy->irq = ret;
+
/* We've remapped the addresses, so we don't need the device any
* more. of_find_device_by_node() says we should release it.
*/
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.h b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.h
index 4a8f6b1..e7c0c3b 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.h
+++ b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.h
@@ -16,17 +16,29 @@
struct emac_adapter;
struct platform_device;
-typedef int (*emac_sgmii_initialize)(struct emac_adapter *adpt);
+typedef int (*emac_sgmii_function)(struct emac_adapter *adpt);
/** emac_sgmii - internal emac phy
* @base base address
* @digital per-lane digital block
+ * @irq the interrupt number
+ * @decode_error_count reference count of consecutive decode errors
* @initialize initialization function
+ * @open called when the driver is opened
+ * @close called when the driver is closed
+ * @link_up called when the link comes up
+ * @link_down called when the link comes down
*/
struct emac_sgmii {
void __iomem *base;
void __iomem *digital;
- emac_sgmii_initialize initialize;
+ unsigned int irq;
+ atomic_t decode_error_count;
+ emac_sgmii_function initialize;
+ emac_sgmii_function open;
+ emac_sgmii_function close;
+ emac_sgmii_function link_up;
+ emac_sgmii_function link_down;
};
int emac_sgmii_config(struct platform_device *pdev, struct emac_adapter *adpt);
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 75305ad..f519351 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -280,6 +280,14 @@ static int emac_open(struct net_device *netdev)
return ret;
}
+ ret = adpt->phy.open(adpt);
+ if (ret) {
+ emac_mac_down(adpt);
+ emac_mac_rx_tx_rings_free_all(adpt);
+ free_irq(irq->irq, irq);
+ return ret;
+ }
+
return 0;
}
@@ -290,6 +298,7 @@ static int emac_close(struct net_device *netdev)
mutex_lock(&adpt->reset_lock);
+ adpt->phy.close(adpt);
emac_mac_down(adpt);
emac_mac_rx_tx_rings_free_all(adpt);
@@ -645,6 +654,7 @@ static int emac_probe(struct platform_device *pdev)
adpt->msg_enable = EMAC_MSG_DEFAULT;
phy = &adpt->phy;
+ atomic_set(&phy->decode_error_count, 0);
mutex_init(&adpt->reset_lock);
spin_lock_init(&adpt->stats.lock);
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 0/5] [net-next] net: qcom/emac:
From: Timur Tabi @ 2017-01-27 22:43 UTC (permalink / raw)
To: David Miller, netdev
Although not related, these patches affect the same files, so they should
be applied in order.
The first patch cleans up logging of when the the phy driver is attached.
The second patch always configures the SGMII to use autonegotiation mode.
The third patch removes a redundant call to emac_mac_start().
The fourth patch removes some extraneous non-functioning WOL code.
The fifth patch adds an error handler for the SGMII block.
Timur Tabi (5):
[net-next] net: qcom/emac: display the phy driver info after we
connect
[net-next] net: qcom/emac: always use autonegotiation to configure the
SGMII link
[net-next] net: qcom/emac: do not call emac_mac_start twice
[net-next] net: qcom/emac: remove extraneous wake-on-lan code
[net-next] net: qcom/emac: add an error interrupt handler for the
sgmii
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 24 ++--
drivers/net/ethernet/qualcomm/emac/emac-mac.h | 1 -
drivers/net/ethernet/qualcomm/emac/emac-phy.c | 3 -
drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 175 ++++++++++++++++++------
drivers/net/ethernet/qualcomm/emac/emac-sgmii.h | 16 ++-
drivers/net/ethernet/qualcomm/emac/emac.c | 10 +-
drivers/net/ethernet/qualcomm/emac/emac.h | 4 -
7 files changed, 166 insertions(+), 67 deletions(-)
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH 4/5] [net-next] net: qcom/emac: remove extraneous wake-on-lan code
From: Timur Tabi @ 2017-01-27 22:43 UTC (permalink / raw)
To: David Miller, netdev
In-Reply-To: <1485557028-32510-1-git-send-email-timur@codeaurora.org>
The EMAC driver does not support wake-on-lan, but there is still
code left-over that partially enables it. Remove that code and a few
macros that support it.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 10 ----------
drivers/net/ethernet/qualcomm/emac/emac.h | 4 ----
2 files changed, 14 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index 3f3cd00..33d7ff1 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -103,14 +103,6 @@
#define RXEN 0x00000002
#define TXEN 0x00000001
-
-/* EMAC_WOL_CTRL0 */
-#define LK_CHG_PME 0x20
-#define LK_CHG_EN 0x10
-#define MG_FRAME_PME 0x8
-#define MG_FRAME_EN 0x4
-#define WK_FRAME_EN 0x1
-
/* EMAC_DESC_CTRL_3 */
#define RFD_RING_SIZE_BMSK 0xfff
@@ -619,8 +611,6 @@ static void emac_mac_start(struct emac_adapter *adpt)
emac_reg_update32(adpt->base + EMAC_ATHR_HEADER_CTRL,
(HEADER_ENABLE | HEADER_CNT_EN), 0);
-
- emac_reg_update32(adpt->csr + EMAC_EMAC_WRAPPER_CSR2, 0, WOL_EN);
}
void emac_mac_stop(struct emac_adapter *adpt)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.h b/drivers/net/ethernet/qualcomm/emac/emac.h
index 2725507..ef91dcc 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.h
+++ b/drivers/net/ethernet/qualcomm/emac/emac.h
@@ -167,10 +167,6 @@ enum emac_clk_id {
#define EMAC_MAX_SETUP_LNK_CYCLE 100
-/* Wake On Lan */
-#define EMAC_WOL_PHY 0x00000001 /* PHY Status Change */
-#define EMAC_WOL_MAGIC 0x00000002 /* Magic Packet */
-
struct emac_stats {
/* rx */
u64 rx_ok; /* good packets */
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 2/5] [net-next] net: qcom/emac: always use autonegotiation to configure the SGMII link
From: Timur Tabi @ 2017-01-27 22:43 UTC (permalink / raw)
To: David Miller, netdev
In-Reply-To: <1485557028-32510-1-git-send-email-timur@codeaurora.org>
Regardless of how the external PHY is configured, the internal PHY
(the "SGMII" block) is capable of configuring the SGMII link automatically.
When the external PHY link comes up, regardless of how it is configured,
the SGMII link is configured automatically.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 49 +++++--------------------
1 file changed, 10 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
index 0149b52..b5269c4 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
@@ -47,44 +47,19 @@
#define SERDES_START_WAIT_TIMES 100
-static int emac_sgmii_link_init(struct emac_adapter *adpt)
+/* Initialize the SGMII link between the internal and external PHYs. */
+static void emac_sgmii_link_init(struct emac_adapter *adpt)
{
- struct phy_device *phydev = adpt->phydev;
struct emac_sgmii *phy = &adpt->phy;
u32 val;
+ /* Always use autonegotiation. It works no matter how the external
+ * PHY is configured.
+ */
val = readl(phy->base + EMAC_SGMII_PHY_AUTONEG_CFG2);
-
- if (phydev->autoneg == AUTONEG_ENABLE) {
- val &= ~(FORCE_AN_RX_CFG | FORCE_AN_TX_CFG);
- val |= AN_ENABLE;
- writel(val, phy->base + EMAC_SGMII_PHY_AUTONEG_CFG2);
- } else {
- u32 speed_cfg;
-
- switch (phydev->speed) {
- case SPEED_10:
- speed_cfg = SPDMODE_10;
- break;
- case SPEED_100:
- speed_cfg = SPDMODE_100;
- break;
- case SPEED_1000:
- speed_cfg = SPDMODE_1000;
- break;
- default:
- return -EINVAL;
- }
-
- if (phydev->duplex == DUPLEX_FULL)
- speed_cfg |= DUPLEX_MODE;
-
- val &= ~AN_ENABLE;
- writel(speed_cfg, phy->base + EMAC_SGMII_PHY_SPEED_CFG1);
- writel(val, phy->base + EMAC_SGMII_PHY_AUTONEG_CFG2);
- }
-
- return 0;
+ val &= ~(FORCE_AN_RX_CFG | FORCE_AN_TX_CFG);
+ val |= AN_ENABLE;
+ writel(val, phy->base + EMAC_SGMII_PHY_AUTONEG_CFG2);
}
static int emac_sgmii_irq_clear(struct emac_adapter *adpt, u32 irq_bits)
@@ -145,12 +120,7 @@ void emac_sgmii_reset(struct emac_adapter *adpt)
int ret;
emac_sgmii_reset_prepare(adpt);
-
- ret = emac_sgmii_link_init(adpt);
- if (ret) {
- netdev_err(adpt->netdev, "unsupported link speed\n");
- return;
- }
+ emac_sgmii_link_init(adpt);
ret = adpt->phy.initialize(adpt);
if (ret)
@@ -287,6 +257,7 @@ int emac_sgmii_config(struct platform_device *pdev, struct emac_adapter *adpt)
goto error;
emac_sgmii_irq_clear(adpt, SGMII_PHY_INTERRUPT_ERR);
+ emac_sgmii_link_init(adpt);
/* We've remapped the addresses, so we don't need the device any
* more. of_find_device_by_node() says we should release it.
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 4/4] [net-next] net: qcom/emac: add an error interrupt handler for the sgmii
From: Timur Tabi @ 2017-01-27 22:43 UTC (permalink / raw)
To: David Miller, netdev
In-Reply-To: <1485557028-32510-1-git-send-email-timur@codeaurora.org>
The SGMII (internal PHY) can report decode errors via an interrupt. It
can also report autonegotiation status changes, but we don't need to track
those. The SGMII can recover automatically from most decode errors, so
we only reset the interface if we get multiple consecutive errors.
It's possible for bogus decode errors to be reported while the link is
being brought up. The interrupt is registered when the interface is
opened, and it's enabled after the link is up.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 8 +-
drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 126 +++++++++++++++++++++++-
drivers/net/ethernet/qualcomm/emac/emac-sgmii.h | 16 ++-
drivers/net/ethernet/qualcomm/emac/emac.c | 10 ++
4 files changed, 153 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index 3f3cd00..a0bc8a85 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -961,12 +961,16 @@ static void emac_mac_rx_descs_refill(struct emac_adapter *adpt,
static void emac_adjust_link(struct net_device *netdev)
{
struct emac_adapter *adpt = netdev_priv(netdev);
+ struct emac_sgmii *sgmii = &adpt->phy;
struct phy_device *phydev = netdev->phydev;
- if (phydev->link)
+ if (phydev->link) {
emac_mac_start(adpt);
- else
+ sgmii->link_up(adpt);
+ } else {
+ sgmii->link_down(adpt);
emac_mac_stop(adpt);
+ }
phy_print_status(phydev);
}
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
index b5269c4..040b289 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
@@ -25,7 +25,9 @@
#define EMAC_SGMII_PHY_SPEED_CFG1 0x0074
#define EMAC_SGMII_PHY_IRQ_CMD 0x00ac
#define EMAC_SGMII_PHY_INTERRUPT_CLEAR 0x00b0
+#define EMAC_SGMII_PHY_INTERRUPT_MASK 0x00b4
#define EMAC_SGMII_PHY_INTERRUPT_STATUS 0x00b8
+#define EMAC_SGMII_PHY_RX_CHK_STATUS 0x00d4
#define FORCE_AN_TX_CFG BIT(5)
#define FORCE_AN_RX_CFG BIT(4)
@@ -36,6 +38,8 @@
#define SPDMODE_100 BIT(0)
#define SPDMODE_10 0
+#define CDR_ALIGN_DET BIT(6)
+
#define IRQ_GLOBAL_CLEAR BIT(0)
#define DECODE_CODE_ERR BIT(7)
@@ -44,6 +48,7 @@
#define SGMII_PHY_IRQ_CLR_WAIT_TIME 10
#define SGMII_PHY_INTERRUPT_ERR (DECODE_CODE_ERR | DECODE_DISP_ERR)
+#define SGMII_ISR_MASK (SGMII_PHY_INTERRUPT_ERR)
#define SERDES_START_WAIT_TIMES 100
@@ -96,6 +101,51 @@ static int emac_sgmii_irq_clear(struct emac_adapter *adpt, u32 irq_bits)
return 0;
}
+/* The number of decode errors that triggers a reset */
+#define DECODE_ERROR_LIMIT 2
+
+static irqreturn_t emac_sgmii_interrupt(int irq, void *data)
+{
+ struct emac_adapter *adpt = data;
+ struct emac_sgmii *phy = &adpt->phy;
+ u32 status;
+
+ status = readl(phy->base + EMAC_SGMII_PHY_INTERRUPT_STATUS);
+ status &= SGMII_ISR_MASK;
+ if (!status)
+ return IRQ_HANDLED;
+
+ /* If we get a decoding error and CDR is not locked, then try
+ * resetting the internal PHY. The internal PHY uses an embedded
+ * clock with Clock and Data Recovery (CDR) to recover the
+ * clock and data.
+ */
+ if (status & SGMII_PHY_INTERRUPT_ERR) {
+ int count;
+
+ /* The SGMII is capable of recovering from some decode
+ * errors automatically. However, if we get multiple
+ * decode errors in a row, then assume that something
+ * is wrong and reset the interface.
+ */
+ count = atomic_inc_return(&phy->decode_error_count);
+ if (count == DECODE_ERROR_LIMIT) {
+ schedule_work(&adpt->work_thread);
+ atomic_set(&phy->decode_error_count, 0);
+ }
+ } else {
+ /* We only care about consecutive decode errors. */
+ atomic_set(&phy->decode_error_count, 0);
+ }
+
+ if (emac_sgmii_irq_clear(adpt, status)) {
+ netdev_warn(adpt->netdev, "failed to clear SGMII interrupt\n");
+ schedule_work(&adpt->work_thread);
+ }
+
+ return IRQ_HANDLED;
+}
+
static void emac_sgmii_reset_prepare(struct emac_adapter *adpt)
{
struct emac_sgmii *phy = &adpt->phy;
@@ -129,6 +179,68 @@ void emac_sgmii_reset(struct emac_adapter *adpt)
ret);
}
+static int emac_sgmii_open(struct emac_adapter *adpt)
+{
+ struct emac_sgmii *sgmii = &adpt->phy;
+ int ret;
+
+ if (sgmii->irq) {
+ /* Make sure interrupts are cleared and disabled first */
+ ret = emac_sgmii_irq_clear(adpt, 0xff);
+ if (ret)
+ return ret;
+ writel(0, sgmii->base + EMAC_SGMII_PHY_INTERRUPT_MASK);
+
+ ret = request_irq(sgmii->irq, emac_sgmii_interrupt, 0,
+ "emac-sgmii", adpt);
+ if (ret) {
+ netdev_err(adpt->netdev,
+ "could not register handler for internal PHY\n");
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int emac_sgmii_close(struct emac_adapter *adpt)
+{
+ struct emac_sgmii *sgmii = &adpt->phy;
+
+ /* Make sure interrupts are disabled */
+ writel(0, sgmii->base + EMAC_SGMII_PHY_INTERRUPT_MASK);
+ free_irq(sgmii->irq, adpt);
+
+ return 0;
+}
+
+/* The error interrupts are only valid after the link is up */
+static int emac_sgmii_link_up(struct emac_adapter *adpt)
+{
+ struct emac_sgmii *sgmii = &adpt->phy;
+ int ret;
+
+ /* Clear and enable interrupts */
+ ret = emac_sgmii_irq_clear(adpt, 0xff);
+ if (ret)
+ return ret;
+
+ writel(SGMII_ISR_MASK, sgmii->base + EMAC_SGMII_PHY_INTERRUPT_MASK);
+
+ return 0;
+}
+
+static int emac_sgmii_link_down(struct emac_adapter *adpt)
+{
+ struct emac_sgmii *sgmii = &adpt->phy;
+
+ /* Disable interrupts */
+ writel(0, sgmii->base + EMAC_SGMII_PHY_INTERRUPT_MASK);
+ synchronize_irq(sgmii->irq);
+
+ return 0;
+}
+
static int emac_sgmii_acpi_match(struct device *dev, void *data)
{
#ifdef CONFIG_ACPI
@@ -139,7 +251,7 @@ static int emac_sgmii_acpi_match(struct device *dev, void *data)
{}
};
const struct acpi_device_id *id = acpi_match_device(match_table, dev);
- emac_sgmii_initialize *initialize = data;
+ emac_sgmii_function *initialize = data;
if (id) {
acpi_handle handle = ACPI_HANDLE(dev);
@@ -226,9 +338,14 @@ int emac_sgmii_config(struct platform_device *pdev, struct emac_adapter *adpt)
goto error_put_device;
}
- phy->initialize = (emac_sgmii_initialize)match->data;
+ phy->initialize = (emac_sgmii_function)match->data;
}
+ phy->open = emac_sgmii_open;
+ phy->close = emac_sgmii_close;
+ phy->link_up = emac_sgmii_link_up;
+ phy->link_down = emac_sgmii_link_down;
+
/* Base address is the first address */
res = platform_get_resource(sgmii_pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -256,9 +373,12 @@ int emac_sgmii_config(struct platform_device *pdev, struct emac_adapter *adpt)
if (ret)
goto error;
- emac_sgmii_irq_clear(adpt, SGMII_PHY_INTERRUPT_ERR);
emac_sgmii_link_init(adpt);
+ ret = platform_get_irq(sgmii_pdev, 0);
+ if (ret > 0)
+ phy->irq = ret;
+
/* We've remapped the addresses, so we don't need the device any
* more. of_find_device_by_node() says we should release it.
*/
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.h b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.h
index 4a8f6b1..e7c0c3b 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.h
+++ b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.h
@@ -16,17 +16,29 @@
struct emac_adapter;
struct platform_device;
-typedef int (*emac_sgmii_initialize)(struct emac_adapter *adpt);
+typedef int (*emac_sgmii_function)(struct emac_adapter *adpt);
/** emac_sgmii - internal emac phy
* @base base address
* @digital per-lane digital block
+ * @irq the interrupt number
+ * @decode_error_count reference count of consecutive decode errors
* @initialize initialization function
+ * @open called when the driver is opened
+ * @close called when the driver is closed
+ * @link_up called when the link comes up
+ * @link_down called when the link comes down
*/
struct emac_sgmii {
void __iomem *base;
void __iomem *digital;
- emac_sgmii_initialize initialize;
+ unsigned int irq;
+ atomic_t decode_error_count;
+ emac_sgmii_function initialize;
+ emac_sgmii_function open;
+ emac_sgmii_function close;
+ emac_sgmii_function link_up;
+ emac_sgmii_function link_down;
};
int emac_sgmii_config(struct platform_device *pdev, struct emac_adapter *adpt);
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 75305ad..f519351 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -280,6 +280,14 @@ static int emac_open(struct net_device *netdev)
return ret;
}
+ ret = adpt->phy.open(adpt);
+ if (ret) {
+ emac_mac_down(adpt);
+ emac_mac_rx_tx_rings_free_all(adpt);
+ free_irq(irq->irq, irq);
+ return ret;
+ }
+
return 0;
}
@@ -290,6 +298,7 @@ static int emac_close(struct net_device *netdev)
mutex_lock(&adpt->reset_lock);
+ adpt->phy.close(adpt);
emac_mac_down(adpt);
emac_mac_rx_tx_rings_free_all(adpt);
@@ -645,6 +654,7 @@ static int emac_probe(struct platform_device *pdev)
adpt->msg_enable = EMAC_MSG_DEFAULT;
phy = &adpt->phy;
+ atomic_set(&phy->decode_error_count, 0);
mutex_init(&adpt->reset_lock);
spin_lock_init(&adpt->stats.lock);
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 3/5] [net-next] net: qcom/emac: do not call emac_mac_start twice
From: Timur Tabi @ 2017-01-27 22:43 UTC (permalink / raw)
To: David Miller, netdev
In-Reply-To: <1485557028-32510-1-git-send-email-timur@codeaurora.org>
emac_mac_start() uses information from the external PHY to program
the MAC, so it makes no sense to call it before the link is up.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 2 +-
drivers/net/ethernet/qualcomm/emac/emac-mac.h | 1 -
drivers/net/ethernet/qualcomm/emac/emac.c | 2 --
3 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index 155e273..3f3cd00 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -556,7 +556,7 @@ void emac_mac_reset(struct emac_adapter *adpt)
emac_reg_update32(adpt->base + EMAC_DMA_MAS_CTRL, 0, INT_RD_CLR_EN);
}
-void emac_mac_start(struct emac_adapter *adpt)
+static void emac_mac_start(struct emac_adapter *adpt)
{
struct phy_device *phydev = adpt->phydev;
u32 mac, csr1;
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.h b/drivers/net/ethernet/qualcomm/emac/emac-mac.h
index f3aa24d..5028fb4 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.h
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.h
@@ -230,7 +230,6 @@ struct emac_tx_queue {
int emac_mac_up(struct emac_adapter *adpt);
void emac_mac_down(struct emac_adapter *adpt);
void emac_mac_reset(struct emac_adapter *adpt);
-void emac_mac_start(struct emac_adapter *adpt);
void emac_mac_stop(struct emac_adapter *adpt);
void emac_mac_mode_config(struct emac_adapter *adpt);
void emac_mac_rx_process(struct emac_adapter *adpt, struct emac_rx_queue *rx_q,
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 3e1be91..75305ad 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -280,8 +280,6 @@ static int emac_open(struct net_device *netdev)
return ret;
}
- emac_mac_start(adpt);
-
return 0;
}
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH 1/5] [net-next] net: qcom/emac: display the phy driver info after we connect
From: Timur Tabi @ 2017-01-27 22:43 UTC (permalink / raw)
To: David Miller, netdev
In-Reply-To: <1485557028-32510-1-git-send-email-timur@codeaurora.org>
The PHY driver is attached only when the driver calls
phy_connect_direct(). Calling phy_attached_print() to display
information about the PHY driver prior to that point is meaningless.
The interface can be brought down, a new PHY driver can be loaded,
and the interface then brought back up. This is the correct time
to display information about the attached driver.
Since phy_attached_print() also prints information about the
interrupt, that needs to be set as well.
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/net/ethernet/qualcomm/emac/emac-mac.c | 4 +++-
drivers/net/ethernet/qualcomm/emac/emac-phy.c | 3 ---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-mac.c b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
index e4793d7..155e273 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-mac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-mac.c
@@ -981,6 +981,7 @@ int emac_mac_up(struct emac_adapter *adpt)
emac_mac_config(adpt);
emac_mac_rx_descs_refill(adpt, &adpt->rx_q);
+ adpt->phydev->irq = PHY_IGNORE_INTERRUPT;
ret = phy_connect_direct(netdev, adpt->phydev, emac_adjust_link,
PHY_INTERFACE_MODE_SGMII);
if (ret) {
@@ -988,11 +989,12 @@ int emac_mac_up(struct emac_adapter *adpt)
return ret;
}
+ phy_attached_print(adpt->phydev, NULL);
+
/* enable mac irq */
writel((u32)~DIS_INT, adpt->base + EMAC_INT_STATUS);
writel(adpt->irq.mask, adpt->base + EMAC_INT_MASK);
- adpt->phydev->irq = PHY_IGNORE_INTERRUPT;
phy_start(adpt->phydev);
napi_enable(&adpt->rx_q.napi);
diff --git a/drivers/net/ethernet/qualcomm/emac/emac-phy.c b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
index 1d7852f..441c1936 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-phy.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-phy.c
@@ -226,8 +226,5 @@ int emac_phy_config(struct platform_device *pdev, struct emac_adapter *adpt)
return -ENODEV;
}
- if (adpt->phydev->drv)
- phy_attached_print(adpt->phydev, NULL);
-
return 0;
}
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* [PATCH net-next v3 0/4] net: ipv6: Improve user experience with multipath routes
From: David Ahern @ 2017-01-27 23:20 UTC (permalink / raw)
To: netdev; +Cc: roopa, nicolas.dichtel, David Ahern
This series closes a couple of gaps between IPv4 and IPv6 with respect
to multipath routes:
1. IPv4 allows all nexthops of multipath routes to be deleted using just
the prefix and length; IPv6 only deletes the first nexthop for the
route if only the prefix and length are given.
2. IPv4 returns multipath routes encoded in the RTA_MULTIPATH attribute.
IPv6 returns a series of routes with the same prefix and length - one
for each nexthop. This happens for both dumps and notifications.
IPv6 does accept RTA_MULTIPATH encoded routes, but installs them as a
series of routes.
Patch 2 addresses the first item by allowing IPv6 multipath routes to be
deleted using just the prefix and length. Patch 3 addresses the second
allowing IPv6 multipath routes to be returned encoded in the RTA_MULTIPATH.
Patch 1 adds the NLM_F_APPEND flag to notifications when the flag is
present in the request. The lack of this flag was noted testing route
appends and comparing to IPv4.
Patch 4 prints IPv6 addresses in compressed format when showing route
replace errors. This was noticed testing REPLACE failures.
The end result for multipath routes:
1. Route Add
- one notification with RTA_MULTIPATH attribute
2. Route Replace
- notification for first route and all siblings that have
succeeded. This is needed regardless of success of remaining
nexthops to maintain add/delete consistency should a failure
happens on the second or following nexthop (ie., need to tell
userspace that original route has been replaced and then the
failure logic deletes all routes inserted thus far).
3. Route Delete
- for multipath route only given nexthops are deleted. This path
is hit when DELETE contains RTA_MULTIPATH. All other route deletes,
all nexthops are deleted for given prefix and length (and any
other specs if given)
- one notification sent per nexthop deleted. This is unavoidable
since IPv6 alllows a single nexthop to be deleted within a multipath
route
4. Route Appends
- IPv6 allows nexthops to be appended to an existing route. In this
case one notification is sent per nexthop added
Addresses some of the inconsistencies also noted by Roopa at netdev0.1:
https://www.netdev01.org/docs/prabhu-linux_ipv4_ipv6_inconsistencies_talk_slides.pdf
v3
- removed the need for a user API to opt-in to change. Requiring an
API just shifts the difference from same API with different
behavior to different API to achieve equivalent behavior
- route notifications changed to use RTA_MULTIPATH for add and replace
- upated commit messages and cover letter
v2
- fixed locking in patch 1 as noted by DaveM
- changed user API for patch 2 to require an rtmsg with RTM_F_ALL_NEXTHOPS
set in rtm_flags
- revamped explanation of patch 2 and cover letter
David Ahern (4):
net: ipv6: add NLM_F_APPEND in notifications when applicable
net: ipv6: Allow shorthand delete of all nexthops in multipath route
net: ipv6: Add support to dump multipath routes via RTA_MULTIPATH
attribute
net: ipv6: Use compressed IPv6 addresses showing route replace error
include/net/ip6_fib.h | 4 +-
include/net/netlink.h | 1 +
net/ipv6/ip6_fib.c | 19 +++++-
net/ipv6/route.c | 163 ++++++++++++++++++++++++++++++++++++++++++++------
4 files changed, 165 insertions(+), 22 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH net-next v3 3/4] net: ipv6: Add support to dump multipath routes via RTA_MULTIPATH attribute
From: David Ahern @ 2017-01-27 23:20 UTC (permalink / raw)
To: netdev; +Cc: roopa, nicolas.dichtel, David Ahern
In-Reply-To: <1485559258-4856-1-git-send-email-dsa@cumulusnetworks.com>
IPv6 returns multipath routes as a series of individual routes making
their display and handling by userspace different and more complicated
than IPv4, putting the burden on the user to see that a route is part of
a multipath route and internally creating a multipath route if desired
(e.g., libnl does this as of commit 29b71371e764). This patch addresses
this difference, allowing multipath routes to be returned using the
RTA_MULTIPATH attribute.
The end result is that IPv6 multipath routes can be treated and displayed
in a format similar to IPv4:
$ ip -6 ro ls vrf red
2001:db8::/120 metric 1024
nexthop via 2001:db8:1::62 dev eth1 weight 1
nexthop via 2001:db8:1::61 dev eth1 weight 1
nexthop via 2001:db8:1::60 dev eth1 weight 1
nexthop via 2001:db8:1::59 dev eth1 weight 1
2001:db8:1::/120 dev eth1 proto kernel metric 256 pref medium
...
Suggested-by: Dinesh Dutt <ddutt@cumulusnetworks.com>
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
v3
- dropped user API to opt-in to change
v2
- changed user api to opt in to new behavior from attribute appended to
the request to requiring an rtmsg struct with the RTM_F_ALL_NEXTHOPS
set
include/net/netlink.h | 1 +
net/ipv6/ip6_fib.c | 16 ++++++-
net/ipv6/route.c | 127 +++++++++++++++++++++++++++++++++++++++++++-------
3 files changed, 126 insertions(+), 18 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index d3938f11ae52..b239fcd33d80 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -229,6 +229,7 @@ struct nl_info {
struct nlmsghdr *nlh;
struct net *nl_net;
u32 portid;
+ bool skip_notify;
};
int netlink_rcv_skb(struct sk_buff *skb,
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index bcaf247232d7..2542794b2c64 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -318,6 +318,16 @@ static int fib6_dump_node(struct fib6_walker *w)
w->leaf = rt;
return 1;
}
+
+ /* if multipath routes are dumped in one route with
+ * the RTA_MULTIPATH attribute, then jump rt to point
+ * to the last sibling of this route (no need to dump
+ * the sibling routes again)
+ */
+ if (rt->rt6i_nsiblings)
+ rt = list_last_entry(&rt->rt6i_siblings,
+ struct rt6_info,
+ rt6i_siblings);
}
w->leaf = NULL;
return 0;
@@ -871,7 +881,8 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
*ins = rt;
rt->rt6i_node = fn;
atomic_inc(&rt->rt6i_ref);
- inet6_rt_notify(RTM_NEWROUTE, rt, info, nlflags);
+ if (!info->skip_notify)
+ inet6_rt_notify(RTM_NEWROUTE, rt, info, nlflags);
info->nl_net->ipv6.rt6_stats->fib_rt_entries++;
if (!(fn->fn_flags & RTN_RTINFO)) {
@@ -897,7 +908,8 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
rt->rt6i_node = fn;
rt->dst.rt6_next = iter->dst.rt6_next;
atomic_inc(&rt->rt6i_ref);
- inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
+ if (!info->skip_notify)
+ inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
if (!(fn->fn_flags & RTN_RTINFO)) {
info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
fn->fn_flags |= RTN_RTINFO;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 81e2b2a28806..747f333ae006 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3010,19 +3010,25 @@ static int ip6_route_info_append(struct list_head *rt6_nh_list,
static int ip6_route_multipath_add(struct fib6_config *cfg)
{
+ struct rt6_info *rt, *rt_first = NULL;
struct fib6_config r_cfg;
struct rtnexthop *rtnh;
- struct rt6_info *rt;
struct rt6_nh *err_nh;
struct rt6_nh *nh, *nh_safe;
+ __u16 nlflags;
int remaining;
int attrlen;
int err = 1;
int nhn = 0;
+ int append = cfg->fc_nlinfo.nlh->nlmsg_flags & NLM_F_APPEND;
int replace = (cfg->fc_nlinfo.nlh &&
(cfg->fc_nlinfo.nlh->nlmsg_flags & NLM_F_REPLACE));
LIST_HEAD(rt6_nh_list);
+ nlflags = replace ? NLM_F_REPLACE : NLM_F_CREATE;
+ if (append)
+ nlflags |= NLM_F_APPEND;
+
remaining = cfg->fc_mp_len;
rtnh = (struct rtnexthop *)cfg->fc_mp;
@@ -3065,9 +3071,20 @@ static int ip6_route_multipath_add(struct fib6_config *cfg)
rtnh = rtnh_next(rtnh, &remaining);
}
+ /* for route append want to send separate notifications on
+ * each add; for add and replace send one notification with
+ * all nexthops
+ */
+ if (!append)
+ cfg->fc_nlinfo.skip_notify = 1;
+
err_nh = NULL;
list_for_each_entry(nh, &rt6_nh_list, next) {
err = __ip6_ins_rt(nh->rt6_info, &cfg->fc_nlinfo, &nh->mxc);
+ /* save reference to first route for notification */
+ if (!rt_first && !err && !append)
+ rt_first = nh->rt6_info;
+
/* nh->rt6_info is used or freed at this point, reset to NULL*/
nh->rt6_info = NULL;
if (err) {
@@ -3089,9 +3106,21 @@ static int ip6_route_multipath_add(struct fib6_config *cfg)
nhn++;
}
+ if (rt_first)
+ inet6_rt_notify(RTM_NEWROUTE, rt_first, &cfg->fc_nlinfo,
+ nlflags);
+
goto cleanup;
add_errout:
+ /* send notification for routes that were added so far so
+ * that the delete notifications sent by ip6_route_del in
+ * the next block are coherent
+ */
+ if (rt_first)
+ inet6_rt_notify(RTM_NEWROUTE, rt_first, &cfg->fc_nlinfo,
+ nlflags);
+
/* Delete routes that were already added */
list_for_each_entry(nh, &rt6_nh_list, next) {
if (err_nh == nh)
@@ -3198,6 +3227,60 @@ static inline size_t rt6_nlmsg_size(struct rt6_info *rt)
+ lwtunnel_get_encap_size(rt->dst.lwtstate);
}
+static int rt6_nexthop_info(struct sk_buff *skb, struct rt6_info *rt,
+ unsigned int *flags)
+{
+ if (!netif_running(rt->dst.dev) || !netif_carrier_ok(rt->dst.dev)) {
+ *flags |= RTNH_F_LINKDOWN;
+ if (rt->rt6i_idev->cnf.ignore_routes_with_linkdown)
+ *flags |= RTNH_F_DEAD;
+ }
+
+ if (rt->rt6i_flags & RTF_GATEWAY) {
+ if (nla_put_in6_addr(skb, RTA_GATEWAY, &rt->rt6i_gateway) < 0)
+ goto nla_put_failure;
+ }
+
+ if (rt->dst.dev &&
+ nla_put_u32(skb, RTA_OIF, rt->dst.dev->ifindex))
+ goto nla_put_failure;
+
+ if (rt->dst.lwtstate &&
+ lwtunnel_fill_encap(skb, rt->dst.lwtstate) < 0)
+ goto nla_put_failure;
+
+ return 0;
+
+nla_put_failure:
+ return -EMSGSIZE;
+}
+
+static int rt6_add_nexthop(struct sk_buff *skb, struct rt6_info *rt)
+{
+ struct rtnexthop *rtnh;
+ unsigned int flags = 0;
+
+ rtnh = nla_reserve_nohdr(skb, sizeof(*rtnh));
+ if (!rtnh)
+ goto nla_put_failure;
+
+ rtnh->rtnh_hops = 0;
+ rtnh->rtnh_ifindex = rt->dst.dev ? rt->dst.dev->ifindex : 0;
+
+ if (rt6_nexthop_info(skb, rt, &flags) < 0)
+ goto nla_put_failure;
+
+ rtnh->rtnh_flags = flags;
+
+ /* length of rtnetlink header + attributes */
+ rtnh->rtnh_len = nlmsg_get_pos(skb) - (void *)rtnh;
+
+ return 0;
+
+nla_put_failure:
+ return -EMSGSIZE;
+}
+
static int rt6_fill_node(struct net *net,
struct sk_buff *skb, struct rt6_info *rt,
struct in6_addr *dst, struct in6_addr *src,
@@ -3249,11 +3332,6 @@ static int rt6_fill_node(struct net *net,
else
rtm->rtm_type = RTN_UNICAST;
rtm->rtm_flags = 0;
- if (!netif_running(rt->dst.dev) || !netif_carrier_ok(rt->dst.dev)) {
- rtm->rtm_flags |= RTNH_F_LINKDOWN;
- if (rt->rt6i_idev->cnf.ignore_routes_with_linkdown)
- rtm->rtm_flags |= RTNH_F_DEAD;
- }
rtm->rtm_scope = RT_SCOPE_UNIVERSE;
rtm->rtm_protocol = rt->rt6i_protocol;
if (rt->rt6i_flags & RTF_DYNAMIC)
@@ -3317,17 +3395,36 @@ static int rt6_fill_node(struct net *net,
if (rtnetlink_put_metrics(skb, metrics) < 0)
goto nla_put_failure;
- if (rt->rt6i_flags & RTF_GATEWAY) {
- if (nla_put_in6_addr(skb, RTA_GATEWAY, &rt->rt6i_gateway) < 0)
- goto nla_put_failure;
- }
-
- if (rt->dst.dev &&
- nla_put_u32(skb, RTA_OIF, rt->dst.dev->ifindex))
- goto nla_put_failure;
if (nla_put_u32(skb, RTA_PRIORITY, rt->rt6i_metric))
goto nla_put_failure;
+ /* For multipath routes, walk the siblings list and add
+ * each as a nexthop within RTA_MULTIPATH. If this route
+ * is an append, then only send the route that is added.
+ */
+ if (rt->rt6i_nsiblings && !(flags & NLM_F_APPEND)) {
+ struct rt6_info *sibling, *next_sibling;
+ struct nlattr *mp;
+
+ mp = nla_nest_start(skb, RTA_MULTIPATH);
+ if (!mp)
+ goto nla_put_failure;
+
+ if (rt6_add_nexthop(skb, rt) < 0)
+ goto nla_put_failure;
+
+ list_for_each_entry_safe(sibling, next_sibling,
+ &rt->rt6i_siblings, rt6i_siblings) {
+ if (rt6_add_nexthop(skb, sibling) < 0)
+ goto nla_put_failure;
+ }
+
+ nla_nest_end(skb, mp);
+ } else {
+ if (rt6_nexthop_info(skb, rt, &rtm->rtm_flags) < 0)
+ goto nla_put_failure;
+ }
+
expires = (rt->rt6i_flags & RTF_EXPIRES) ? rt->dst.expires - jiffies : 0;
if (rtnl_put_cacheinfo(skb, &rt->dst, 0, expires, rt->dst.error) < 0)
@@ -3336,8 +3433,6 @@ static int rt6_fill_node(struct net *net,
if (nla_put_u8(skb, RTA_PREF, IPV6_EXTRACT_PREF(rt->rt6i_flags)))
goto nla_put_failure;
- if (lwtunnel_fill_encap(skb, rt->dst.lwtstate) < 0)
- goto nla_put_failure;
nlmsg_end(skb, nlh);
return 0;
--
2.1.4
^ permalink raw reply related
* [PATCH net-next v3 1/4] net: ipv6: add NLM_F_APPEND in notifications when applicable
From: David Ahern @ 2017-01-27 23:20 UTC (permalink / raw)
To: netdev; +Cc: roopa, nicolas.dichtel, David Ahern
In-Reply-To: <1485559258-4856-1-git-send-email-dsa@cumulusnetworks.com>
IPv6 does not set the NLM_F_APPEND flag in notifications to signal that
a NEWROUTE is an append versus a new route or a replaced one. Add the
flag if the request has it.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
net/ipv6/ip6_fib.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index ef5485204522..bcaf247232d7 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -746,6 +746,9 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
u16 nlflags = NLM_F_EXCL;
int err;
+ if (info->nlh && info->nlh->nlmsg_flags & NLM_F_APPEND)
+ nlflags |= NLM_F_APPEND;
+
ins = &fn->leaf;
for (iter = fn->leaf; iter; iter = iter->dst.rt6_next) {
--
2.1.4
^ permalink raw reply related
* [PATCH net-next v3 2/4] net: ipv6: Allow shorthand delete of all nexthops in multipath route
From: David Ahern @ 2017-01-27 23:20 UTC (permalink / raw)
To: netdev; +Cc: roopa, nicolas.dichtel, David Ahern
In-Reply-To: <1485559258-4856-1-git-send-email-dsa@cumulusnetworks.com>
IPv4 allows multipath routes to be deleted using just the prefix and
length. For example:
$ ip ro ls vrf red
unreachable default metric 8192
1.1.1.0/24
nexthop via 10.100.1.254 dev eth1 weight 1
nexthop via 10.11.200.2 dev eth11.200 weight 1
10.11.200.0/24 dev eth11.200 proto kernel scope link src 10.11.200.3
10.100.1.0/24 dev eth1 proto kernel scope link src 10.100.1.3
$ ip ro del 1.1.1.0/24 vrf red
$ ip ro ls vrf red
unreachable default metric 8192
10.11.200.0/24 dev eth11.200 proto kernel scope link src 10.11.200.3
10.100.1.0/24 dev eth1 proto kernel scope link src 10.100.1.3
The same notation does not work with IPv6 because of how multipath routes
are implemented for IPv6. For IPv6 only the first nexthop of a multipath
route is deleted if the request contains only a prefix and length. This
leads to unnecessary complexity in userspace dealing with IPv6 multipath
routes.
This patch allows all nexthops to be deleted without specifying each one
in the delete request. Internally, this is done by walking the sibling
list of the route matching the specifications given (prefix, length,
metric, protocol, etc).
$ ip -6 ro ls vrf red
2001:db8::/120 via 2001:db8:1::62 dev eth1 metric 256 pref medium
2001:db8::/120 via 2001:db8:1::61 dev eth1 metric 256 pref medium
2001:db8::/120 via 2001:db8:1::60 dev eth1 metric 256 pref medium
2001:db8:1::/120 dev eth1 proto kernel metric 256 pref medium
...
$ ip -6 ro del vrf red 1111::1/120
$ ip -6 ro ls vrf red
2001:db8:1::/120 dev eth1 proto kernel metric 256 pref medium
...
Because IPv6 allows individual nexthops to be deleted without deleting
the entire route, the mutipath and non-multipath code paths have to be
discriminated so that all nexthops are only deleted for the latter case.
This is done by making the existing fc_type in fib6_config a u16 and then
adding a new u16 field with fc_delete_all_nh as the first bit.
Suggested-by: Dinesh Dutt <ddutt@cumulusnetworks.com>
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
v3
- removed need for RTM_F_ALL_NEXTHOPS user api
v2
- fixed locking deleting route and its siblings as noted by DaveM
v2' (patch originally submitted standalone)
- switched examples to rfc 3849 documentation address per request
- changed delete loop to explicitly look at siblings list for
first route matching specs given (metric, protocol, etc)
include/net/ip6_fib.h | 4 +++-
net/ipv6/route.c | 34 ++++++++++++++++++++++++++++++++--
2 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index a74e2aa40ef4..c979c878df1c 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -37,7 +37,9 @@ struct fib6_config {
int fc_ifindex;
u32 fc_flags;
u32 fc_protocol;
- u32 fc_type; /* only 8 bits are used */
+ u16 fc_type; /* only 8 bits are used */
+ u16 fc_delete_all_nh : 1,
+ __unused : 15;
struct in6_addr fc_dst;
struct in6_addr fc_src;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 5046d2b24004..81e2b2a28806 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2143,6 +2143,34 @@ int ip6_del_rt(struct rt6_info *rt)
return __ip6_del_rt(rt, &info);
}
+static int __ip6_del_rt_siblings(struct rt6_info *rt, struct fib6_config *cfg)
+{
+ struct nl_info *info = &cfg->fc_nlinfo;
+ struct fib6_table *table;
+ int err;
+
+ table = rt->rt6i_table;
+ write_lock_bh(&table->tb6_lock);
+
+ if (rt->rt6i_nsiblings && cfg->fc_delete_all_nh) {
+ struct rt6_info *sibling, *next_sibling;
+
+ list_for_each_entry_safe(sibling, next_sibling,
+ &rt->rt6i_siblings,
+ rt6i_siblings) {
+ err = fib6_del(sibling, info);
+ if (err)
+ goto out;
+ }
+ }
+
+ err = fib6_del(rt, info);
+out:
+ write_unlock_bh(&table->tb6_lock);
+ ip6_rt_put(rt);
+ return err;
+}
+
static int ip6_route_del(struct fib6_config *cfg)
{
struct fib6_table *table;
@@ -2179,7 +2207,7 @@ static int ip6_route_del(struct fib6_config *cfg)
dst_hold(&rt->dst);
read_unlock_bh(&table->tb6_lock);
- return __ip6_del_rt(rt, &cfg->fc_nlinfo);
+ return __ip6_del_rt_siblings(rt, cfg);
}
}
read_unlock_bh(&table->tb6_lock);
@@ -3131,8 +3159,10 @@ static int inet6_rtm_delroute(struct sk_buff *skb, struct nlmsghdr *nlh)
if (cfg.fc_mp)
return ip6_route_multipath_del(&cfg);
- else
+ else {
+ cfg.fc_delete_all_nh = 1;
return ip6_route_del(&cfg);
+ }
}
static int inet6_rtm_newroute(struct sk_buff *skb, struct nlmsghdr *nlh)
--
2.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox