* Re: [PATCH -next] bridge: fix potential use-after-free when hook returns QUEUE or STOLEN verdict
From: Zefir Kurtisi @ 2016-03-14 15:28 UTC (permalink / raw)
To: Florian Westphal, netdev; +Cc: stephen, Felix Fietkau
In-Reply-To: <1457777682-24689-1-git-send-email-fw@strlen.de>
On 03/12/2016 11:14 AM, Florian Westphal wrote:
> Zefir Kurtisi reported kernel panic with an openwrt specific patch.
> However, it turns out that mainline has a similar bug waiting to happen.
>
> Once NF_HOOK() returns the skb is in undefined state and must not be
> used. Moreover, the okfn must consume the skb to support async
> processing (NF_QUEUE).
>
> Current okfn in this spot doesn't consume it and caller assumes that
> NF_HOOK return value tells us if skb was freed or not, but thats wrong.
>
> It "works" because no in-tree user registers a NFPROTO_BRIDGE hook at
> LOCAL_IN that returns STOLEN or NF_QUEUE verdicts.
>
> Once we add NF_QUEUE support for nftables bridge this will break --
> NF_QUEUE holds the skb for async processing, caller will erronoulsy
> return RX_HANDLER_PASS and on reinject netfilter will access free'd skb.
>
> Fix this by pushing skb up the stack in the okfn instead.
>
> NB: It also seems dubious to use LOCAL_IN while bypassing PRE_ROUTING
> completely in this case but this is how its been forever so it seems
> preferable to not change this.
>
> Cc: Felix Fietkau <nbd@openwrt.org>
> Cc: Zefir Kurtisi <zefir.kurtisi@neratec.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>
Looks good: applying the same fix-pattern to OpenWRT private patches solved the
oops previously observed.
Thanks for the quick resolution.
Tested-by: Zefir Kurtisi <zefir.kurtisi@neratec.com>
^ permalink raw reply
* Re: userns, netns, and quick physical memory consumption by unprivileged user
From: Yuriy M. Kaminskiy @ 2016-03-14 15:12 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, containers
In-Reply-To: <20160314091417.GA11400@dhcp22.suse.cz>
On 03/14/16 12:14 , Michal Hocko wrote:
> On Fri 11-03-16 18:06:59, Yuriy M. Kaminskiy wrote:
> [...]
>> And also tried with memcg:
>> t=/sys/fs/cgroup/memory/test1;mkdir $t;echo 0 >$t/tasks;
>> echo 48M >$t/memory.limit_in_bytes; su testuser [...]
>> and it has not helped at all (rather opposite, it ended up with killed
>> init and kernel panic; well, later is pure (un)luck; but point is, memcg
>> apparently *CANNOT* curb net/ns allocations).
>
> It seems you were using memcg v1 here. This didn't have the kernel
> memory accounting enabled by default. With the v2 you get both user and
Hrr. Indeed. And used (distro) kernel compiled without MEMCG_KMEM, so
this test was useless. (However, as distro kernel lacks MEMCG_KMEM, it
means most users won't be able to use it as well[*], so unpriv userns are
unsafe to use for all of them and should be disabled).
That said, not sure if it would have helped in kernels <= 4.4 (would
those allocation be called in context that allows them to be
accounted by [correct] memcg?), but it looks like with upcoming change to
whitelisting (explicit GPF_ACCOUNT), it won't (as almost nothing in
net/* uses it).
> kernel (well some subset of it) accounting enabled. Whether we account
> also netns related data structures sufficiently is a question. I haven't
Except for conntrack tables, it is not exactly tied to netnes, it's
regular CAP_NET_ADMIN things (routing, addresses, links, iptables, etc
that can be added via netlink messages). Just userns+netns gives right
to tweak with them to regular user.
> checked. But it would be worth trying and fix.
^ permalink raw reply
* [PATCH] fsl/fman: fix dtsec_set_tx_pause_frames
From: igal.liberman @ 2016-03-13 19:14 UTC (permalink / raw)
To: netdev; +Cc: scottwood, madalin.bucur, Igal Liberman
From: Igal Liberman <igal.liberman@freescale.com>
Fix a bug introduced in e06a03b (fsl/fman: fix the pause_time test)
When pause_time is set to '0' - pause frames are disabled and
there's no need to apply dTSEC-A003 Errata workaround.
Signed-off-by: Igal Liberman <igal.liberman@freescale.com>
---
drivers/net/ethernet/freescale/fman/fman_dtsec.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fman/fman_dtsec.c b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
index 7c92eb8..c88918c 100644
--- a/drivers/net/ethernet/freescale/fman/fman_dtsec.c
+++ b/drivers/net/ethernet/freescale/fman/fman_dtsec.c
@@ -932,15 +932,14 @@ int dtsec_set_tx_pause_frames(struct fman_mac *dtsec,
if (!is_init_done(dtsec->dtsec_drv_param))
return -EINVAL;
- /* FM_BAD_TX_TS_IN_B_2_B_ERRATA_DTSEC_A003 Errata workaround */
- if (dtsec->fm_rev_info.major == 2)
- if (pause_time <= 320) {
+ if (pause_time) {
+ /* FM_BAD_TX_TS_IN_B_2_B_ERRATA_DTSEC_A003 Errata workaround */
+ if (dtsec->fm_rev_info.major == 2 && pause_time <= 320) {
pr_warn("pause-time: %d illegal.Should be > 320\n",
pause_time);
return -EINVAL;
}
- if (pause_time) {
ptv = ioread32be(®s->ptv);
ptv &= PTV_PTE_MASK;
ptv |= pause_time & PTV_PT_MASK;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next 1/2] rtnetlink: add new RTM_GETSTATS message to dump link stats
From: Nicolas Dichtel @ 2016-03-14 15:00 UTC (permalink / raw)
To: Roopa Prabhu, netdev; +Cc: jhs, davem
In-Reply-To: <1457834186-45727-2-git-send-email-roopa@cumulusnetworks.com>
Le 13/03/2016 02:56, Roopa Prabhu a écrit :
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This patch adds a new RTM_GETSTATS message to query link stats via netlink
> from the kernel. RTM_NEWLINK also dumps stats today, but RTM_NEWLINK
> returns a lot more than just stats and is expensive in some cases when
> frequent polling for stats from userspace is a common operation.
>
> RTM_GETSTATS is an attempt to provide a light weight netlink message
> to explicity query only link stats from the kernel on an interface.
> The idea is to also keep it extensible so that new kinds of stats can be
> added to it in the future.
>
> This patch adds the following attribute for NETDEV stats:
> struct nla_policy ifla_stats_policy[IFLA_STATS_MAX + 1] = {
> [IFLA_STATS_LINK64] = { .len = sizeof(struct rtnl_link_stats64) },
> };
>
> This patch also allows for af family stats (an example af stats for IPV6
> is available with the second patch in the series).
>
> Like any other rtnetlink message, RTM_GETSTATS can be used to get stats of
> a single interface or all interfaces with NLM_F_DUMP.
>
> Future possible new types of stat attributes:
> - IFLA_MPLS_STATS (nested. for mpls/mdev stats)
> - IFLA_EXTENDED_STATS (nested. extended software netdev stats like bridge,
> vlan, vxlan etc)
> - IFLA_EXTENDED_HW_STATS (nested. extended hardware stats which are
> available via ethtool today)
>
> This patch also declares a filter mask for all stat attributes.
> User has to provide a mask of stats attributes to query. This will be
> specified in a new hdr 'struct if_stats_msg' for stats messages.
>
> Without any attributes in the filter_mask, no stats will be returned.
>
> This patch has been tested with modified iproute2 ifstat.
>
> Suggested-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
[snip]
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 249eef9..0840f3e 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
[snip]
> +enum {
> + IFLA_STATS_UNSPEC,
> + IFLA_STATS_LINK64,
> + IFLA_STATS_INET6,
IFLA_STATS_INET6 is part on patch #2, it's not used in this patch.
> + __IFLA_STATS_MAX,
> +};
> +
> +#define IFLA_STATS_MAX (__IFLA_STATS_MAX - 1)
> +
> +#define IFLA_STATS_FILTER_BIT(ATTR) (1 << (ATTR))
> +
> #endif /* _UAPI_LINUX_IF_LINK_H */
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index ca764b5..2bbb300 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -139,6 +139,13 @@ enum {
> RTM_GETNSID = 90,
> #define RTM_GETNSID RTM_GETNSID
>
> + RTM_NEWSTATS = 92,
> +#define RTM_NEWSTATS RTM_NEWSTATS
> + RTM_DELSTATS = 93,
> +#define RTM_DELSTATS RTM_DELSTATS
RTM_DELSTATS is never used.
> + RTM_GETSTATS = 94,
> +#define RTM_GETSTATS RTM_GETSTATS
> +
> __RTM_MAX,
> #define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
> };
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index d2d9e5e..d1e3d17 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
[snip]
> +static noinline size_t if_nlmsg_stats_size(const struct net_device *dev,
> + u32 filter_mask)
Why are you using the 'noinline' attribute?
> +{
> + size_t size = 0;
> +
> + if (filter_mask & IFLA_STATS_FILTER_BIT(IFLA_STATS_LINK64))
> + size += nla_total_size(sizeof(struct rtnl_link_stats64));
> +
> + size += rtnl_link_get_af_stats_size(dev, filter_mask);
> +
> + return size;
> +}
^ permalink raw reply
* [PATCH] vmxnet3: fix lock imbalance in vmxnet3_tq_xmit()
From: Arnd Bergmann @ 2016-03-14 14:53 UTC (permalink / raw)
To: Shrikrishna Khare, VMware, Inc.
Cc: Arnd Bergmann, David S. Miller, Shreyas Bhatewara, Guolin Yang,
Neil Horman, Alexey Khoroshilov, netdev, linux-kernel
A recent bug fix rearranged the code in vmxnet3_tq_xmit() in a
way that left the error handling for oversized headers unlock
a lock that had not been taken yet. Gcc warns about the incorrect
use of the 'flags' variable because of that:
drivers/net/vmxnet3/vmxnet3_drv.c: In function 'vmxnet3_tq_xmit.constprop':
include/linux/spinlock.h:246:3: error: 'flags' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This changes the error handling path to 'goto' the end of the function
beyond the lock/unlock pair.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: cec05562fb1d ("vmxnet3: avoid calling pskb_may_pull with interrupts disabled")
---
drivers/net/vmxnet3/vmxnet3_drv.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index fc895d0e85d9..b2348f67b00a 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -1022,14 +1022,16 @@ vmxnet3_tq_xmit(struct sk_buff *skb, struct vmxnet3_tx_queue *tq,
if (ctx.mss) {
if (unlikely(ctx.eth_ip_hdr_size + ctx.l4_hdr_size >
VMXNET3_MAX_TX_BUF_SIZE)) {
- goto hdr_too_big;
+ tq->stats.drop_oversized_hdr++;
+ goto drop_pkt;
}
} else {
if (skb->ip_summed == CHECKSUM_PARTIAL) {
if (unlikely(ctx.eth_ip_hdr_size +
skb->csum_offset >
VMXNET3_MAX_CSUM_OFFSET)) {
- goto hdr_too_big;
+ tq->stats.drop_oversized_hdr++;
+ goto drop_pkt;
}
}
}
@@ -1123,8 +1125,6 @@ vmxnet3_tq_xmit(struct sk_buff *skb, struct vmxnet3_tx_queue *tq,
return NETDEV_TX_OK;
-hdr_too_big:
- tq->stats.drop_oversized_hdr++;
unlock_drop_pkt:
spin_unlock_irqrestore(&tq->tx_lock, flags);
drop_pkt:
--
2.7.0
^ permalink raw reply related
* Re: [PATCH net-next 1/2] rtnetlink: add new RTM_GETSTATS message to dump link stats
From: Jiri Pirko @ 2016-03-14 14:51 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: netdev, jhs, davem
In-Reply-To: <1457834186-45727-2-git-send-email-roopa@cumulusnetworks.com>
Sun, Mar 13, 2016 at 02:56:25AM CET, roopa@cumulusnetworks.com wrote:
>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
>This patch adds a new RTM_GETSTATS message to query link stats via netlink
>from the kernel. RTM_NEWLINK also dumps stats today, but RTM_NEWLINK
>returns a lot more than just stats and is expensive in some cases when
>frequent polling for stats from userspace is a common operation.
>
>RTM_GETSTATS is an attempt to provide a light weight netlink message
>to explicity query only link stats from the kernel on an interface.
>The idea is to also keep it extensible so that new kinds of stats can be
>added to it in the future.
>
>This patch adds the following attribute for NETDEV stats:
>struct nla_policy ifla_stats_policy[IFLA_STATS_MAX + 1] = {
> [IFLA_STATS_LINK64] = { .len = sizeof(struct rtnl_link_stats64) },
>};
>
>This patch also allows for af family stats (an example af stats for IPV6
>is available with the second patch in the series).
>
>Like any other rtnetlink message, RTM_GETSTATS can be used to get stats of
>a single interface or all interfaces with NLM_F_DUMP.
>
>Future possible new types of stat attributes:
>- IFLA_MPLS_STATS (nested. for mpls/mdev stats)
>- IFLA_EXTENDED_STATS (nested. extended software netdev stats like bridge,
> vlan, vxlan etc)
>- IFLA_EXTENDED_HW_STATS (nested. extended hardware stats which are
> available via ethtool today)
>
>This patch also declares a filter mask for all stat attributes.
>User has to provide a mask of stats attributes to query. This will be
>specified in a new hdr 'struct if_stats_msg' for stats messages.
>
>Without any attributes in the filter_mask, no stats will be returned.
>
>This patch has been tested with modified iproute2 ifstat.
>
>Suggested-by: Jamal Hadi Salim <jhs@mojatatu.com>
>Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>---
> include/net/rtnetlink.h | 5 ++
> include/uapi/linux/if_link.h | 19 ++++
> include/uapi/linux/rtnetlink.h | 7 ++
> net/core/rtnetlink.c | 200 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 231 insertions(+)
>
>diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
>index 2f87c1b..fa68158 100644
>--- a/include/net/rtnetlink.h
>+++ b/include/net/rtnetlink.h
>@@ -131,6 +131,11 @@ struct rtnl_af_ops {
> const struct nlattr *attr);
> int (*set_link_af)(struct net_device *dev,
> const struct nlattr *attr);
>+ size_t (*get_link_af_stats_size)(const struct net_device *dev,
>+ u32 filter_mask);
>+ int (*fill_link_af_stats)(struct sk_buff *skb,
>+ const struct net_device *dev,
>+ u32 filter_mask);
> };
>
> void __rtnl_af_unregister(struct rtnl_af_ops *ops);
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index 249eef9..0840f3e 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -741,4 +741,23 @@ enum {
>
> #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1)
>
>+/* STATS section */
>+
>+struct if_stats_msg {
>+ __u8 family;
>+ __u32 ifindex;
>+ __u32 filter_mask;
This limit future extension to only 32 groups of stats. I can imagine
that more than that can be added, easily. Why don't you use nested
attribute IFLA_STATS_FILTER with flag attributes for every type? That
would be easily extendable.
Using netlink header struct for this does not look correct to me.
In past, this was done lot of times and turned out to be a problem later.
>+};
>+
>+enum {
>+ IFLA_STATS_UNSPEC,
>+ IFLA_STATS_LINK64,
>+ IFLA_STATS_INET6,
>+ __IFLA_STATS_MAX,
>+};
>+
>+#define IFLA_STATS_MAX (__IFLA_STATS_MAX - 1)
>+
>+#define IFLA_STATS_FILTER_BIT(ATTR) (1 << (ATTR))
>+
^ permalink raw reply
* net: gcc-6.0 warning fixes
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
I've just installed gcc-6.0 to see what kinds of new warnings
we get. It turns out that it's actually really useful once I
disabled -Wunused-const-variable, and all of the warnings it
found in network drivers seem valid.
Sorry for the bad timing in the merge window, but I figured
it would be better to send the fixes as I found the bugs
rather than waiting for the next cycle. The first three
look appropriate for stable backports.
The other two only fix a gcc warning about incorrect whitespace,
probably not worth backporting those.
Arnd
^ permalink raw reply
* [PATCH 5/5] net: caif: fix misleading indentation
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Arnd Bergmann, Dmitry Tarnyagin, linux-kernel
In-Reply-To: <1457965120-3155420-1-git-send-email-arnd@arndb.de>
gcc points out code that is not indented the way it is
interpreted:
net/caif/cfpkt_skbuff.c: In function 'cfpkt_setlen':
net/caif/cfpkt_skbuff.c:289:4: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
return cfpkt_getlen(pkt);
^~~~~~
net/caif/cfpkt_skbuff.c:286:3: note: ...this 'else' clause, but it is not
else
^~~~
It is clear from the context that not returning here would be
a bug, as we'd end up passing a negative length into a function
that takes a u16 length, so it is not missing curly braces
here, and I'm assuming that the indentation is the only part
that's wrong about it.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/caif/cfpkt_skbuff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c
index f6c3b2137eea..59ce1fcc220c 100644
--- a/net/caif/cfpkt_skbuff.c
+++ b/net/caif/cfpkt_skbuff.c
@@ -286,7 +286,7 @@ int cfpkt_setlen(struct cfpkt *pkt, u16 len)
else
skb_trim(skb, len);
- return cfpkt_getlen(pkt);
+ return cfpkt_getlen(pkt);
}
/* Need to expand SKB */
--
2.7.0
^ permalink raw reply related
* [PATCH 4/5] ath9k: fix misleading indentation
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Arnd Bergmann, QCA ath9k Development, Kalle Valo,
linux-wireless, ath9k-devel, linux-kernel
In-Reply-To: <1457965120-3155420-1-git-send-email-arnd@arndb.de>
A cleanup patch in linux-3.18 moved around some code in the ath9k
driver and left some code to be indented in a misleading way,
made worse by the addition of some new code for p2p mode, as
discovered by a new gcc-6 warning:
drivers/net/wireless/ath/ath9k/init.c: In function 'ath9k_set_hw_capab':
drivers/net/wireless/ath/ath9k/init.c:851:4: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
hw->wiphy->iface_combinations = if_comb;
^~
drivers/net/wireless/ath/ath9k/init.c:847:3: note: ...this 'if' clause, but it is not
if (ath9k_is_chanctx_enabled())
^~
The code is in fact correct, but the indentation is not, so I'm
reformatting it as it should have been after the original cleanup.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 499afaccf6f3 ("ath9k: Isolate ath9k_use_chanctx module parameter")
Fixes: eb61f9f623f7 ("ath9k: advertise p2p dev support when chanctx")
---
drivers/net/wireless/ath/ath9k/init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index d4e0ac946c3a..1c226d63bb03 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -848,8 +848,8 @@ static void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
hw->wiphy->interface_modes |=
BIT(NL80211_IFTYPE_P2P_DEVICE);
- hw->wiphy->iface_combinations = if_comb;
- hw->wiphy->n_iface_combinations = ARRAY_SIZE(if_comb);
+ hw->wiphy->iface_combinations = if_comb;
+ hw->wiphy->n_iface_combinations = ARRAY_SIZE(if_comb);
}
hw->wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;
--
2.7.0
^ permalink raw reply related
* [PATCH 3/5] ath9k: fix buffer overrun for ar9287
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, netdev, linux-wireless, QCA ath9k Development,
linux-kernel, ath9k-devel, Kalle Valo
In-Reply-To: <1457965120-3155420-1-git-send-email-arnd@arndb.de>
Code that was added back in 2.6.38 has an obvious overflow
when accessing a static array, and at the time it was added
only a code comment was put in front of it as a reminder
to have it reviewed properly.
This has not happened, but gcc-6 now points to the specific
overflow:
drivers/net/wireless/ath/ath9k/eeprom.c: In function 'ath9k_hw_get_gain_boundaries_pdadcs':
drivers/net/wireless/ath/ath9k/eeprom.c:483:44: error: array subscript is above array bounds [-Werror=array-bounds]
maxPwrT4[i] = data_9287[idxL].pwrPdg[i][4];
~~~~~~~~~~~~~~~~~~~~~~~~~^~~
It turns out that the correct array length exists in the local
'intercepts' variable of this function, so we can just use that
instead of hardcoding '4', so this patch changes all three
instances to use that variable. The other two instances were
already correct, but it's more consistent this way.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 940cd2c12ebf ("ath9k_hw: merge the ar9287 version of ath9k_hw_get_gain_boundaries_pdadcs")
---
drivers/net/wireless/ath/ath9k/eeprom.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
index 73fb4232f9f2..a794157a147d 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom.c
@@ -477,10 +477,9 @@ void ath9k_hw_get_gain_boundaries_pdadcs(struct ath_hw *ah,
if (match) {
if (AR_SREV_9287(ah)) {
- /* FIXME: array overrun? */
for (i = 0; i < numXpdGains; i++) {
minPwrT4[i] = data_9287[idxL].pwrPdg[i][0];
- maxPwrT4[i] = data_9287[idxL].pwrPdg[i][4];
+ maxPwrT4[i] = data_9287[idxL].pwrPdg[i][intercepts - 1];
ath9k_hw_fill_vpd_table(minPwrT4[i], maxPwrT4[i],
data_9287[idxL].pwrPdg[i],
data_9287[idxL].vpdPdg[i],
@@ -490,7 +489,7 @@ void ath9k_hw_get_gain_boundaries_pdadcs(struct ath_hw *ah,
} else if (eeprom_4k) {
for (i = 0; i < numXpdGains; i++) {
minPwrT4[i] = data_4k[idxL].pwrPdg[i][0];
- maxPwrT4[i] = data_4k[idxL].pwrPdg[i][4];
+ maxPwrT4[i] = data_4k[idxL].pwrPdg[i][intercepts - 1];
ath9k_hw_fill_vpd_table(minPwrT4[i], maxPwrT4[i],
data_4k[idxL].pwrPdg[i],
data_4k[idxL].vpdPdg[i],
@@ -500,7 +499,7 @@ void ath9k_hw_get_gain_boundaries_pdadcs(struct ath_hw *ah,
} else {
for (i = 0; i < numXpdGains; i++) {
minPwrT4[i] = data_def[idxL].pwrPdg[i][0];
- maxPwrT4[i] = data_def[idxL].pwrPdg[i][4];
+ maxPwrT4[i] = data_def[idxL].pwrPdg[i][intercepts - 1];
ath9k_hw_fill_vpd_table(minPwrT4[i], maxPwrT4[i],
data_def[idxL].pwrPdg[i],
data_def[idxL].vpdPdg[i],
--
2.7.0
^ permalink raw reply related
* [PATCH 2/5] farsync: fix off-by-one bug in fst_add_one
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Arnd Bergmann, Kevin Curtis, linux-kernel
In-Reply-To: <1457965120-3155420-1-git-send-email-arnd@arndb.de>
gcc-6 finds an out of bounds access in the fst_add_one function
when calculating the end of the mmio area:
drivers/net/wan/farsync.c: In function 'fst_add_one':
drivers/net/wan/farsync.c:418:53: error: index 2 denotes an offset greater than size of 'u8[2][8192] {aka unsigned char[2][8192]}' [-Werror=array-bounds]
#define BUF_OFFSET(X) (BFM_BASE + offsetof(struct buf_window, X))
^
include/linux/compiler-gcc.h:158:21: note: in definition of macro '__compiler_offsetof'
__builtin_offsetof(a, b)
^
drivers/net/wan/farsync.c:418:37: note: in expansion of macro 'offsetof'
#define BUF_OFFSET(X) (BFM_BASE + offsetof(struct buf_window, X))
^~~~~~~~
drivers/net/wan/farsync.c:2519:36: note: in expansion of macro 'BUF_OFFSET'
+ BUF_OFFSET ( txBuffer[i][NUM_TX_BUFFER][0]);
^~~~~~~~~~
The warning is correct, but not critical because this appears
to be a write-only variable that is set by each WAN driver but
never accessed afterwards.
I'm taking the minimal fix here, using the correct pointer by
pointing 'mem_end' to the last byte inside of the register area
as all other WAN drivers do, rather than the first byte outside of
it. An alternative would be to just remove the mem_end member
entirely.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wan/farsync.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wan/farsync.c b/drivers/net/wan/farsync.c
index 44541dbc5c28..69b994f3b8c5 100644
--- a/drivers/net/wan/farsync.c
+++ b/drivers/net/wan/farsync.c
@@ -2516,7 +2516,7 @@ fst_add_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dev->mem_start = card->phys_mem
+ BUF_OFFSET ( txBuffer[i][0][0]);
dev->mem_end = card->phys_mem
- + BUF_OFFSET ( txBuffer[i][NUM_TX_BUFFER][0]);
+ + BUF_OFFSET ( txBuffer[i][NUM_TX_BUFFER - 1][LEN_RX_BUFFER - 1]);
dev->base_addr = card->pci_conf;
dev->irq = card->irq;
--
2.7.0
^ permalink raw reply related
* [PATCH 1/5] mlx4: add missing braces in verify_qp_parameters
From: Arnd Bergmann @ 2016-03-14 14:18 UTC (permalink / raw)
To: David S. Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, Yishai Hadas,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1457965120-3155420-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
The implementation of QP paravirtualization back in linux-3.7 included
some code that looks very dubious, and gcc-6 has grown smart enough
to warn about it:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function 'verify_qp_parameters':
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:3154:5: error: statement is indented as if it were guarded by... [-Werror=misleading-indentation]
if (optpar & MLX4_QP_OPTPAR_ALT_ADDR_PATH) {
^~
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:3144:4: note: ...this 'if' clause, but it is not
if (slave != mlx4_master_func_num(dev))
>From looking at the context, I'm reasonably sure that the indentation
is correct but that it should have contained curly braces from the
start, as the update_gid() function in the same patch correctly does.
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Fixes: 54679e148287 ("mlx4: Implement QP paravirtualization and maintain phys_pkey_cache for smp_snoop")
---
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index 25ce1b030a00..cd9b2b28df88 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -3141,7 +3141,7 @@ static int verify_qp_parameters(struct mlx4_dev *dev,
case QP_TRANS_RTS2RTS:
case QP_TRANS_SQD2SQD:
case QP_TRANS_SQD2RTS:
- if (slave != mlx4_master_func_num(dev))
+ if (slave != mlx4_master_func_num(dev)) {
if (optpar & MLX4_QP_OPTPAR_PRIMARY_ADDR_PATH) {
port = (qp_ctx->pri_path.sched_queue >> 6 & 1) + 1;
if (dev->caps.port_mask[port] != MLX4_PORT_TYPE_IB)
@@ -3160,6 +3160,7 @@ static int verify_qp_parameters(struct mlx4_dev *dev,
if (qp_ctx->alt_path.mgid_index >= num_gids)
return -EINVAL;
}
+ }
break;
default:
break;
--
2.7.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 related
* Re: [net-next RFC 0/4] SO_BINDTOSUBNET
From: Willem de Bruijn @ 2016-03-14 14:12 UTC (permalink / raw)
To: Gilberto Bertin; +Cc: Tom Herbert, Linux Kernel Network Developers
In-Reply-To: <69BFB2EA-BADE-420C-93E5-1320D53EB2C0@gmail.com>
> - write a bpf filter like this:
..
>
> - compile it:
> $ clang -target bpf -c -o socket_bpf.o socket_bpf.c
>
> - add this to your server.c:
> bpf_load_file("/path/to/socket_bpf.o");
> setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd, sizeof(prog_fd[0]));
>
> - link your server with a couple of libbpf libraries (I'm
> using the kernel ones from samples/bpf) and -lelf
>
> And this is still simplified (since instead of hardcoding the subnet
> into the bpf filter it would be preferable to use maps).
>
>
> thoughts:
> - SO_BINDTOSUBNET is much simpler to configure than BPF
> - BPF requires some external C libraries and I think it would not be
> trivial to get it working with other languages than C/C++.
You can easily write BPF by hand.
See for instance attach_ebpf() in tools/testing/selftests/net/reuseport_bpf.c
^ permalink raw reply
* [PATCH 3/3] net: mediatek: check device_reset return code
From: Arnd Bergmann @ 2016-03-14 14:07 UTC (permalink / raw)
To: David S. Miller
Cc: Arnd Bergmann, Felix Fietkau, John Crispin, Matthias Brugger,
netdev, linux-arm-kernel, linux-mediatek, linux-kernel
In-Reply-To: <1457964435-2945038-1-git-send-email-arnd@arndb.de>
The device_reset() function may fail, so we have to check
its return value, e.g. to make deferred probing work correctly.
gcc warns about it because of the warn_unused_result attribute:
drivers/net/ethernet/mediatek/mtk_eth_soc.c: In function 'mtk_probe':
drivers/net/ethernet/mediatek/mtk_eth_soc.c:1679:2: error: ignoring return value of 'device_reset', declared with attribute warn_unused_result [-Werror=unused-result]
This adds the trivial error check to propagate the return value
to the generic platform device probe code.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 87f417712da0..1e6c5498bba9 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1676,7 +1676,9 @@ static int mtk_probe(struct platform_device *pdev)
struct mtk_eth *eth;
int err;
- device_reset(&pdev->dev);
+ err = device_reset(&pdev->dev);
+ if (err)
+ return err;
match = of_match_device(of_mtk_match, &pdev->dev);
soc = (struct mtk_soc_data *)match->data;
--
2.7.0
^ permalink raw reply related
* [PATCH 2/3] net: mediatek: remove incorrect dma_mask assignment
From: Arnd Bergmann @ 2016-03-14 14:07 UTC (permalink / raw)
To: David S. Miller
Cc: Felix Fietkau, Arnd Bergmann, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, John Crispin
In-Reply-To: <1457964435-2945038-1-git-send-email-arnd-r2nGTMty4D4@public.gmane.org>
Device drivers should not mess with the DMA mask directly,
but instead call dma_set_mask() etc if needed.
In case of the mtk_eth_soc driver, the mask already gets set
correctly when the device is created, and setting it again
is against the documented API.
This removes the incorrect setting.
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 3e42204adfe5..87f417712da0 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1676,9 +1676,6 @@ static int mtk_probe(struct platform_device *pdev)
struct mtk_eth *eth;
int err;
- pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
- pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
-
device_reset(&pdev->dev);
match = of_match_device(of_mtk_match, &pdev->dev);
--
2.7.0
^ permalink raw reply related
* [PATCH 1/3] net: mediatek: use dma_addr_t correctly
From: Arnd Bergmann @ 2016-03-14 14:07 UTC (permalink / raw)
To: David S. Miller
Cc: Felix Fietkau, Arnd Bergmann, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Matthias Brugger,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, John Crispin
dma_alloc_coherent() expects a dma_addr_t pointer as its argument,
not an 'unsigned int', and gcc correctly warns about broken
code in the mtk_init_fq_dma function:
drivers/net/ethernet/mediatek/mtk_eth_soc.c: In function 'mtk_init_fq_dma':
drivers/net/ethernet/mediatek/mtk_eth_soc.c:463:13: error: passing argument 3 of 'dma_alloc_coherent' from incompatible pointer type [-Werror=incompatible-pointer-types]
This changes the type of the local variable to dma_addr_t.
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ba3afa5d4640..3e42204adfe5 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -453,7 +453,7 @@ static inline void mtk_rx_get_desc(struct mtk_rx_dma *rxd,
/* the qdma core needs scratch memory to be setup */
static int mtk_init_fq_dma(struct mtk_eth *eth)
{
- unsigned int phy_ring_head, phy_ring_tail;
+ dma_addr_t phy_ring_head, phy_ring_tail;
int cnt = MTK_DMA_SIZE;
dma_addr_t dma_addr;
int i;
--
2.7.0
^ permalink raw reply related
* [PATCH 1/1] net: Fix use after free in the recvmmsg exit path
From: Arnaldo Carvalho de Melo @ 2016-03-14 12:56 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, linux-kernel, Arnaldo Carvalho de Melo,
Alexander Potapenko, Eric Dumazet, Kostya Serebryany, Sasha Levin
In-Reply-To: <1457960195-4302-1-git-send-email-acme@kernel.org>
From: Arnaldo Carvalho de Melo <acme@redhat.com>
The syzkaller fuzzer hit the following use-after-free:
Call Trace:
[<ffffffff8175ea0e>] __asan_report_load8_noabort+0x3e/0x40 mm/kasan/report.c:295
[<ffffffff851cc31a>] __sys_recvmmsg+0x6fa/0x7f0 net/socket.c:2261
[< inline >] SYSC_recvmmsg net/socket.c:2281
[<ffffffff851cc57f>] SyS_recvmmsg+0x16f/0x180 net/socket.c:2270
[<ffffffff86332bb6>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
And, as Dmitry rightly assessed, that is because we can drop the
reference and then touch it when the underlying recvmsg calls return
some packets and then hit an error, which will make recvmmsg to set
sock->sk->sk_err, oops, fix it.
Reported-and-Tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Sasha Levin <sasha.levin@oracle.com>
Fixes: a2e2725541fa ("net: Introduce recvmmsg socket syscall")
http://lkml.kernel.org/r/20160122211644.GC2470@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
net/socket.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index c044d1e8508c..db13ae893dce 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2240,31 +2240,31 @@ int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
cond_resched();
}
-out_put:
- fput_light(sock->file, fput_needed);
-
if (err == 0)
- return datagrams;
+ goto out_put;
- if (datagrams != 0) {
+ if (datagrams == 0) {
+ datagrams = err;
+ goto out_put;
+ }
+
+ /*
+ * We may return less entries than requested (vlen) if the
+ * sock is non block and there aren't enough datagrams...
+ */
+ if (err != -EAGAIN) {
/*
- * We may return less entries than requested (vlen) if the
- * sock is non block and there aren't enough datagrams...
+ * ... or if recvmsg returns an error after we
+ * received some datagrams, where we record the
+ * error to return on the next call or if the
+ * app asks about it using getsockopt(SO_ERROR).
*/
- if (err != -EAGAIN) {
- /*
- * ... or if recvmsg returns an error after we
- * received some datagrams, where we record the
- * error to return on the next call or if the
- * app asks about it using getsockopt(SO_ERROR).
- */
- sock->sk->sk_err = -err;
- }
-
- return datagrams;
+ sock->sk->sk_err = -err;
}
+out_put:
+ fput_light(sock->file, fput_needed);
- return err;
+ return datagrams;
}
SYSCALL_DEFINE5(recvmmsg, int, fd, struct mmsghdr __user *, mmsg,
--
2.5.0
^ permalink raw reply related
* [PATCH 0/1] recvmmsg use-after-free fix
From: Arnaldo Carvalho de Melo @ 2016-03-14 12:56 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, linux-kernel, Arnaldo Carvalho de Melo,
Alexander Potapenko, Eric Dumazet, Kostya Serebryany, Sasha Levin
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Hi David,
Please consider applying,
- Arnaldo
Arnaldo Carvalho de Melo (1):
net: Fix use after free in the recvmmsg exit path
net/socket.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
--
2.5.0
^ permalink raw reply
* Re: [PATCH 1/2 net v3.16]r8169: Not enable/disable bus mastering when is enabled on BIOS
From: Michal Kubecek @ 2016-03-14 12:22 UTC (permalink / raw)
To: Corcodel Marian; +Cc: netdev, Francois Romieu
In-Reply-To: <1457861569-1844-1-git-send-email-asd@marian1000.go.ro>
On Sun, Mar 13, 2016 at 11:32:48AM +0200, Corcodel Marian wrote:
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 02aec96..ec555e7 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -754,6 +754,7 @@ struct rtl8169_private {
> struct timer_list timer;
> u16 cp_cmd;
> bool pcie;
> + bool bios_support;
You shouldn't base your patches on earlier patches that haven't been
accepted yet (unless they are part of the same series).
Michal Kubecek
^ permalink raw reply
* Re: [PATCH v3 0/8] arm64: rockchip: Initial GeekBox enablement
From: Tomeu Vizoso @ 2016-03-14 11:43 UTC (permalink / raw)
To: Giuseppe CAVALLARO
Cc: Dinh Nguyen, Andreas Färber, Fabrice GASNIER,
devicetree@vger.kernel.org, Heiko Stübner, netdev,
open list:ARM/Rockchip SoC..., LAKML, Gabriel Fernandez,
Alexandre TORGUE, Frank Schäfer, LKML
In-Reply-To: <56E28B5D.6000708@st.com>
On 11 March 2016 at 10:09, Giuseppe CAVALLARO <peppe.cavallaro@st.com> wrote:
> On 3/10/2016 5:47 PM, Dinh Nguyen wrote:
>>
>> On Thu, Mar 10, 2016 at 3:13 AM, Giuseppe CAVALLARO
>> <peppe.cavallaro@st.com> wrote:
>>>
>>> On 3/9/2016 5:31 PM, Dinh Nguyen wrote:
>>>>
>>>>
>>>> On Wed, Mar 9, 2016 at 8:53 AM, Giuseppe CAVALLARO
>>>> <peppe.cavallaro@st.com> wrote:
>>>>>
>>>>>
>>>>> Hi Tomeu, Dinh, Andreas
>>>>>
>>>>> I need a sum and help from you to go ahead on the
>>>>> tx timeout.
>>>>>
>>>>> The "stmmac: MDIO fixes" seems to be the candidate to
>>>>> fix the phy connection and I will send the V2 asap (Andreas' comment).
>>>>>
>>>>> So, supposing the probe is ok and phy is connected,
>>>>> I need your input ...
>>>>>
>>>>> Tomeu: after revering the 0e80bdc9a72d (stmmac: first frame
>>>>> prep at the end of xmit routine) the network is
>>>>> not stable and there is a timeout after a while.
>>>>> The box has 3.50 with normal desc settings.
>>>>>
>>>>> Dinh: the network is ok, I wonder if you can share a boot
>>>>> log just to understand if the normal or enhanced
>>>>> descriptors are used.
>>>>>
>>>>
>>>> Here it is:
>>>
>>>
>>> ...
>>>>
>>>>
>>>> [ 0.850523] stmmac - user ID: 0x10, Synopsys ID: 0x37
>>>> [ 0.855570] Ring mode enabled
>>>> [ 0.858611] DMA HW capability register supported
>>>> [ 0.863128] Enhanced/Alternate descriptors
>>>> [ 0.867482] Enabled extended descriptors
>>>> [ 0.871482] RX Checksum Offload Engine supported (type 2)
>>>> [ 0.876948] TX Checksum insertion supported
>>>> [ 0.881204] Enable RX Mitigation via HW Watchdog Timer
>>>> [ 0.886863] socfpga-dwmac ff702000.ethernet eth0: No MDIO subnode
>>>> found
>>>> [ 0.899090] libphy: stmmac: probed
>>>> [ 0.902484] eth0: PHY ID 00221611 at 4 IRQ POLL (stmmac-0:04) active
>>>
>>>
>>>
>>> Thx Dinh, so you are using the Enhanced/Alternate descriptors
>>> I am debugging on my side on a setup with normal descriptors, I let you
>>> know
>>>
>>
>> Doesn't the printout "Enhanced/Alternate descriptors" mean that I'm using
>> Enhanced/Alternate descriptors?
>
>
> yes this means that you have the Databook 3.70a and, from the HW
> capability register, the driver will use the Enhanced/Alternate
> descriptors. This is the same HW I am using on my side where the
> stmmac is working fine.
>
> In the case where it is failing on net-next, although on Databook 3.50a,
> the HW capability register says that there is no enhanced descriptors
> and the driver uses the normal ones.
>
> Tomeu, I kindly ask you to try the patch attached. I found a bug on Tx
> path for normal descriptors. Please let me know if this help.
> Also let me know if we actually need to revert the 0e80bdc9a72d.
Hi Peppe,
with that patch I don't see any difference at all in my setup.
So to be clear, with these commits on top of next-20160314, I still
get the hang during boot:
209afef6f0cd ARM: dts: rockchip: Add mdio node to ethernet node
2315acc6cf7f Revert "stmmac: first frame prep at the end of xmit routine"
b5e08e810c63 stmmac: fix tx prepare for normal desc
37c15a31d850 i2c: immediately mark ourselves as registered
4342eec3c5a2 Add linux-next specific files for 20160314
[ 27.521026] WARNING: CPU: 1 PID: 0 at net/sched/sch_generic.c:303
dev_watchdog+0x284/0x288
[ 27.529460] NETDEV WATCHDOG: eth0 (rk_gmac-dwmac): transmit queue 0 timed out
https://git.collabora.com/cgit/user/tomeu/linux.git/log/?h=broken-eth-on-rock2
> I am trying to find some HW where test the normal descriptors to
> speed-up the tests on my side directly.
Maybe get your tree in kernelci.org? I'm not sure if it's currently
doing any nfsroot boots, though.
Regards,
Tomeu
> Let me know and thx in advance.
>
> Regards,
> Peppe
>
>>
>> Dinh
>>
>
^ permalink raw reply
* [PATCH] netfilter: nf_conntrack: consolidate lock/unlock into unlock_wait
From: Nicholas Mc Guire @ 2016-03-14 11:39 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
netfilter-devel, coreteam, netdev, linux-kernel,
Nicholas Mc Guire
The spin_lock()/spin_unlock() is synchronizing on the
nf_conntrack_locks_all_lock which is equivalent to
spin_unlock_wait() but the later should be more efficient.
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---
Patch was compile tested with: x86_64_defconfig (implies CONFIG_NETFILTER=y)
Simple run test on x86 64 with a few trivial ipfilter rules active.
Patch is against linux-next (localversion-next is next-20160311)
net/netfilter/nf_conntrack_core.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index f60b4fd..afde5f5 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -74,8 +74,7 @@ void nf_conntrack_lock(spinlock_t *lock) __acquires(lock)
spin_lock(lock);
while (unlikely(nf_conntrack_locks_all)) {
spin_unlock(lock);
- spin_lock(&nf_conntrack_locks_all_lock);
- spin_unlock(&nf_conntrack_locks_all_lock);
+ spin_unlock_wait(&nf_conntrack_locks_all_lock);
spin_lock(lock);
}
}
@@ -121,8 +120,7 @@ static void nf_conntrack_all_lock(void)
nf_conntrack_locks_all = true;
for (i = 0; i < CONNTRACK_LOCKS; i++) {
- spin_lock(&nf_conntrack_locks[i]);
- spin_unlock(&nf_conntrack_locks[i]);
+ spin_unlock_wait(&nf_conntrack_locks[i]);
}
}
--
2.1.4
^ permalink raw reply related
* [PATCH v2 2/2] net: thunderx: Adjust nicvf structure to reduce cache misses
From: sunil.kovvuri @ 2016-03-14 11:06 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, linux-arm-kernel, davem, sgoutham, robert.richter
In-Reply-To: <1457953578-7054-1-git-send-email-sunil.kovvuri@gmail.com>
From: Sunil Goutham <sgoutham@cavium.com>
Adjusted nicvf structure such that all elements used in hot
path like napi, xmit e.t.c fall into same cache line. This reduced
no of cache misses and resulted in ~2% increase in no of packets
handled on a core.
Also modified elements with :1 notation to boolean, to be
consistent with other element definitions.
Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 52 ++++++++++++++++------------
1 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 872b22d..83025bb 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -272,46 +272,54 @@ struct nicvf {
struct nicvf *pnicvf;
struct net_device *netdev;
struct pci_dev *pdev;
+ void __iomem *reg_base;
+ struct queue_set *qs;
+ struct nicvf_cq_poll *napi[8];
u8 vf_id;
- u8 node;
- u8 tns_mode:1;
- u8 sqs_mode:1;
- u8 loopback_supported:1;
+ u8 sqs_id;
+ bool sqs_mode;
bool hw_tso;
- u16 mtu;
- struct queue_set *qs;
+
+ /* Receive buffer alloc */
+ u32 rb_page_offset;
+ u16 rb_pageref;
+ bool rb_alloc_fail;
+ bool rb_work_scheduled;
+ struct page *rb_page;
+ struct delayed_work rbdr_work;
+ struct tasklet_struct rbdr_task;
+
+ /* Secondary Qset */
+ u8 sqs_count;
#define MAX_SQS_PER_VF_SINGLE_NODE 5
#define MAX_SQS_PER_VF 11
- u8 sqs_id;
- u8 sqs_count; /* Secondary Qset count */
struct nicvf *snicvf[MAX_SQS_PER_VF];
+
+ /* Queue count */
u8 rx_queues;
u8 tx_queues;
u8 max_queues;
- void __iomem *reg_base;
+
+ u8 node;
+ u8 cpi_alg;
+ u16 mtu;
bool link_up;
u8 duplex;
u32 speed;
- struct page *rb_page;
- u32 rb_page_offset;
- u16 rb_pageref;
- bool rb_alloc_fail;
- bool rb_work_scheduled;
- struct delayed_work rbdr_work;
- struct tasklet_struct rbdr_task;
- struct tasklet_struct qs_err_task;
- struct tasklet_struct cq_task;
- struct nicvf_cq_poll *napi[8];
+ bool tns_mode;
+ bool loopback_supported;
struct nicvf_rss_info rss_info;
- u8 cpi_alg;
+ struct tasklet_struct qs_err_task;
+ struct work_struct reset_task;
+
/* Interrupt coalescing settings */
u32 cq_coalesce_usecs;
-
u32 msg_enable;
+
+ /* Stats */
struct nicvf_hw_stats hw_stats;
struct nicvf_drv_stats drv_stats;
struct bgx_stats bgx_stats;
- struct work_struct reset_task;
/* MSI-X */
bool msix_enabled;
--
1.7.1
^ permalink raw reply related
* [PATCH v2 1/2] net: thunderx: Set recevie buffer page usage count in bulk
From: sunil.kovvuri @ 2016-03-14 11:06 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, linux-arm-kernel, davem, sgoutham, robert.richter
In-Reply-To: <1457953578-7054-1-git-send-email-sunil.kovvuri@gmail.com>
From: Sunil Goutham <sgoutham@cavium.com>
Instead of calling get_page() for every receive buffer carved out
of page, set page's usage count at the end, to reduce no of atomic
calls.
Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
---
drivers/net/ethernet/cavium/thunder/nic.h | 1 +
drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 31 ++++++++++++++-----
2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
index 092f097..872b22d 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -294,6 +294,7 @@ struct nicvf {
u32 speed;
struct page *rb_page;
u32 rb_page_offset;
+ u16 rb_pageref;
bool rb_alloc_fail;
bool rb_work_scheduled;
struct delayed_work rbdr_work;
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
index 0dd1abf..fa05e34 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
@@ -18,6 +18,15 @@
#include "q_struct.h"
#include "nicvf_queues.h"
+static void nicvf_get_page(struct nicvf *nic)
+{
+ if (!nic->rb_pageref || !nic->rb_page)
+ return;
+
+ atomic_add(nic->rb_pageref, &nic->rb_page->_count);
+ nic->rb_pageref = 0;
+}
+
/* Poll a register for a specific value */
static int nicvf_poll_reg(struct nicvf *nic, int qidx,
u64 reg, int bit_pos, int bits, int val)
@@ -81,16 +90,15 @@ static inline int nicvf_alloc_rcv_buffer(struct nicvf *nic, gfp_t gfp,
int order = (PAGE_SIZE <= 4096) ? PAGE_ALLOC_COSTLY_ORDER : 0;
/* Check if request can be accomodated in previous allocated page */
- if (nic->rb_page) {
- if ((nic->rb_page_offset + buf_len + buf_len) >
- (PAGE_SIZE << order)) {
- nic->rb_page = NULL;
- } else {
- nic->rb_page_offset += buf_len;
- get_page(nic->rb_page);
- }
+ if (nic->rb_page &&
+ ((nic->rb_page_offset + buf_len) < (PAGE_SIZE << order))) {
+ nic->rb_pageref++;
+ goto ret;
}
+ nicvf_get_page(nic);
+ nic->rb_page = NULL;
+
/* Allocate a new page */
if (!nic->rb_page) {
nic->rb_page = alloc_pages(gfp | __GFP_COMP | __GFP_NOWARN,
@@ -102,7 +110,9 @@ static inline int nicvf_alloc_rcv_buffer(struct nicvf *nic, gfp_t gfp,
nic->rb_page_offset = 0;
}
+ret:
*rbuf = (u64 *)((u64)page_address(nic->rb_page) + nic->rb_page_offset);
+ nic->rb_page_offset += buf_len;
return 0;
}
@@ -158,6 +168,9 @@ static int nicvf_init_rbdr(struct nicvf *nic, struct rbdr *rbdr,
desc = GET_RBDR_DESC(rbdr, idx);
desc->buf_addr = virt_to_phys(rbuf) >> NICVF_RCV_BUF_ALIGN;
}
+
+ nicvf_get_page(nic);
+
return 0;
}
@@ -241,6 +254,8 @@ refill:
new_rb++;
}
+ nicvf_get_page(nic);
+
/* make sure all memory stores are done before ringing doorbell */
smp_wmb();
--
1.7.1
^ permalink raw reply related
* [PATCH v2 0/2] net: thunderx: Performance enhancement changes
From: sunil.kovvuri @ 2016-03-14 11:06 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, linux-arm-kernel, davem, sgoutham, robert.richter
From: Sunil Goutham <sgoutham@cavium.com>
Below patches attempts to improve performance by reducing
no of atomic operations while allocating new receive buffers
and reducing cache misses by adjusting nicvf structure elements.
Changes from v1:
No changes, resubmitting a fresh as per David's suggestion.
Sunil Goutham (2):
net: thunderx: Set recevie buffer page usage count in bulk
net: thunderx: Adjust nicvf structure to reduce cache misses
drivers/net/ethernet/cavium/thunder/nic.h | 51 ++++++++++++--------
drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 31 +++++++++---
2 files changed, 53 insertions(+), 29 deletions(-)
^ permalink raw reply
* Re: [PATCH v6 net-next 09/10] net: add a hardware buffer management helper API
From: Jesper Dangaard Brouer @ 2016-03-14 10:33 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: brouer, David S. Miller, linux-kernel, netdev, Thomas Petazzoni,
Florian Fainelli, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, linux-arm-kernel, Lior Amsalem,
Nadav Haklai, Marcin Wojtas, Simon Guinot,
Russell King - ARM Linux, Willy Tarreau, Timor Kardashov,
Dmitri Epshtein, Sebastian Careba
In-Reply-To: <1457944745-7634-10-git-send-email-gregory.clement@free-electrons.com>
I've not fully understood the hardware support part.
But I do think this generalization is very interesting work, and would
like to cooperate. If my use-case can fit into this, where my use-case
is in the extreme 100Gbit/s area.
There is some potential for performance improvements, if the API from
start is designed distinguish between being called from NAPI-context
(BH-disabled) and outside NAPI context.
See: netdev_alloc_frag() vs napi_alloc_frag().
Nitpicks inlined below...
On Mon, 14 Mar 2016 09:39:04 +0100
Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:
> This basic implementation allows to share code between driver using
> hardware buffer management. As the code is hardware agnostic, there is
> few helpers, most of the optimization brought by the an HW BM has to be
> done at driver level.
>
> Tested-by: Sebastian Careba <nitroshift@yahoo.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> include/net/hwbm.h | 28 ++++++++++++++++++
> net/Kconfig | 3 ++
> net/core/Makefile | 1 +
> net/core/hwbm.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 119 insertions(+)
> create mode 100644 include/net/hwbm.h
> create mode 100644 net/core/hwbm.c
>
> diff --git a/include/net/hwbm.h b/include/net/hwbm.h
> new file mode 100644
> index 000000000000..47d08662501b
> --- /dev/null
> +++ b/include/net/hwbm.h
> @@ -0,0 +1,28 @@
> +#ifndef _HWBM_H
> +#define _HWBM_H
> +
> +struct hwbm_pool {
> + /* Capacity of the pool */
> + int size;
> + /* Size of the buffers managed */
> + int frag_size;
> + /* Number of buffers currently used by this pool */
> + int buf_num;
> + /* constructor called during alocation */
alocation -> allocation
> + int (*construct)(struct hwbm_pool *bm_pool, void *buf);
> + /* protect acces to the buffer counter*/
acces -> access
Space after "counter"
> + spinlock_t lock;
> + /* private data */
> + void *priv;
> +};
> +#ifdef CONFIG_HWBM
> +void hwbm_buf_free(struct hwbm_pool *bm_pool, void *buf);
> +int hwbm_pool_refill(struct hwbm_pool *bm_pool, gfp_t gfp);
> +int hwbm_pool_add(struct hwbm_pool *bm_pool, unsigned int buf_num, gfp_t gfp);
> +#else
> +void hwbm_buf_free(struct hwbm_pool *bm_pool, void *buf) {}
> +int hwbm_pool_refill(struct hwbm_pool *bm_pool, gfp_t gfp) { return 0; }
> +int hwbm_pool_add(struct hwbm_pool *bm_pool, unsigned int buf_num, gfp_t gfp)
> +{ return 0; }
> +#endif /* CONFIG_HWBM */
> +#endif /* _HWBM_H */
> diff --git a/net/Kconfig b/net/Kconfig
> index 10640d5f8bee..e13449870d06 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -253,6 +253,9 @@ config XPS
> depends on SMP
> default y
>
> +config HWBM
> + bool
> +
> config SOCK_CGROUP_DATA
> bool
> default n
> diff --git a/net/core/Makefile b/net/core/Makefile
> index 014422e2561f..d6508c2ddca5 100644
> --- a/net/core/Makefile
> +++ b/net/core/Makefile
> @@ -25,4 +25,5 @@ obj-$(CONFIG_CGROUP_NET_PRIO) += netprio_cgroup.o
> obj-$(CONFIG_CGROUP_NET_CLASSID) += netclassid_cgroup.o
> obj-$(CONFIG_LWTUNNEL) += lwtunnel.o
> obj-$(CONFIG_DST_CACHE) += dst_cache.o
> +obj-$(CONFIG_HWBM) += hwbm.o
> obj-$(CONFIG_NET_DEVLINK) += devlink.o
> diff --git a/net/core/hwbm.c b/net/core/hwbm.c
> new file mode 100644
> index 000000000000..941c28486896
> --- /dev/null
> +++ b/net/core/hwbm.c
> @@ -0,0 +1,87 @@
> +/* Support for hardware buffer manager.
> + *
> + * Copyright (C) 2016 Marvell
> + *
> + * Gregory CLEMENT <gregory.clement@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/kernel.h>
> +#include <linux/printk.h>
> +#include <linux/skbuff.h>
> +#include <net/hwbm.h>
> +
> +void hwbm_buf_free(struct hwbm_pool *bm_pool, void *buf)
> +{
> + if (likely(bm_pool->frag_size <= PAGE_SIZE))
> + skb_free_frag(buf);
> + else
> + kfree(buf);
> +}
> +EXPORT_SYMBOL_GPL(hwbm_buf_free);
> +
> +/* Refill processing for HW buffer management */
> +int hwbm_pool_refill(struct hwbm_pool *bm_pool, gfp_t gfp)
> +{
> + int frag_size = bm_pool->frag_size;
> + void *buf;
> +
> + if (likely(frag_size <= PAGE_SIZE))
> + buf = netdev_alloc_frag(frag_size);
If we know the NAPI-context, there is a performance potential in
netdev_alloc_frag() vs napi_alloc_frag().
> + else
> + buf = kmalloc(frag_size, gfp);
> +
> + if (!buf)
> + return -ENOMEM;
> +
> + if (bm_pool->construct)
> + if (bm_pool->construct(bm_pool, buf)) {
> + hwbm_buf_free(bm_pool, buf);
> + return -ENOMEM;
> + }
Why don't we refill more objects? and do so with a bulk of memory
objects? The "refill" name just lead me to believe that the function
might refill several objects...
Maybe that is the role of hwbm_pool_add() ?
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(hwbm_pool_refill);
> +
> +int hwbm_pool_add(struct hwbm_pool *bm_pool, unsigned int buf_num, gfp_t gfp)
> +{
> + int err, i;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&bm_pool->lock, flags);
This might be needed, and you take the lock for several objects. But
the save/restore variant is the most expensive lock we have (at-least
based on my measurements[1] for Intel).
[1] https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/lib/time_bench_sample.c
> + if (bm_pool->buf_num == bm_pool->size) {
> + pr_warn("pool already filled\n");
> + return bm_pool->buf_num;
> + }
> +
> + if (buf_num + bm_pool->buf_num > bm_pool->size) {
> + pr_warn("cannot allocate %d buffers for pool\n",
> + buf_num);
> + return 0;
> + }
> +
> + if ((buf_num + bm_pool->buf_num) < bm_pool->buf_num) {
> + pr_warn("Adding %d buffers to the %d current buffers will overflow\n",
> + buf_num, bm_pool->buf_num);
> + return 0;
> + }
> +
> + for (i = 0; i < buf_num; i++) {
> + err = hwbm_pool_refill(bm_pool, gfp);
I'm thinking why not use some bulk allocation API here...
> + if (err < 0)
> + break;
> + }
> +
> + /* Update BM driver with number of buffers added to pool */
> + bm_pool->buf_num += i;
> +
> + pr_debug("hwpm pool: %d of %d buffers added\n", i, buf_num);
> + spin_unlock_irqrestore(&bm_pool->lock, flags);
> +
> + return i;
> +}
> +EXPORT_SYMBOL_GPL(hwbm_pool_add);
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox