Re: Issue with load across multiple connections
Dear Eric, My apologies for taking so long to get back to you - I had to wait for some experiments to finish until I could grab hold of two machines that weren't busy and had a more or less direct connection. On the server (a Super Micro): root@serverQ:/home/lei/Desktop/servers-20160311# cat /proc/sys/net/ipv4/tcp_rmem 409687380 6291456 root@serverQ:/home/lei/Desktop/servers-20160311# cat /proc/sys/net/ipv4/tcp_wmem 409616384 4194304 root@serverQ:/home/lei/Desktop/servers-20160311# cat /proc/sys/net/ipv4/tcp_mem 47337 63117 94674 On the client (a Raspberry Pi): root@server-controller:/home/lei/20160226/servers-20160226# cat /proc/sys/net/ipv4/tcp_rmem 409687380 6291456 root@server-controller:/home/lei/20160226/servers-20160226# cat /proc/sys/net/ipv4/tcp_wmem 409616384 4194304 root@server-controller:/home/lei/20160226/servers-20160226# cat /proc/sys/net/ipv4/tcp_mem 22206 29611 44412 nstat output: On server: root@serverQ:/home/lei/Desktop/servers-20160311# nstat #kernel IpInReceives223487 0.0 IpInDelivers223487 0.0 IpOutRequests 242888 0.0 TcpPassiveOpens 2625 0.0 TcpEstabResets 1 0.0 TcpInSegs 217980 0.0 TcpOutSegs 227965 0.0 TcpRetransSegs 14888 0.0 TcpOutRsts 6350.0 UdpInDatagrams 8090.0 UdpOutDatagrams 32 0.0 Ip6InReceives 21 0.0 Ip6InDelivers 17 0.0 Ip6OutRequests 4 0.0 Ip6InMcastPkts 17 0.0 Ip6OutMcastPkts 8 0.0 Ip6InOctets 1480 0.0 Ip6OutOctets2880.0 Ip6InMcastOctets1192 0.0 Ip6OutMcastOctets 5760.0 Ip6InNoECTPkts 21 0.0 Icmp6InMsgs 13 0.0 Icmp6OutMsgs4 0.0 Icmp6InGroupMembQueries 4 0.0 Icmp6InGroupMembResponses 4 0.0 Icmp6InNeighborAdvertisements 5 0.0 Icmp6OutGroupMembResponses 4 0.0 Icmp6InType130 4 0.0 Icmp6InType131 4 0.0 Icmp6InType136 5 0.0 Icmp6OutType131 4 0.0 TcpExtSyncookiesSent1820.0 TcpExtSyncookiesRecv1820.0 TcpExtSyncookiesFailed 6220.0 TcpExtTW3370.0 TcpExtPAWSEstab 34317 0.0 TcpExtDelayedACKs 3 0.0 TcpExtDelayedACKLost7 0.0 TcpExtListenOverflows 8 0.0 TcpExtListenDrops 1900.0 TcpExtTCPHPHits 2 0.0 TcpExtTCPPureAcks 95602 0.0 TcpExtTCPHPAcks 14 0.0 TcpExtTCPSackRecovery 2784 0.0 TcpExtTCPSACKReorder1 0.0 TcpExtTCPFullUndo 1901 0.0 TcpExtTCPPartialUndo8830.0 TcpExtTCPFastRetrans1292 0.0 TcpExtTCPForwardRetrans 13592 0.0 TcpExtTCPTimeouts 4 0.0 TcpExtTCPLossProbes 18 0.0 TcpExtTCPDSACKOldSent 7 0.0 TcpExtTCPDSACKRecv 97 0.0 TcpExtTCPDSACKIgnoredNoUndo 97 0.0 TcpExtTCPSackShiftFallback 207045 0.0 TcpExtTCPReqQFullDoCookies 1820.0 IpExtInMcastPkts8170.0 IpExtOutMcastPkts 2 0.0 IpExtInBcastPkts4690 0.0 IpExtInOctets 15946943 0.0 IpExtOutOctets 295423944 0.0 IpExtInMcastOctets 200946 0.0 IpExtOutMcastOctets 64 0.0 IpExtInBcastOctets 629914 0.0 IpExtInNoECTPkts223487 0.0 On client: root@server-controller:/home/lei/20160226/servers-20160226# nstat #kernel IpInReceives249082 0.0 IpInDelivers249030 0.0 IpOutRequests 218185 0.0 TcpActiveOpens 2641 0.0 TcpInSegs 242884 0.0 TcpOutSegs
Re: [kernel-hardening] [PATCH net-next v6 07/11] landlock: Add ptrace restrictions
On Wed, Mar 29, 2017 at 1:46 AM, Mickaël Salaün wrote:
> A landlocked process has less privileges than a non-landlocked process
> and must then be subject to additional restrictions when manipulating
> processes. To be allowed to use ptrace(2) and related syscalls on a
> target process, a landlocked process must have a subset of the target
> process' rules.
>
> New in v6
>
> Signed-off-by: Mickaël Salaün
> Cc: Alexei Starovoitov
> Cc: Andy Lutomirski
> Cc: Daniel Borkmann
> Cc: David S. Miller
> Cc: James Morris
> Cc: Kees Cook
> Cc: Serge E. Hallyn
> ---
> security/landlock/Makefile | 2 +-
> security/landlock/hooks_ptrace.c | 126
> +++
> security/landlock/hooks_ptrace.h | 11
> security/landlock/init.c | 2 +
> 4 files changed, 140 insertions(+), 1 deletion(-)
> create mode 100644 security/landlock/hooks_ptrace.c
> create mode 100644 security/landlock/hooks_ptrace.h
>
[...]
> +/**
> + * landlock_ptrace_access_check - determine whether the current process may
> + * access another
> + *
> + * @child: the process to be accessed
> + * @mode: the mode of attachment
> + *
> + * If the current task has Landlock rules, then the child must have at least
> + * the same rules. Else denied.
> + *
> + * Determine whether a process may access another, returning 0 if permission
> + * granted, -errno if denied.
> + */
> +static int landlock_ptrace_access_check(struct task_struct *child,
> + unsigned int mode)
> +{
> + if (!landlocked(current))
> + return 0;
> +
> + if (!landlocked(child))
> + return -EPERM;
> +
> + if (landlock_task_has_subset_events(current, child))
> + return 0;
> +
> + return -EPERM;
> +}
> +
Maybe you want to check the mode argument here if it is a
PTRACE_ATTACH which may translate to read/writes ? PTRACE_READ are
normally for reads only. Or also which creds were used if this was a
direct syscall or a filesystem call through procfs.
I'm bringing this, since you may want to make some room for landlock
ptrace events and what others may want to do with it. Also I'm
planning to send another v2 RFC for procfs separate instances [1], the
aim is to give LSMs a security_ptrace_access_check hook path when
dealing with /proc// [2] . Right now LSMs don't really have a
security path there, and the implementation does not guarantee that.
With this Yama ptrace scope or other LSMs may take advantage of it and
check the 'PTRACE_MODE_READ_FSCRED' mode for filesystem accesses.
That's why I think it would be better if the default landlock ptrace
semantics are not that wide.
Thanks!
[1] https://lkml.org/lkml/2017/3/30/670
[2] http://lxr.free-electrons.com/source/fs/proc/base.c#L719
[PATCH] ipv6: Fix idev->addr_list corruption
From: Rabin Vincent
addrconf_ifdown() removes elements from the idev->addr_list without
holding the idev->lock.
If this happens while the loop in __ipv6_dev_get_saddr() is handling the
same element, that function ends up in an infinite loop:
NMI watchdog: BUG: soft lockup - CPU#1 stuck for 23s! [test:1719]
Call Trace:
ipv6_get_saddr_eval+0x13c/0x3a0
__ipv6_dev_get_saddr+0xe4/0x1f0
ipv6_dev_get_saddr+0x1b4/0x204
ip6_dst_lookup_tail+0xcc/0x27c
ip6_dst_lookup_flow+0x38/0x80
udpv6_sendmsg+0x708/0xba8
sock_sendmsg+0x18/0x30
SyS_sendto+0xb8/0xf8
syscall_common+0x34/0x58
Fixes: 6a923934c33 (Revert "ipv6: Revert optional address flusing on ifdown.")
Signed-off-by: Rabin Vincent
---
net/ipv6/addrconf.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 3631725..80ce478 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3626,14 +3626,19 @@ static int addrconf_ifdown(struct net_device *dev, int
how)
INIT_LIST_HEAD(&del_list);
list_for_each_entry_safe(ifa, tmp, &idev->addr_list, if_list) {
struct rt6_info *rt = NULL;
+ bool keep;
addrconf_del_dad_work(ifa);
+ keep = keep_addr && (ifa->flags & IFA_F_PERMANENT) &&
+ !addr_is_local(&ifa->addr);
+ if (!keep)
+ list_move(&ifa->if_list, &del_list);
+
write_unlock_bh(&idev->lock);
spin_lock_bh(&ifa->lock);
- if (keep_addr && (ifa->flags & IFA_F_PERMANENT) &&
- !addr_is_local(&ifa->addr)) {
+ if (keep) {
/* set state to skip the notifier below */
state = INET6_IFADDR_STATE_DEAD;
ifa->state = 0;
@@ -3645,8 +3650,6 @@ static int addrconf_ifdown(struct net_device *dev, int
how)
} else {
state = ifa->state;
ifa->state = INET6_IFADDR_STATE_DEAD;
-
- list_move(&ifa->if_list, &del_list);
}
spin_unlock_bh(&ifa->lock);
--
2.7.0
Re: [PATCH 1/5] netlink: extended ACK reporting
> perhaps I misunderstand something, but nla_parse suggests attribute > type can not be 0: [...] Yes, some - very few - families still insist on using attribute 0, perhaps parsing by hand or so. Like you say though, the entire infrastructure makes that hard and undesirable, so I don't really see why we need to invest the extra code/work into making it work *here*, especially since it's such a corner case as I described in my other email. johannes
Re: [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list
> > If you just don't want to list things multiple times how about: > > > > linux/bpf_verifiers.h: > > BPF_VERIFIER(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter_prog_ops) > > BPF_VERIFIER(BPF_PROG_TYPE_SCHED_CLS, tc_cls_prog_ops) > > ... > > > > Then in bpf.h: > > > > #define BPF_VERIFIER(TYPE_VAL, SYM) extern const struct > > bpf_verifier_ops SYM; > > #include > > > and in kernel/bpf/syscall.c: > > > > #define BPF_VERIFIER(TYPE_VAL, SYM) [TYPE_VAL] = &SYM, > > #include > > > or something like that. > > That is great suggestion! I like it. It'd doable, but then I need to play with the include guards, like having "#define __BPF_REINCLUDE" or something like that before reincluding it, I guess? Unless that file can simply not be included anywhere? Maybe I'll play with it - though perhaps it should be named bpf_types.h or something like that so we don't have to have two files. johannes
[PATCH] p54: add null pointer check before releasing socket buffer
Kernel panic is caused by trying to dereference null pointer. Check if the pointer is null before freeing space. Signed-off-by: Myungho Jung --- drivers/net/wireless/intersil/p54/txrx.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c index 1af7da0..8956061 100644 --- a/drivers/net/wireless/intersil/p54/txrx.c +++ b/drivers/net/wireless/intersil/p54/txrx.c @@ -503,7 +503,9 @@ static void p54_rx_eeprom_readback(struct p54_common *priv, priv->eeprom = NULL; tmp = p54_find_and_unlink_skb(priv, hdr->req_id); - dev_kfree_skb_any(tmp); + if (unlikely(!tmp)) + dev_kfree_skb_any(tmp); + complete(&priv->eeprom_comp); } @@ -597,7 +599,9 @@ static void p54_rx_stats(struct p54_common *priv, struct sk_buff *skb) } tmp = p54_find_and_unlink_skb(priv, hdr->req_id); - dev_kfree_skb_any(tmp); + if (unlikely(!tmp)) + dev_kfree_skb_any(tmp); + complete(&priv->stat_comp); } -- 2.7.4
Re: [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list
On Sun, Apr 09, 2017 at 06:22:36PM -0700, David Miller wrote: > From: Johannes Berg > Date: Fri, 7 Apr 2017 21:00:07 +0200 > > > From: Johannes Berg > > > > There's no need to have struct bpf_prog_type_list since > > it just contains a list_head, the type, and the ops > > pointer. Since the types are densely packed and not > > actually dynamically registered, it's much easier and > > smaller to have an array of type->ops pointer. Also > > initialize this array statically to remove code needed > > to initialize it. > > > > Signed-off-by: Johannes Berg > > If you just don't want to list things multiple times how about: > > linux/bpf_verifiers.h: > BPF_VERIFIER(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter_prog_ops) > BPF_VERIFIER(BPF_PROG_TYPE_SCHED_CLS, tc_cls_prog_ops) > ... > > Then in bpf.h: > > #define BPF_VERIFIER(TYPE_VAL, SYM) extern const struct bpf_verifier_ops SYM; > #include > and in kernel/bpf/syscall.c: > > #define BPF_VERIFIER(TYPE_VAL, SYM) [TYPE_VAL] = &SYM, > #include > or something like that. That is great suggestion! I like it.
Re: [PATCH v2 net-next RFC] Generic XDP
On Sun, Apr 09, 2017 at 01:35:28PM -0700, David Miller wrote:
>
> This provides a generic non-optimized XDP implementation when the
> device driver does not provide an optimized one.
>
> It is arguable that perhaps I should have required something like
> this as part of the initial XDP feature merge.
>
> I believe this is critical for two reasons:
>
> 1) Accessibility. More people can play with XDP with less
>dependencies. Yes I know we have XDP support in virtio_net, but
>that just creates another depedency for learning how to use this
>facility.
>
>I wrote this to make life easier for the XDP newbies.
>
> 2) As a model for what the expected semantics are. If there is a pure
>generic core implementation, it serves as a semantic example for
>driver folks adding XDP support.
>
> This is just a rough draft and is untested.
>
> Signed-off-by: David S. Miller
...
> +static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> + struct bpf_prog *xdp_prog)
> +{
> + struct xdp_buff xdp;
> + u32 act = XDP_DROP;
> + void *orig_data;
> + int hlen, off;
> +
> + if (skb_linearize(skb))
> + goto do_drop;
do we need to force disable gro ?
Otherwise if we linearize skb_is_gso packet it will be huge
and not similar to normal xdp packets?
gso probably needs to disabled too to avoid veth surprises?
> +
> + hlen = skb_headlen(skb);
> + xdp.data = skb->data;
it probably should be
hlen = skb_headlen(skb) + skb->mac_len;
xdp.data = skb->data - skb->mac_len;
to make sure xdp program is looking at l2 header.
> + xdp.data_end = xdp.data + hlen;
> + xdp.data_hard_start = xdp.data - skb_headroom(skb);
> + orig_data = xdp.data;
> + act = bpf_prog_run_xdp(xdp_prog, &xdp);
> +
> + off = xdp.data - orig_data;
> + if (off)
> + __skb_push(skb, off);
and restore l2 back somehow and get new skb->protocol ?
if we simply do __skb_pull(skb, skb->mac_len); like
we do with cls_bpf, it will not work correctly,
since if the program did ip->ipip encap (like our balancer
does and the test tools/testing/selftests/bpf/test_xdp.c)
the skb metadata fields will be wrong.
So we need to repeat eth_type_trans() here if (xdp.data != orig_data)
In case of cls_bpf when we mess with skb sizes we always
adjust skb metafields in helpers, so there it's fine
and __skb_pull(skb, skb->mac_len); is enough.
Here we need to be a bit more careful.
> static int netif_receive_skb_internal(struct sk_buff *skb)
> {
> int ret;
> @@ -4258,6 +4336,21 @@ static int netif_receive_skb_internal(struct sk_buff
> *skb)
>
> rcu_read_lock();
>
> + if (static_key_false(&generic_xdp_needed)) {
> + struct bpf_prog *xdp_prog = rcu_dereference(skb->dev->xdp_prog);
> +
> + if (xdp_prog) {
> + u32 act = netif_receive_generic_xdp(skb, xdp_prog);
That's indeed the best attachment point in the stack.
I was trying to see whether it can be lowered into something like
dev_gro_receive(), but not everyone calls it.
Another option to put it into eth_type_trans() itself, then
there are no problems with gro, l2 headers, and adjust_head,
but changing all drivers is too much.
> +
> + if (act != XDP_PASS) {
> + rcu_read_unlock();
> + if (act == XDP_TX)
> + dev_queue_xmit(skb);
It should be fine. For cls_bpf we do recursion check __bpf_tx_skb()
but I forgot specific details. May be here it's fine as-is.
Daniel, do we need recursion check here?
> @@ -6725,14 +6819,16 @@ int dev_change_xdp_fd(struct net_device *dev, int fd,
> u32 flags)
>
> ASSERT_RTNL();
>
> - if (!ops->ndo_xdp)
> - return -EOPNOTSUPP;
> + xdp_op = ops->ndo_xdp;
> + if (!xdp_op)
> + xdp_op = generic_xdp_install;
I suspect there always be drivers that don't support xdp (like e100),
so this generic_xdp_install() will stay with us forever.
Since it will stay, can we enable it for xdp enabled drivers too?
This will allow us to test both raw xdp and skb-based paths neck to neck.
Today bpf-newbies typically start developing with cls_bpf, since
it can run on tap/veth and then refactor the program to xdp.
Unfortunately cls_bpf and xdp programs are substantially different,
so it pretty much means that cls_bpf prog is a throw away.
If we can add a flag to this xdp netlink attach command
that says 'enable skb-based xdp', we'll have exactly the same
program running on raw dma buffer and on skb, which will help
developing on veth/tap and moving the same prog into physical
eth0 later. And users will be able to switch between skb-based
mode on eth0 and raw-buffer mode back and forth to see perf difference
(and hopefully nothing else).
Another advantage that it will help to flush out the differences
between skb- and raw- modes in the drivers that support xdp already.
Yet another b
[PATCH v2 02/12] ftgmac100: Move ftgmac100_hard_start_xmit() around
Move it below ftgmac100_xmit() and the rest of the tx path
No code change.
Signed-off-by: Benjamin Herrenschmidt
---
v2. - Fix patch splitting mistake
---
drivers/net/ethernet/faraday/ftgmac100.c | 58
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index f4b719214..21df322 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -673,6 +673,35 @@ static int ftgmac100_xmit(struct ftgmac100 *priv, struct
sk_buff *skb,
return NETDEV_TX_OK;
}
+static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
+struct net_device *netdev)
+{
+ struct ftgmac100 *priv = netdev_priv(netdev);
+ dma_addr_t map;
+
+ if (unlikely(skb->len > MAX_PKT_SIZE)) {
+ if (net_ratelimit())
+ netdev_dbg(netdev, "tx packet too big\n");
+
+ netdev->stats.tx_dropped++;
+ kfree_skb(skb);
+ return NETDEV_TX_OK;
+ }
+
+ map = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
DMA_TO_DEVICE);
+ if (unlikely(dma_mapping_error(priv->dev, map))) {
+ /* drop packet */
+ if (net_ratelimit())
+ netdev_err(netdev, "map socket buffer failed\n");
+
+ netdev->stats.tx_dropped++;
+ kfree_skb(skb);
+ return NETDEV_TX_OK;
+ }
+
+ return ftgmac100_xmit(priv, skb, map);
+}
+
static void ftgmac100_free_buffers(struct ftgmac100 *priv)
{
int i;
@@ -1210,35 +1239,6 @@ static int ftgmac100_stop(struct net_device *netdev)
return 0;
}
-static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
-struct net_device *netdev)
-{
- struct ftgmac100 *priv = netdev_priv(netdev);
- dma_addr_t map;
-
- if (unlikely(skb->len > MAX_PKT_SIZE)) {
- if (net_ratelimit())
- netdev_dbg(netdev, "tx packet too big\n");
-
- netdev->stats.tx_dropped++;
- kfree_skb(skb);
- return NETDEV_TX_OK;
- }
-
- map = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
DMA_TO_DEVICE);
- if (unlikely(dma_mapping_error(priv->dev, map))) {
- /* drop packet */
- if (net_ratelimit())
- netdev_err(netdev, "map socket buffer failed\n");
-
- netdev->stats.tx_dropped++;
- kfree_skb(skb);
- return NETDEV_TX_OK;
- }
-
- return ftgmac100_xmit(priv, skb, map);
-}
-
/* optional */
static int ftgmac100_do_ioctl(struct net_device *netdev, struct ifreq *ifr,
int cmd)
{
--
2.9.3
[PATCH v2 10/12] ftgmac100: Don't clear tx desc fields unnecessarily
Those are non-cachable stores, let's avoid those we don't need. Remove
the helper, it's not particularly helpful and since it uses "priv"
I can't move it to the header file.
Signed-off-by: Benjamin Herrenschmidt
---
drivers/net/ethernet/faraday/ftgmac100.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index b33de5c..8a44c22 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -466,16 +466,6 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv,
int *processed)
return true;
}
-static void ftgmac100_txdes_reset(const struct ftgmac100 *priv,
- struct ftgmac100_txdes *txdes)
-{
- /* clear all except end of ring bit */
- txdes->txdes0 &= cpu_to_le32(priv->txdes0_edotr_mask);
- txdes->txdes1 = 0;
- txdes->txdes2 = 0;
- txdes->txdes3 = 0;
-}
-
static bool ftgmac100_txdes_owned_by_dma(struct ftgmac100_txdes *txdes)
{
return txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
@@ -575,7 +565,12 @@ static void ftgmac100_free_tx_packet(struct ftgmac100
*priv,
dev_kfree_skb(skb);
priv->tx_skbs[pointer] = NULL;
- ftgmac100_txdes_reset(priv, txdes);
+ /* Clear txdes0 except end of ring bit, clear txdes1 as we
+* only "OR" into it, leave 2 and 3 alone as 2 is unused
+* and 3 will be overwritten entirely
+*/
+ txdes->txdes0 &= cpu_to_le32(priv->txdes0_edotr_mask);
+ txdes->txdes1 = 0;
}
static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv)
--
2.9.3
[PATCH v2 03/12] ftgmac100: Merge ftgmac100_xmit() into ftgmac100_hard_start_xmit()
This will make subsequent rework of the tx path simpler
Signed-off-by: Benjamin Herrenschmidt
---
drivers/net/ethernet/faraday/ftgmac100.c | 58 ++--
1 file changed, 25 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 21df322..a5bab0d 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -627,12 +627,33 @@ static void ftgmac100_tx_complete(struct ftgmac100 *priv)
;
}
-static int ftgmac100_xmit(struct ftgmac100 *priv, struct sk_buff *skb,
- dma_addr_t map)
+static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
+struct net_device *netdev)
{
- struct net_device *netdev = priv->netdev;
- struct ftgmac100_txdes *txdes;
unsigned int len = (skb->len < ETH_ZLEN) ? ETH_ZLEN : skb->len;
+ struct ftgmac100 *priv = netdev_priv(netdev);
+ struct ftgmac100_txdes *txdes;
+ dma_addr_t map;
+
+ if (unlikely(skb->len > MAX_PKT_SIZE)) {
+ if (net_ratelimit())
+ netdev_dbg(netdev, "tx packet too big\n");
+
+ netdev->stats.tx_dropped++;
+ kfree_skb(skb);
+ return NETDEV_TX_OK;
+ }
+
+ map = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
DMA_TO_DEVICE);
+ if (unlikely(dma_mapping_error(priv->dev, map))) {
+ /* drop packet */
+ if (net_ratelimit())
+ netdev_err(netdev, "map socket buffer failed\n");
+
+ netdev->stats.tx_dropped++;
+ kfree_skb(skb);
+ return NETDEV_TX_OK;
+ }
txdes = ftgmac100_current_txdes(priv);
ftgmac100_tx_pointer_advance(priv);
@@ -673,35 +694,6 @@ static int ftgmac100_xmit(struct ftgmac100 *priv, struct
sk_buff *skb,
return NETDEV_TX_OK;
}
-static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
-struct net_device *netdev)
-{
- struct ftgmac100 *priv = netdev_priv(netdev);
- dma_addr_t map;
-
- if (unlikely(skb->len > MAX_PKT_SIZE)) {
- if (net_ratelimit())
- netdev_dbg(netdev, "tx packet too big\n");
-
- netdev->stats.tx_dropped++;
- kfree_skb(skb);
- return NETDEV_TX_OK;
- }
-
- map = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
DMA_TO_DEVICE);
- if (unlikely(dma_mapping_error(priv->dev, map))) {
- /* drop packet */
- if (net_ratelimit())
- netdev_err(netdev, "map socket buffer failed\n");
-
- netdev->stats.tx_dropped++;
- kfree_skb(skb);
- return NETDEV_TX_OK;
- }
-
- return ftgmac100_xmit(priv, skb, map);
-}
-
static void ftgmac100_free_buffers(struct ftgmac100 *priv)
{
int i;
--
2.9.3
[PATCH v2 04/12] ftgmac100: Factor tx packet dropping path
Use a simple goto to a drop path at the tail of the function,
it will be used in a few more cases soon
Signed-off-by: Benjamin Herrenschmidt
---
drivers/net/ethernet/faraday/ftgmac100.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index a5bab0d..84ae800 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -638,10 +638,7 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
if (unlikely(skb->len > MAX_PKT_SIZE)) {
if (net_ratelimit())
netdev_dbg(netdev, "tx packet too big\n");
-
- netdev->stats.tx_dropped++;
- kfree_skb(skb);
- return NETDEV_TX_OK;
+ goto drop;
}
map = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
DMA_TO_DEVICE);
@@ -649,10 +646,7 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
/* drop packet */
if (net_ratelimit())
netdev_err(netdev, "map socket buffer failed\n");
-
- netdev->stats.tx_dropped++;
- kfree_skb(skb);
- return NETDEV_TX_OK;
+ goto drop;
}
txdes = ftgmac100_current_txdes(priv);
@@ -692,6 +686,13 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
ftgmac100_txdma_normal_prio_start_polling(priv);
return NETDEV_TX_OK;
+
+ drop:
+ /* Drop the packet */
+ dev_kfree_skb_any(skb);
+ netdev->stats.tx_dropped++;
+
+ return NETDEV_TX_OK;
}
static void ftgmac100_free_buffers(struct ftgmac100 *priv)
--
2.9.3
RE: IMX6 FEC connection drops occasionally with 'MDIO read timeout'
From: Tim Harvey Sent: Friday, April 07, 2017 10:47 PM >To: netdev >Cc: Fabio Estevam ; Lucas Stach >; Andy Duan ; Koen >Vandeputte >Subject: IMX6 FEC connection drops occasionally with 'MDIO read timeout' > >Greetings, > >I've had a couple of users report that they are getting occasional link drops >when using IMX6 FEC on our boards which are using a Marvell >88E1510 PHY: > >[ 9348.474418] fec 2188000.ethernet eth0: MDIO read timeout [ 9350.478804] >fec 2188000.ethernet eth0: Link is Down > >If they bring the interface back up it works fine. > >The most recent version of Linux they have tested and seen this on is >v4.9 with CONFIG_MARVELL_PHY=y > >Has anyone else run into this or know what the issue could be? Should there >be any retries at the MDIO level or above? > >Regards, > >Tim Pls keep ipg clock always on and try it. In general, MDIO timeout maybe caused by ipg clock off and mii interrupt lost. Regards, Andy
Re: [PATCH net-next] net: dsa: mt7530: Include gpio/consumer.h for GPIO functions
From: Florian Fainelli
Date: Sat, 8 Apr 2017 08:52:02 -0700
> Fixes build errors seen with CONFIG_GPIOLIB disabled and warnings enabled:
...
> Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530
> switch")
> Signed-off-by: Florian Fainelli
Applied, thanks Florian.
Re: [PATCH net] tcp: clear saved_syn in tcp_disconnect()
From: Eric Dumazet
Date: Sat, 08 Apr 2017 08:07:33 -0700
> From: Eric Dumazet
>
> In the (very unlikely) case a passive socket becomes a listener,
> we do not want to duplicate its saved SYN headers.
>
> This would lead to double frees, use after free, and please hackers and
> various fuzzers
>
> Tested:
...
> Fixes: cd8ae85299d5 ("tcp: provide SYN headers for passive connections")
> Signed-off-by: Eric Dumazet
Applied, thanks Eric.
Re: [PATCH net-next] bpf: fix comment typo
From: Alexander Alemayhu Date: Sat, 8 Apr 2017 22:08:10 +0200 > o s/bpf_bpf_get_socket_cookie/bpf_get_socket_cookie > > Signed-off-by: Alexander Alemayhu Applied.
Re: [PATCH v2 1/2] bpf: remove struct bpf_prog_type_list
From: Johannes Berg Date: Fri, 7 Apr 2017 21:00:07 +0200 > From: Johannes Berg > > There's no need to have struct bpf_prog_type_list since > it just contains a list_head, the type, and the ops > pointer. Since the types are densely packed and not > actually dynamically registered, it's much easier and > smaller to have an array of type->ops pointer. Also > initialize this array statically to remove code needed > to initialize it. > > Signed-off-by: Johannes Berg If you just don't want to list things multiple times how about: linux/bpf_verifiers.h: BPF_VERIFIER(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter_prog_ops) BPF_VERIFIER(BPF_PROG_TYPE_SCHED_CLS, tc_cls_prog_ops) ... Then in bpf.h: #define BPF_VERIFIER(TYPE_VAL, SYM) extern const struct bpf_verifier_ops SYM; #include
[PATCH v2 11/12] ftgmac100: Add support for fragmented tx
Add NETIF_F_SG and create multiple TX ring entries for skb fragments.
On reclaim, the skb is only freed on the segment marked as "last".
Signed-off-by: Benjamin Herrenschmidt
--
v2. - Remove skb headlen size adjustments now that we use
eth_skb_pad().
---
drivers/net/ethernet/faraday/ftgmac100.c | 119 ---
1 file changed, 94 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 8a44c22..2a4d903 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -45,7 +45,7 @@
#define RX_BUF_SIZEMAX_PKT_SIZE/* must be smaller than 0x3fff
*/
/* Min number of tx ring entries before stopping queue */
-#define TX_THRESHOLD (1)
+#define TX_THRESHOLD (MAX_SKB_FRAGS + 1)
struct ftgmac100_descs {
struct ftgmac100_rxdes rxdes[RX_QUEUE_ENTRIES];
@@ -487,20 +487,30 @@ static void ftgmac100_txdes_set_first_segment(struct
ftgmac100_txdes *txdes)
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_FTS);
}
+static inline bool ftgmac100_txdes_get_first_segment(struct ftgmac100_txdes
*txdes)
+{
+ return (txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_FTS)) != 0;
+}
+
static void ftgmac100_txdes_set_last_segment(struct ftgmac100_txdes *txdes)
{
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_LTS);
}
+static inline bool ftgmac100_txdes_get_last_segment(struct ftgmac100_txdes
*txdes)
+{
+ return (txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_LTS)) != 0;
+}
+
static void ftgmac100_txdes_set_buffer_size(struct ftgmac100_txdes *txdes,
unsigned int len)
{
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXBUF_SIZE(len));
}
-static void ftgmac100_txdes_set_txint(struct ftgmac100_txdes *txdes)
+static inline unsigned int ftgmac100_txdes_get_buffer_size(struct
ftgmac100_txdes *txdes)
{
- txdes->txdes1 |= cpu_to_le32(FTGMAC100_TXDES1_TXIC);
+ return FTGMAC100_TXDES0_TXBUF_SIZE(cpu_to_le32(txdes->txdes0));
}
static void ftgmac100_txdes_set_tcpcs(struct ftgmac100_txdes *txdes)
@@ -526,7 +536,7 @@ static void ftgmac100_txdes_set_dma_addr(struct
ftgmac100_txdes *txdes,
static dma_addr_t ftgmac100_txdes_get_dma_addr(struct ftgmac100_txdes *txdes)
{
- return le32_to_cpu(txdes->txdes3);
+ return (dma_addr_t)le32_to_cpu(txdes->txdes3);
}
static int ftgmac100_next_tx_pointer(int pointer)
@@ -556,13 +566,19 @@ static void ftgmac100_free_tx_packet(struct ftgmac100
*priv,
struct sk_buff *skb,
struct ftgmac100_txdes *txdes)
{
- dma_addr_t map;
+ dma_addr_t map = ftgmac100_txdes_get_dma_addr(txdes);
- map = ftgmac100_txdes_get_dma_addr(txdes);
-
- dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
+ if (ftgmac100_txdes_get_first_segment(txdes)) {
+ dma_unmap_single(priv->dev, map, skb_headlen(skb),
+DMA_TO_DEVICE);
+ } else {
+ dma_unmap_page(priv->dev, map,
+ ftgmac100_txdes_get_buffer_size(txdes),
+ DMA_TO_DEVICE);
+ }
- dev_kfree_skb(skb);
+ if (ftgmac100_txdes_get_last_segment(txdes))
+ dev_kfree_skb(skb);
priv->tx_skbs[pointer] = NULL;
/* Clear txdes0 except end of ring bit, clear txdes1 as we
@@ -624,8 +640,8 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
struct net_device *netdev)
{
struct ftgmac100 *priv = netdev_priv(netdev);
- struct ftgmac100_txdes *txdes;
- unsigned int pointer;
+ struct ftgmac100_txdes *txdes, *first;
+ unsigned int pointer, nfrags, len, i, j;
dma_addr_t map;
/* The HW doesn't pad small frames */
@@ -641,26 +657,33 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
goto drop;
}
- map = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
DMA_TO_DEVICE);
- if (unlikely(dma_mapping_error(priv->dev, map))) {
- /* drop packet */
+ /* Do we have a limit on #fragments ? I yet have to get a reply
+* from Aspeed. If there's one I haven't hit it.
+*/
+ nfrags = skb_shinfo(skb)->nr_frags;
+
+ /* Get header len */
+ len = skb_headlen(skb);
+
+ /* Map the packet head */
+ map = dma_map_single(priv->dev, skb->data, len, DMA_TO_DEVICE);
+ if (dma_mapping_error(priv->dev, map)) {
if (net_ratelimit())
- netdev_err(netdev, "map socket buffer failed\n");
+ netdev_err(netdev, "map tx packet head failed\n");
goto drop;
}
/* Grab the next free tx descriptor */
pointer = priv->tx_pointer;
- txdes
[PATCH v2 07/12] ftgmac100: Cleanup tx queue handling
We have a private lock which isn't terribly useful, and we maintain
a "tx_pending" counter for information that's already available
via a trivial arithmetic operation. Then we unconditionaly wake
the queue even when not stopped. Finally our code in tx isn't
really safe vs. a concurrent reclaim. The aspeed chips aren't SMP
today but I prefer the code being right and future proof.
So rip that out and replace it with more "standard" queue handling,
currently with a threshold of 1 queue element, which will be
increased when we implement fragmented sends.
Signed-off-by: Benjamin Herrenschmidt
---
drivers/net/ethernet/faraday/ftgmac100.c | 103 ---
1 file changed, 68 insertions(+), 35 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 8d85f1d..8078d65 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -44,6 +44,9 @@
#define MAX_PKT_SIZE 1536
#define RX_BUF_SIZEMAX_PKT_SIZE/* must be smaller than 0x3fff
*/
+/* Min number of tx ring entries before stopping queue */
+#define TX_THRESHOLD (1)
+
struct ftgmac100_descs {
struct ftgmac100_rxdes rxdes[RX_QUEUE_ENTRIES];
struct ftgmac100_txdes txdes[TX_QUEUE_ENTRIES];
@@ -66,9 +69,7 @@ struct ftgmac100 {
struct sk_buff *tx_skbs[TX_QUEUE_ENTRIES];
unsigned int tx_clean_pointer;
unsigned int tx_pointer;
- unsigned int tx_pending;
u32 txdes0_edotr_mask;
- spinlock_t tx_lock;
/* Scratch page to use when rx skb alloc fails */
void *rx_scratch;
@@ -163,7 +164,6 @@ static int ftgmac100_reset_and_config_mac(struct ftgmac100
*priv)
priv->rx_pointer = 0;
priv->tx_clean_pointer = 0;
priv->tx_pointer = 0;
- priv->tx_pending = 0;
/* The doc says reset twice with 10us interval */
if (ftgmac100_reset_mac(priv, maccr))
@@ -549,6 +549,23 @@ static int ftgmac100_next_tx_pointer(int pointer)
return (pointer + 1) & (TX_QUEUE_ENTRIES - 1);
}
+static u32 ftgmac100_tx_buf_avail(struct ftgmac100 *priv)
+{
+ /* Returns the number of available slots in the TX queue
+*
+* This always leaves one free slot so we don't have to
+* worry about empty vs. full, and this simplifies the
+* test for ftgmac100_tx_buf_cleanable() below
+*/
+ return (priv->tx_clean_pointer - priv->tx_pointer - 1) &
+ (TX_QUEUE_ENTRIES - 1);
+}
+
+static bool ftgmac100_tx_buf_cleanable(struct ftgmac100 *priv)
+{
+ return priv->tx_pointer != priv->tx_clean_pointer;
+}
+
static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv)
{
struct net_device *netdev = priv->netdev;
@@ -557,9 +574,6 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100
*priv)
struct sk_buff *skb;
dma_addr_t map;
- if (priv->tx_pending == 0)
- return false;
-
pointer = priv->tx_clean_pointer;
txdes = &priv->descs->txdes[pointer];
@@ -581,18 +595,31 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100
*priv)
priv->tx_clean_pointer = ftgmac100_next_tx_pointer(pointer);
- spin_lock(&priv->tx_lock);
- priv->tx_pending--;
- spin_unlock(&priv->tx_lock);
- netif_wake_queue(netdev);
-
return true;
}
static void ftgmac100_tx_complete(struct ftgmac100 *priv)
{
- while (ftgmac100_tx_complete_packet(priv))
+ struct net_device *netdev = priv->netdev;
+
+ /* Process all completed packets */
+ while (ftgmac100_tx_buf_cleanable(priv) &&
+ ftgmac100_tx_complete_packet(priv))
;
+
+ /* Restart queue if needed */
+ smp_mb();
+ if (unlikely(netif_queue_stopped(netdev) &&
+ftgmac100_tx_buf_avail(priv) >= TX_THRESHOLD)) {
+ struct netdev_queue *txq;
+
+ txq = netdev_get_tx_queue(netdev, 0);
+ __netif_tx_lock(txq, smp_processor_id());
+ if (netif_queue_stopped(netdev) &&
+ ftgmac100_tx_buf_avail(priv) >= TX_THRESHOLD)
+ netif_wake_queue(netdev);
+ __netif_tx_unlock(txq);
+ }
}
static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
@@ -650,17 +677,22 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
}
}
+ ftgmac100_txdes_set_dma_own(txdes);
+
/* Update next TX pointer */
priv->tx_pointer = ftgmac100_next_tx_pointer(pointer);
- spin_lock(&priv->tx_lock);
- priv->tx_pending++;
- if (priv->tx_pending == TX_QUEUE_ENTRIES)
+ /* If there isn't enough room for all the fragments of a new packet
+* in the TX ring, stop the queue. The sequence below is race free
+* vs. a concurrent restart in ftgmac100_poll()
+*/
+ if (unlikely(ftgmac100_
[PATCH v2 06/12] ftgmac100: Store tx skbs in a separate array
Rather than in the descriptor. The descriptor is mapped non-cachable
and rather slow to access.
Since to do that we need to keep track of the tx "pointer" we also
have no use of all the accesors to manipulate it, just open code
it, it's as clear and will help when adding fragmented sends.
Signed-off-by: Benjamin Herrenschmidt
---
v2. - Fix patch splitting mistake
---
drivers/net/ethernet/faraday/ftgmac100.c | 59 +---
1 file changed, 16 insertions(+), 43 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 739916d..8d85f1d 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -63,6 +63,7 @@ struct ftgmac100 {
u32 rxdes0_edorr_mask;
/* Tx ring */
+ struct sk_buff *tx_skbs[TX_QUEUE_ENTRIES];
unsigned int tx_clean_pointer;
unsigned int tx_pointer;
unsigned int tx_pending;
@@ -543,63 +544,29 @@ static dma_addr_t ftgmac100_txdes_get_dma_addr(struct
ftgmac100_txdes *txdes)
return le32_to_cpu(txdes->txdes3);
}
-/*
- * txdes2 is not used by hardware. We use it to keep track of socket buffer.
- * Since hardware does not touch it, we can skip cpu_to_le32()/le32_to_cpu().
- */
-static void ftgmac100_txdes_set_skb(struct ftgmac100_txdes *txdes,
- struct sk_buff *skb)
-{
- txdes->txdes2 = (unsigned int)skb;
-}
-
-static struct sk_buff *ftgmac100_txdes_get_skb(struct ftgmac100_txdes *txdes)
-{
- return (struct sk_buff *)txdes->txdes2;
-}
-
static int ftgmac100_next_tx_pointer(int pointer)
{
return (pointer + 1) & (TX_QUEUE_ENTRIES - 1);
}
-static void ftgmac100_tx_pointer_advance(struct ftgmac100 *priv)
-{
- priv->tx_pointer = ftgmac100_next_tx_pointer(priv->tx_pointer);
-}
-
-static void ftgmac100_tx_clean_pointer_advance(struct ftgmac100 *priv)
-{
- priv->tx_clean_pointer =
ftgmac100_next_tx_pointer(priv->tx_clean_pointer);
-}
-
-static struct ftgmac100_txdes *ftgmac100_current_txdes(struct ftgmac100 *priv)
-{
- return &priv->descs->txdes[priv->tx_pointer];
-}
-
-static struct ftgmac100_txdes *
-ftgmac100_current_clean_txdes(struct ftgmac100 *priv)
-{
- return &priv->descs->txdes[priv->tx_clean_pointer];
-}
-
static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv)
{
struct net_device *netdev = priv->netdev;
struct ftgmac100_txdes *txdes;
+ unsigned int pointer;
struct sk_buff *skb;
dma_addr_t map;
if (priv->tx_pending == 0)
return false;
- txdes = ftgmac100_current_clean_txdes(priv);
+ pointer = priv->tx_clean_pointer;
+ txdes = &priv->descs->txdes[pointer];
if (ftgmac100_txdes_owned_by_dma(txdes))
return false;
- skb = ftgmac100_txdes_get_skb(txdes);
+ skb = priv->tx_skbs[pointer];
map = ftgmac100_txdes_get_dma_addr(txdes);
netdev->stats.tx_packets++;
@@ -608,10 +575,11 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100
*priv)
dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
dev_kfree_skb(skb);
+ priv->tx_skbs[pointer] = NULL;
ftgmac100_txdes_reset(priv, txdes);
- ftgmac100_tx_clean_pointer_advance(priv);
+ priv->tx_clean_pointer = ftgmac100_next_tx_pointer(pointer);
spin_lock(&priv->tx_lock);
priv->tx_pending--;
@@ -632,6 +600,7 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
{
struct ftgmac100 *priv = netdev_priv(netdev);
struct ftgmac100_txdes *txdes;
+ unsigned int pointer;
dma_addr_t map;
/* The HW doesn't pad small frames */
@@ -655,11 +624,12 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
goto drop;
}
- txdes = ftgmac100_current_txdes(priv);
- ftgmac100_tx_pointer_advance(priv);
+ /* Grab the next free tx descriptor */
+ pointer = priv->tx_pointer;
+ txdes = &priv->descs->txdes[pointer];
/* setup TX descriptor */
- ftgmac100_txdes_set_skb(txdes, skb);
+ priv->tx_skbs[pointer] = skb;
ftgmac100_txdes_set_dma_addr(txdes, map);
ftgmac100_txdes_set_buffer_size(txdes, skb->len);
@@ -680,6 +650,9 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
}
}
+ /* Update next TX pointer */
+ priv->tx_pointer = ftgmac100_next_tx_pointer(pointer);
+
spin_lock(&priv->tx_lock);
priv->tx_pending++;
if (priv->tx_pending == TX_QUEUE_ENTRIES)
@@ -722,7 +695,7 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
/* Free all TX buffers */
for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
struct ftgmac100_txdes *txdes = &priv->descs->txdes[i];
- struct sk_buff *skb = ftgmac100_txdes_get_skb(txdes);
+ struct sk_buff *
[PATCH v2 08/12] ftgmac100: Move the barrier out of ftgmac100_txdes_set_dma_own()
We'll use variants of this accessor without barriers when
building series of descriptors for fragmented sends
Signed-off-by: Benjamin Herrenschmidt
---
drivers/net/ethernet/faraday/ftgmac100.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 8078d65..bdec14f 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -483,11 +483,6 @@ static bool ftgmac100_txdes_owned_by_dma(struct
ftgmac100_txdes *txdes)
static void ftgmac100_txdes_set_dma_own(struct ftgmac100_txdes *txdes)
{
- /*
-* Make sure dma own bit will not be set before any other
-* descriptor fields.
-*/
- wmb();
txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
}
@@ -677,6 +672,10 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
}
}
+ /* Order the previous packet and descriptor udpates
+* before setting the OWN bit.
+*/
+ dma_wmb();
ftgmac100_txdes_set_dma_own(txdes);
/* Update next TX pointer */
--
2.9.3
[PATCH v2 12/12] ftgmac100: Remove tx descriptor accessors
Directly access the fields when needed. The accessors add clutter
not clarity and in some cases cause unnecessary read-modify-write
type access on the slow (uncached) descriptor memory.
Signed-off-by: Benjamin Herrenschmidt
---
v2. - Adjust for changes in previous patches
---
drivers/net/ethernet/faraday/ftgmac100.c | 175 ---
drivers/net/ethernet/faraday/ftgmac100.h | 8 +-
2 files changed, 69 insertions(+), 114 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 2a4d903..98b8956 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -466,77 +466,13 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv,
int *processed)
return true;
}
-static bool ftgmac100_txdes_owned_by_dma(struct ftgmac100_txdes *txdes)
+static u32 ftgmac100_base_tx_ctlstat(struct ftgmac100 *priv,
+unsigned int index)
{
- return txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
-}
-
-static void ftgmac100_txdes_set_dma_own(struct ftgmac100_txdes *txdes)
-{
- txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXDMA_OWN);
-}
-
-static void ftgmac100_txdes_set_end_of_ring(const struct ftgmac100 *priv,
- struct ftgmac100_txdes *txdes)
-{
- txdes->txdes0 |= cpu_to_le32(priv->txdes0_edotr_mask);
-}
-
-static void ftgmac100_txdes_set_first_segment(struct ftgmac100_txdes *txdes)
-{
- txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_FTS);
-}
-
-static inline bool ftgmac100_txdes_get_first_segment(struct ftgmac100_txdes
*txdes)
-{
- return (txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_FTS)) != 0;
-}
-
-static void ftgmac100_txdes_set_last_segment(struct ftgmac100_txdes *txdes)
-{
- txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_LTS);
-}
-
-static inline bool ftgmac100_txdes_get_last_segment(struct ftgmac100_txdes
*txdes)
-{
- return (txdes->txdes0 & cpu_to_le32(FTGMAC100_TXDES0_LTS)) != 0;
-}
-
-static void ftgmac100_txdes_set_buffer_size(struct ftgmac100_txdes *txdes,
- unsigned int len)
-{
- txdes->txdes0 |= cpu_to_le32(FTGMAC100_TXDES0_TXBUF_SIZE(len));
-}
-
-static inline unsigned int ftgmac100_txdes_get_buffer_size(struct
ftgmac100_txdes *txdes)
-{
- return FTGMAC100_TXDES0_TXBUF_SIZE(cpu_to_le32(txdes->txdes0));
-}
-
-static void ftgmac100_txdes_set_tcpcs(struct ftgmac100_txdes *txdes)
-{
- txdes->txdes1 |= cpu_to_le32(FTGMAC100_TXDES1_TCP_CHKSUM);
-}
-
-static void ftgmac100_txdes_set_udpcs(struct ftgmac100_txdes *txdes)
-{
- txdes->txdes1 |= cpu_to_le32(FTGMAC100_TXDES1_UDP_CHKSUM);
-}
-
-static void ftgmac100_txdes_set_ipcs(struct ftgmac100_txdes *txdes)
-{
- txdes->txdes1 |= cpu_to_le32(FTGMAC100_TXDES1_IP_CHKSUM);
-}
-
-static void ftgmac100_txdes_set_dma_addr(struct ftgmac100_txdes *txdes,
-dma_addr_t addr)
-{
- txdes->txdes3 = cpu_to_le32(addr);
-}
-
-static dma_addr_t ftgmac100_txdes_get_dma_addr(struct ftgmac100_txdes *txdes)
-{
- return (dma_addr_t)le32_to_cpu(txdes->txdes3);
+ if (index == (TX_QUEUE_ENTRIES - 1))
+ return priv->txdes0_edotr_mask;
+ else
+ return 0;
}
static int ftgmac100_next_tx_pointer(int pointer)
@@ -564,29 +500,24 @@ static bool ftgmac100_tx_buf_cleanable(struct ftgmac100
*priv)
static void ftgmac100_free_tx_packet(struct ftgmac100 *priv,
unsigned int pointer,
struct sk_buff *skb,
-struct ftgmac100_txdes *txdes)
+struct ftgmac100_txdes *txdes,
+u32 ctl_stat)
{
- dma_addr_t map = ftgmac100_txdes_get_dma_addr(txdes);
+ dma_addr_t map = le32_to_cpu(txdes->txdes3);
+ size_t len;
- if (ftgmac100_txdes_get_first_segment(txdes)) {
- dma_unmap_single(priv->dev, map, skb_headlen(skb),
-DMA_TO_DEVICE);
+ if (ctl_stat & FTGMAC100_TXDES0_FTS) {
+ len = skb_headlen(skb);
+ dma_unmap_single(priv->dev, map, len, DMA_TO_DEVICE);
} else {
- dma_unmap_page(priv->dev, map,
- ftgmac100_txdes_get_buffer_size(txdes),
- DMA_TO_DEVICE);
+ len = FTGMAC100_TXDES0_TXBUF_SIZE(ctl_stat);
+ dma_unmap_page(priv->dev, map, len, DMA_TO_DEVICE);
}
- if (ftgmac100_txdes_get_last_segment(txdes))
+ /* Free SKB on last segment */
+ if (ctl_stat & FTGMAC100_TXDES0_LTS)
dev_kfree_skb(skb);
priv->tx_skbs[pointer] = NULL;
-
- /* Clear txdes0 except end of ring bit, clear txdes1 as we
-* only "OR" into it, leave 2 and 3 alone as 2 is unused
-
[PATCH v2 09/12] ftgmac100: Split tx packet freeing from ftgmac100_tx_complete_packet()
This moves the packet freeing to a separate function
which is also used by ftgmac100_free_buffers() and will
be used more in the error path of fragmented sends.
Signed-off-by: Benjamin Herrenschmidt
---
drivers/net/ethernet/faraday/ftgmac100.c | 38 ++--
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index bdec14f..b33de5c 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -561,13 +561,29 @@ static bool ftgmac100_tx_buf_cleanable(struct ftgmac100
*priv)
return priv->tx_pointer != priv->tx_clean_pointer;
}
+static void ftgmac100_free_tx_packet(struct ftgmac100 *priv,
+unsigned int pointer,
+struct sk_buff *skb,
+struct ftgmac100_txdes *txdes)
+{
+ dma_addr_t map;
+
+ map = ftgmac100_txdes_get_dma_addr(txdes);
+
+ dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
+
+ dev_kfree_skb(skb);
+ priv->tx_skbs[pointer] = NULL;
+
+ ftgmac100_txdes_reset(priv, txdes);
+}
+
static bool ftgmac100_tx_complete_packet(struct ftgmac100 *priv)
{
struct net_device *netdev = priv->netdev;
struct ftgmac100_txdes *txdes;
- unsigned int pointer;
struct sk_buff *skb;
- dma_addr_t map;
+ unsigned int pointer;
pointer = priv->tx_clean_pointer;
txdes = &priv->descs->txdes[pointer];
@@ -576,17 +592,9 @@ static bool ftgmac100_tx_complete_packet(struct ftgmac100
*priv)
return false;
skb = priv->tx_skbs[pointer];
- map = ftgmac100_txdes_get_dma_addr(txdes);
-
netdev->stats.tx_packets++;
netdev->stats.tx_bytes += skb->len;
-
- dma_unmap_single(priv->dev, map, skb_headlen(skb), DMA_TO_DEVICE);
-
- dev_kfree_skb(skb);
- priv->tx_skbs[pointer] = NULL;
-
- ftgmac100_txdes_reset(priv, txdes);
+ ftgmac100_free_tx_packet(priv, pointer, skb, txdes);
priv->tx_clean_pointer = ftgmac100_next_tx_pointer(pointer);
@@ -727,13 +735,9 @@ static void ftgmac100_free_buffers(struct ftgmac100 *priv)
for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
struct ftgmac100_txdes *txdes = &priv->descs->txdes[i];
struct sk_buff *skb = priv->tx_skbs[i];
- dma_addr_t map = ftgmac100_txdes_get_dma_addr(txdes);
-
- if (!skb)
- continue;
- dma_unmap_single(priv->dev, map, skb_headlen(skb),
DMA_TO_DEVICE);
- kfree_skb(skb);
+ if (skb)
+ ftgmac100_free_tx_packet(priv, i, skb, txdes);
}
}
--
2.9.3
[PATCH v2 05/12] ftgmac100: Pad small frames properly
Rather than just transmitting garbage past the end of the small
packet.
Signed-off-by: Benjamin Herrenschmidt
--
v2. Use eth_skb_pad (wrapper around skb_put_padto)
---
drivers/net/ethernet/faraday/ftgmac100.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 84ae800..739916d 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -630,11 +630,17 @@ static void ftgmac100_tx_complete(struct ftgmac100 *priv)
static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
struct net_device *netdev)
{
- unsigned int len = (skb->len < ETH_ZLEN) ? ETH_ZLEN : skb->len;
struct ftgmac100 *priv = netdev_priv(netdev);
struct ftgmac100_txdes *txdes;
dma_addr_t map;
+ /* The HW doesn't pad small frames */
+ if (eth_skb_pad(skb)) {
+ netdev->stats.tx_dropped++;
+ return NETDEV_TX_OK;
+ }
+
+ /* Reject oversize packets */
if (unlikely(skb->len > MAX_PKT_SIZE)) {
if (net_ratelimit())
netdev_dbg(netdev, "tx packet too big\n");
@@ -655,7 +661,7 @@ static int ftgmac100_hard_start_xmit(struct sk_buff *skb,
/* setup TX descriptor */
ftgmac100_txdes_set_skb(txdes, skb);
ftgmac100_txdes_set_dma_addr(txdes, map);
- ftgmac100_txdes_set_buffer_size(txdes, len);
+ ftgmac100_txdes_set_buffer_size(txdes, skb->len);
ftgmac100_txdes_set_first_segment(txdes);
ftgmac100_txdes_set_last_segment(txdes);
--
2.9.3
[PATCH v2 01/12] ftgmac100: Add a tx timeout handler
We have a reset task to reset our chip, use it.
Signed-off-by: Benjamin Herrenschmidt
---
drivers/net/ethernet/faraday/ftgmac100.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
b/drivers/net/ethernet/faraday/ftgmac100.c
index 9f18e5e..f4b719214 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1248,6 +1248,17 @@ static int ftgmac100_do_ioctl(struct net_device *netdev,
struct ifreq *ifr, int
return phy_mii_ioctl(netdev->phydev, ifr, cmd);
}
+static void ftgmac100_tx_timeout(struct net_device *netdev)
+{
+ struct ftgmac100 *priv = netdev_priv(netdev);
+
+ /* Disable all interrupts */
+ iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+
+ /* Do the reset outside of interrupt context */
+ schedule_work(&priv->reset_task);
+}
+
static const struct net_device_ops ftgmac100_netdev_ops = {
.ndo_open = ftgmac100_open,
.ndo_stop = ftgmac100_stop,
@@ -1255,6 +1266,7 @@ static const struct net_device_ops ftgmac100_netdev_ops =
{
.ndo_set_mac_address= ftgmac100_set_mac_addr,
.ndo_validate_addr = eth_validate_addr,
.ndo_do_ioctl = ftgmac100_do_ioctl,
+ .ndo_tx_timeout = ftgmac100_tx_timeout,
};
static int ftgmac100_setup_mdio(struct net_device *netdev)
@@ -1359,6 +1371,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
netdev->ethtool_ops = &ftgmac100_ethtool_ops;
netdev->netdev_ops = &ftgmac100_netdev_ops;
+ netdev->watchdog_timeo = 5 * HZ;
platform_set_drvdata(pdev, netdev);
--
2.9.3
[PATCH v2 00/12] ftgmac100: Rework batch 3 - TX path
This is version 2 of the third batch of updates to the ftgmac100 driver. This one tackles the TX path of the driver. This provides the bulk of the performance improvements by adding support for fragmented sends along with a bunch of cleanups. Version 2 fixes a patch splitting mistake and uses eth_skb_pad() (which uses skb_put_padto) to pad ethernet frames rather than skb_padto(), thus removing the need to also pad the packet headlen in a couple of places. Subsequent batches will add various features (ethtool functions, vlan offlan, ...) and cleanups.
Re: pull-request: wireless-drivers-next 2017-04-07
From: Kalle Valo Date: Fri, 07 Apr 2017 17:36:39 +0300 > here's a pull request for net-next, more info in the signed tag below. > Please let me know if there are any problems. Pulled, thanks Kalle.
Re: [PATCH v3 iproute] ip: Add support for netdev events to monitor
From: David Ahern Date: Sat, 8 Apr 2017 22:54:07 -0400 > On 4/8/17 10:33 PM, David Miller wrote: >> From: David Ahern >> Date: Sat, 8 Apr 2017 18:24:06 -0400 >> >>> per comments on the email thread about reducing notifications, the >>> kernel patch for this should be reverted (and hence this iproute2 patch >>> is not needed) in favor of using a bitmask. Right now there are too many >>> redundant notifications to userspace. >> >> I must have missed something in all the discussion, which patch needs >> to be reverted from my tree exactly? >> > > Here's the thread: > https://www.spinics.net/lists/netdev/msg429154.html > > The comment is that def12888c161 is adding a uapi that leads to way too > many notifications (e.g., on a setlink). > > It would be more efficient (read less notifications) to have do_setlink > emit a single message with the IFLA_EVENT (or something else > appropriately named) that indicates what attributes changed. Right now, > a change MTU leads to 3 notifications causing unnecessary churn in > userspace to track what the state of the link is. Ok, I'll queue up the revert. I guess this means your rtnetlink patch set is going to need changes or a respin, therefore.
Re: [PATCH net-next 1/7] ibmvnic: Add is_up flag to avoid transmits when driver is down
From: Nathan Fontenot Date: Sun, 09 Apr 2017 00:11:32 -0400 > From: Thomas Falcon > > There are brief windows when handling events such as failover where we > could attempt to transmit packets between receiving the transport event > notification and handling the reset in the workqueue. > > This patch introduces an is_up flag so we can avoid transmit attempts at > these times. > > Signed-off-by: Thomas Falcon > Signed-off-by: Nathan Fontenot Simply stop the netdev queue when the device is in this state. The kernel will stop queueing packets to the device if you do that. Please do not add unnecessary extra state and logic for this, it doesn't appear to be necessary. Thank you.
Re: [net-next 00/12][pull request] 40GbE Intel Wired LAN Driver Updates 2017-04-08
From: Jeff Kirsher Date: Sat, 8 Apr 2017 02:55:41 -0700 > This series contains updates to i40e and i40evf only. Pulled, thanks Jeff.
[PATCH v2 net-next RFC] Generic XDP
This provides a generic non-optimized XDP implementation when the
device driver does not provide an optimized one.
It is arguable that perhaps I should have required something like
this as part of the initial XDP feature merge.
I believe this is critical for two reasons:
1) Accessibility. More people can play with XDP with less
dependencies. Yes I know we have XDP support in virtio_net, but
that just creates another depedency for learning how to use this
facility.
I wrote this to make life easier for the XDP newbies.
2) As a model for what the expected semantics are. If there is a pure
generic core implementation, it serves as a semantic example for
driver folks adding XDP support.
This is just a rough draft and is untested.
Signed-off-by: David S. Miller
---
v2:
- Add some "fall through" comments in switch statements based
upon feedback from Andrew Lunn
- Use RCU for generic xdp_prog, thanks to Johannes Berg.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cc07c3b..f8ff49c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1892,6 +1892,7 @@ struct net_device {
struct lock_class_key *qdisc_tx_busylock;
struct lock_class_key *qdisc_running_key;
boolproto_down;
+ struct bpf_prog __rcu *xdp_prog;
};
#define to_net_dev(d) container_of(d, struct net_device, dev)
diff --git a/net/core/dev.c b/net/core/dev.c
index 7efb417..ad6de2d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -95,6 +95,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -4247,6 +4248,83 @@ static int __netif_receive_skb(struct sk_buff *skb)
return ret;
}
+static struct static_key generic_xdp_needed __read_mostly;
+
+static int generic_xdp_install(struct net_device *dev, struct netdev_xdp *xdp)
+{
+ struct bpf_prog *new = xdp->prog;
+ int ret = 0;
+
+ switch (xdp->command) {
+ case XDP_SETUP_PROG: {
+ struct bpf_prog *old = rtnl_dereference(dev->xdp_prog);
+
+ rcu_assign_pointer(dev->xdp_prog, new);
+ if (old)
+ bpf_prog_put(old);
+
+ if (old && !new)
+ static_key_slow_dec(&generic_xdp_needed);
+ else if (new && !old)
+ static_key_slow_inc(&generic_xdp_needed);
+ break;
+ }
+
+ case XDP_QUERY_PROG:
+ xdp->prog_attached = !!rcu_access_pointer(dev->xdp_prog);
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static u32 netif_receive_generic_xdp(struct sk_buff *skb,
+struct bpf_prog *xdp_prog)
+{
+ struct xdp_buff xdp;
+ u32 act = XDP_DROP;
+ void *orig_data;
+ int hlen, off;
+
+ if (skb_linearize(skb))
+ goto do_drop;
+
+ hlen = skb_headlen(skb);
+ xdp.data = skb->data;
+ xdp.data_end = xdp.data + hlen;
+ xdp.data_hard_start = xdp.data - skb_headroom(skb);
+ orig_data = xdp.data;
+
+ act = bpf_prog_run_xdp(xdp_prog, &xdp);
+
+ off = xdp.data - orig_data;
+ if (off)
+ __skb_push(skb, off);
+
+ switch (act) {
+ case XDP_PASS:
+ case XDP_TX:
+ break;
+
+ default:
+ bpf_warn_invalid_xdp_action(act);
+ /* fall through */
+ case XDP_ABORTED:
+ trace_xdp_exception(skb->dev, xdp_prog, act);
+ /* fall through */
+ case XDP_DROP:
+ do_drop:
+ kfree_skb(skb);
+ break;
+ }
+
+ return act;
+}
+
static int netif_receive_skb_internal(struct sk_buff *skb)
{
int ret;
@@ -4258,6 +4336,21 @@ static int netif_receive_skb_internal(struct sk_buff
*skb)
rcu_read_lock();
+ if (static_key_false(&generic_xdp_needed)) {
+ struct bpf_prog *xdp_prog = rcu_dereference(skb->dev->xdp_prog);
+
+ if (xdp_prog) {
+ u32 act = netif_receive_generic_xdp(skb, xdp_prog);
+
+ if (act != XDP_PASS) {
+ rcu_read_unlock();
+ if (act == XDP_TX)
+ dev_queue_xmit(skb);
+ return NET_RX_DROP;
+ }
+ }
+ }
+
#ifdef CONFIG_RPS
if (static_key_false(&rps_needed)) {
struct rps_dev_flow voidflow, *rflow = &voidflow;
@@ -6718,6 +6811,7 @@ EXPORT_SYMBOL(dev_change_proto_down);
*/
int dev_change_xdp_fd(struct net_device *dev, int fd, u32 flags)
{
+ int (*xdp_op)(struct net_device *dev, struct netdev_xdp *xdp);
const struct net_device_ops *ops = dev->netdev_ops;
struct bpf_prog *prog = NULL;
struct netdev_xdp xdp;
@@ -6725,14
Re: [PATCH net-next RFC] Generic XDP
From: Johannes Berg Date: Sun, 09 Apr 2017 09:04:07 +0200 > On Sun, 2017-04-09 at 08:25 +0200, Johannes Berg wrote: >> That would also let you use rcu_assign_pointer() which seems like the >> right thing to do here, along with marking the xdp_prog pointer as >> __rcu? That'd also let you use rcu_dereference() instead of >> READ_ONCE() which seems like the better annotation? > > Looks like drivers do it exactly this way too though. Every driver does things differently. For example bnxt_en (which I largely based my patch upon) uses xchg(), whilst mlx4 uses RCU operations. It just goes to show why it's good to have a common implementation of XDP like this. > What I forgot: I guess we could make drivers use dev->xdp_prog after > this, instead of having their own? Yes, but we have to resolve xchg() vs. RCU in order for that to work. When evaluating this, we have to keep in mind that drivers tend to have an extra pointer to the XDP program in their per-queue datastructures. They do this for the purposes of locality of refernece during packet processing. Instinctively I agree with you that RCU should be the way to go so for now I'll adjust my patch to do things that way. Thanks.
Re: [PATCH net-next RFC] Generic XDP
From: Andy Gospodarek Date: Sun, 9 Apr 2017 01:17:26 -0400 > I should be able to run this on Monday and see how the performance > compares to the driver/native XDP case on some bnxt_en-based hardware. Thanks in advance for testing Andy.
Re: [PATCH net-next RFC] Generic XDP
From: Andrew Lunn
Date: Sun, 9 Apr 2017 15:46:55 +0200
>> +switch (act) {
>> +case XDP_PASS:
>> +case XDP_TX:
>> +break;
>> +
>> +default:
>> +bpf_warn_invalid_xdp_action(act);
>
> Hi David
>
> You might want to put a /* fall through */ comment here, just to
> prevent newbies from submitting patches moving the default clause to
> the end.
Probably need two, ala:
default:
bpf_warn_invalid_xdp_action(act);
/* fall through */
case XDP_ABORTED:
trace_xdp_exception(skb->dev, xdp_prog, act);
/* fall through */
case XDP_DROP:
do_drop:
kfree_skb(skb);
break;
So that's what I've done.
Thanks!
Re: [PATCH 1/5] netlink: extended ACK reporting
On 4/8/17 2:40 PM, Jiri Pirko wrote:
> Sat, Apr 08, 2017 at 08:37:01PM CEST, johan...@sipsolutions.net wrote:
>> On Sat, 2017-04-08 at 20:34 +0200, Jiri Pirko wrote:
>>> nla_total_size(sizeof(u32));
+ if (extack &&
+ (extack->missing_attr || extack-
> bad_attr))
>>>
>>> Attr could be 0, right? I know that on the most of the places 0 is
>>> UNSPEC, but I'm pretty sure not everywhere.
perhaps I misunderstand something, but nla_parse suggests attribute type
can not be 0:
int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
int len, const struct nla_policy *policy,
struct netlink_ext_ack *extack)
{
...
nla_for_each_attr(nla, head, len, rem) {
u16 type = nla_type(nla);
if (type > 0 && type <= maxtype) {
...
tb[type] = (struct nlattr *)nla;
}
}
> Also, could you please attach a patch to iproute2 for example which
> would make use of this. I just want to make sure it clicks.
I have follow on patches to Johannes' set that plumbs extack for
rtnetlink doit function and iproute2. will send later.
Re: [PATCH net-next 0/1 v2] skbuff: Extend gso_type to unsigned int.
> struct skb_shared_info {
> + unsigned short _unused;
> unsigned char nr_frags;
This makes _all_ fields to be accessed with offset, but if you move
padding down, at least ->nr_frags will enjoy clean and simple [R64]
addressing.
On allyesconfig-ish kernel:
before: +542 = 720-178
after: -158 = 53-211 (negative, because 16-bit field became 32-bit)
You may want to add configurable redzone there instead of padding
if that what you want. 2 bytes is nothing.
Re: [PATCH] net: rfkill: gpio: Add OBDA8723 ACPI HID
On 04/09/2017 07:11 AM, Marcel Holtmann wrote:
Hi Hans,
The OBDA8723 ACPI HID is used on quite a few Bay Trail based tablets
for bluetooth rfkill functionality.
Tested-by: russianneuroman...@ya.ru
Signed-off-by: Hans de Goede
---
net/rfkill/rfkill-gpio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 76c01cb..50ca65e 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -163,6 +163,7 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
static const struct acpi_device_id rfkill_acpi_match[] = {
{ "BCM4752", RFKILL_TYPE_GPS },
{ "LNV4752", RFKILL_TYPE_GPS },
+ { "OBDA8723", RFKILL_TYPE_BLUETOOTH },
{ },
};
NAK. We are integrating these with hci_bcm.c or hci_intel.c drivers.
This is for the bluetooth side of the rtl8723bs driver which recently
(yesterday) got merged in into drivers/staging. Which still needs
hciattach from userspace. I completely agree that eventually we should
fix that. In the mean time it would be nice if we could carry this
one line patch to give people using the staging driver working bluetooth.
why are Bluetooth drivers in staging? I objected to them before. The only
reason to have them in staging would be people being to lazy to clean things up.
The Bluetooth driver is not in staging. It is only the wifi part; however,
having a driver for that hardware in the kernel makes it easier for users to
handle the BT part of the device. As Hans stated, the current BT driver runs in
userspace. Until we actually produce hci_rtk, having this one line patch will be
helpful.
Larry
--
If I was stranded on an island and the only way to get off
the island was to make a pretty UI, I’d die there.
Linus Torvalds
Re: [PATCH 06/22] staging: rtl8723bs: Fix various errors in os_dep/ioctl_cfg80211.c
On 04/09/2017 10:28 AM, Bastien Nocera wrote: On Sat, 2017-04-08 at 11:07 -0500, Larry Finger wrote: Smatch lists the following: CHECK drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:470 rtw_cfg80211_ibss_indicate_connect() error: we previously assumed 'scanned' could be null (see line 466) drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:942 rtw_cfg80211_set_encryption() warn: inconsistent indenting drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:955 rtw_cfg80211_set_encryption() error: buffer overflow 'psecuritypriv- dot11DefKey' 4 <= 4 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1017 rtw_cfg80211_set_encryption() error: buffer overflow 'padapter- securitypriv.dot118021XGrpKey' 5 <= 5 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1216 cfg80211_rtw_set_default_key() warn: inconsistent indenting drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2498 rtw_cfg80211_monitor_if_xmit_entry() error: we previously assumed 'skb' could be null (see line 2495) drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2850 cfg80211_rtw_start_ap() warn: if statement not indented drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2860 cfg80211_rtw_start_ap() warn: if statement not indented drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3417 rtw_cfg80211_preinit_wiphy() warn: inconsistent indenting drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3547 rtw_wdev_alloc() info: ignoring unreachable code. The indenting warnings were fixed by simple white space changes. The section where 'scanned' could be null required an immediate exit from the routine at that point. A similar fix was required where 'skb' could be null. The two buffer overflow errors were caused by off-by-one errors. While locating these problems, another one was found in os_dep/ioctl_linux.c. Could you please split those up into patches that fix one kind of problem? Makes it easier to review. These patches were merged earlier today. Thanks for the reviews. Larry
Re: [PATCH 00/22] staging: rtl87232bs: Fix errors and warnings detected by Smatch
On Sat, 2017-04-08 at 11:07 -0500, Larry Finger wrote: > A number of routines have indenting, off by one, and possible usage > while null warnings or errors listed by Smatch. This set of patches > fix all but one of these, and it is in code that will be removed in a > subsequent patch. > > Signed-off-by: Larry Finger Feel free to add: Reviewed-by: Bastien Nocera for all the patches I didn't directly comment on. Cheers
Re: [PATCH 20/22] staging rtl8723bs: Fix indenting errors and an off-by-one mistake in core/rtw_mlme_ext.c
On Sat, 2017-04-08 at 11:07 -0500, Larry Finger wrote:
> } else {
> - for (pstat->aid = 1; pstat->aid <= NUM_STA; pstat->aid++)
> + for (pstat->aid = 1; pstat->aid < NUM_STA; pstat->aid++)
> if (pstapriv->sta_aid[pstat->aid - 1] == NULL)
> break;
why not start at 0 and increment pstat->aid afterwards? Meh.
Re: [PATCH 16/22] staging: rtl8723bs: Fix some indenting problems and a potential data overrun
On Sat, 2017-04-08 at 11:07 -0500, Larry Finger wrote: > + if (cam_id >= 0 && cam_id < 32) Isn't there a constant we could use instead of hard-coding this?
Re: [PATCH 13/22] staging: rtl8723bs: Fix indenting mistake in core/rtw_ap.c
On Sat, 2017-04-08 at 11:07 -0500, Larry Finger wrote: > Fixing this requires changing the indentatikon of a long for loop. Typo here.
Re: [PATCH 06/22] staging: rtl8723bs: Fix various errors in os_dep/ioctl_cfg80211.c
On Sat, 2017-04-08 at 11:07 -0500, Larry Finger wrote: > Smatch lists the following: > > CHECK drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:470 > rtw_cfg80211_ibss_indicate_connect() error: we previously assumed > 'scanned' could be null (see line 466) > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:942 > rtw_cfg80211_set_encryption() warn: inconsistent indenting > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:955 > rtw_cfg80211_set_encryption() error: buffer overflow 'psecuritypriv- > >dot11DefKey' 4 <= 4 > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1017 > rtw_cfg80211_set_encryption() error: buffer overflow 'padapter- > >securitypriv.dot118021XGrpKey' 5 <= 5 > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1216 > cfg80211_rtw_set_default_key() warn: inconsistent indenting > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2498 > rtw_cfg80211_monitor_if_xmit_entry() error: we previously assumed > 'skb' could be null (see line 2495) > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2850 > cfg80211_rtw_start_ap() warn: if statement not indented > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2860 > cfg80211_rtw_start_ap() warn: if statement not indented > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3417 > rtw_cfg80211_preinit_wiphy() warn: inconsistent indenting > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3547 > rtw_wdev_alloc() info: ignoring unreachable code. > > The indenting warnings were fixed by simple white space changes. > > The section where 'scanned' could be null required an immediate exit > from > the routine at that point. A similar fix was required where 'skb' > could be null. > > The two buffer overflow errors were caused by off-by-one errors. > While > locating these problems, another one was found in > os_dep/ioctl_linux.c. Could you please split those up into patches that fix one kind of problem? Makes it easier to review.
Re: [PATCH net-next RFC] Generic XDP
> + switch (act) {
> + case XDP_PASS:
> + case XDP_TX:
> + break;
> +
> + default:
> + bpf_warn_invalid_xdp_action(act);
Hi David
You might want to put a /* fall through */ comment here, just to
prevent newbies from submitting patches moving the default clause to
the end.
Andrew
> + case XDP_ABORTED:
> + trace_xdp_exception(skb->dev, xdp_prog, act);
> + case XDP_DROP:
> + do_drop:
> + kfree_skb(skb);
> + break;
> + }
> +
> + return act;
> +}
[PATCH net 1/1] net: tcp: Increase TCPABORTONLINGER when send RST by linger2 in keepalive timer
From: Gao Feng
It should increase TCPABORTONLINGER counter when send RST caused by
linger2 in keepalive timer.
Signed-off-by: Gao Feng
---
net/ipv4/tcp_timer.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index b2ab411..5c01f21 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -650,6 +650,8 @@ static void tcp_keepalive_timer (unsigned long data)
tcp_time_wait(sk, TCP_FIN_WAIT2, tmo);
goto out;
}
+ } else {
+ NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONLINGER);
}
tcp_send_active_reset(sk, GFP_ATOMIC);
goto death;
--
1.9.1
Re: [PATCH] net: rfkill: gpio: Add OBDA8723 ACPI HID
Hi Hans,
>>> The OBDA8723 ACPI HID is used on quite a few Bay Trail based tablets
>>> for bluetooth rfkill functionality.
>>>
>>> Tested-by: russianneuroman...@ya.ru
>>> Signed-off-by: Hans de Goede
>>> ---
>>> net/rfkill/rfkill-gpio.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
>>> index 76c01cb..50ca65e 100644
>>> --- a/net/rfkill/rfkill-gpio.c
>>> +++ b/net/rfkill/rfkill-gpio.c
>>> @@ -163,6 +163,7 @@ static int rfkill_gpio_remove(struct platform_device
>>> *pdev)
>>> static const struct acpi_device_id rfkill_acpi_match[] = {
>>> { "BCM4752", RFKILL_TYPE_GPS },
>>> { "LNV4752", RFKILL_TYPE_GPS },
>>> + { "OBDA8723", RFKILL_TYPE_BLUETOOTH },
>>> { },
>>> };
>>
>> NAK. We are integrating these with hci_bcm.c or hci_intel.c drivers.
>
> This is for the bluetooth side of the rtl8723bs driver which recently
> (yesterday) got merged in into drivers/staging. Which still needs
> hciattach from userspace. I completely agree that eventually we should
> fix that. In the mean time it would be nice if we could carry this
> one line patch to give people using the staging driver working bluetooth.
why are Bluetooth drivers in staging? I objected to them before. The only
reason to have them in staging would be people being to lazy to clean things up.
Regards
Marcel
Re: [PATCH] net: netfilter: Replace explicit NULL comparisons
2017-04-09 16:26 GMT+08:00 Jan Engelhardt : > > On Sunday 2017-04-09 05:42, Arushi Singhal wrote: >>On Sun, Apr 9, 2017 at 1:44 AM, Pablo Neira Ayuso wrote: >> On Sat, Apr 08, 2017 at 08:21:56PM +0200, Jan Engelhardt wrote: >> > On Saturday 2017-04-08 19:21, Arushi Singhal wrote: >> > >> > >Replace explicit NULL comparison with ! operator to simplify code. >> > >> > I still wouldn't do this, for the same reason as before. Comparing to >> > NULL explicitly more or less gave an extra guarantee that the other >> > operand was also a pointer. >> >> Arushi, where does it say in the coding style that this is prefered? >> >>This is reported by checkpatch.pl script. > > checkpatch has been controversial at times, like when people took the 80 > character limit way too literally. Changing pointer comparisons looks like > another thing that is better left ignored. Yes, I agree too. Converting the "if (p != NULL)" to "if (p)" like this seems unnecessary.
Re: [PATCH] net: rfkill: gpio: Add OBDA8723 ACPI HID
Hi,
On 09-04-17 10:01, Marcel Holtmann wrote:
Hi Hans,
The OBDA8723 ACPI HID is used on quite a few Bay Trail based tablets
for bluetooth rfkill functionality.
Tested-by: russianneuroman...@ya.ru
Signed-off-by: Hans de Goede
---
net/rfkill/rfkill-gpio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 76c01cb..50ca65e 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -163,6 +163,7 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
static const struct acpi_device_id rfkill_acpi_match[] = {
{ "BCM4752", RFKILL_TYPE_GPS },
{ "LNV4752", RFKILL_TYPE_GPS },
+ { "OBDA8723", RFKILL_TYPE_BLUETOOTH },
{ },
};
NAK. We are integrating these with hci_bcm.c or hci_intel.c drivers.
This is for the bluetooth side of the rtl8723bs driver which recently
(yesterday) got merged in into drivers/staging. Which still needs
hciattach from userspace. I completely agree that eventually we should
fix that. In the mean time it would be nice if we could carry this
one line patch to give people using the staging driver working bluetooth.
Regards,
Hans
Re: [PATCH] net: netfilter: Replace explicit NULL comparisons
On Sunday 2017-04-09 05:42, Arushi Singhal wrote: >On Sun, Apr 9, 2017 at 1:44 AM, Pablo Neira Ayuso wrote: > On Sat, Apr 08, 2017 at 08:21:56PM +0200, Jan Engelhardt wrote: > > On Saturday 2017-04-08 19:21, Arushi Singhal wrote: > > > > >Replace explicit NULL comparison with ! operator to simplify code. > > > > I still wouldn't do this, for the same reason as before. Comparing to > > NULL explicitly more or less gave an extra guarantee that the other > > operand was also a pointer. > > Arushi, where does it say in the coding style that this is prefered? > >This is reported by checkpatch.pl script. checkpatch has been controversial at times, like when people took the 80 character limit way too literally. Changing pointer comparisons looks like another thing that is better left ignored.
Re: [PATCH] net: rfkill: gpio: Add OBDA8723 ACPI HID
Hi Hans,
> The OBDA8723 ACPI HID is used on quite a few Bay Trail based tablets
> for bluetooth rfkill functionality.
>
> Tested-by: russianneuroman...@ya.ru
> Signed-off-by: Hans de Goede
> ---
> net/rfkill/rfkill-gpio.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> index 76c01cb..50ca65e 100644
> --- a/net/rfkill/rfkill-gpio.c
> +++ b/net/rfkill/rfkill-gpio.c
> @@ -163,6 +163,7 @@ static int rfkill_gpio_remove(struct platform_device
> *pdev)
> static const struct acpi_device_id rfkill_acpi_match[] = {
> { "BCM4752", RFKILL_TYPE_GPS },
> { "LNV4752", RFKILL_TYPE_GPS },
> + { "OBDA8723", RFKILL_TYPE_BLUETOOTH },
> { },
> };
NAK. We are integrating these with hci_bcm.c or hci_intel.c drivers.
Regards
Marcel
Re: [PATCH net-next RFC] Generic XDP
On Sun, 2017-04-09 at 08:25 +0200, Johannes Berg wrote: > That would also let you use rcu_assign_pointer() which seems like the > right thing to do here, along with marking the xdp_prog pointer as > __rcu? That'd also let you use rcu_dereference() instead of > READ_ONCE() which seems like the better annotation? Looks like drivers do it exactly this way too though. What I forgot: I guess we could make drivers use dev->xdp_prog after this, instead of having their own? johannes
