* Re: [PATCH] mdio: mux: fix device_node_continue.cocci warnings
From: Florian Fainelli @ 2017-05-12 22:52 UTC (permalink / raw)
To: David Miller, julia.lawall, jon.mason
Cc: netdev, jon.mason, andrew, kbuild-all, linux-kernel
In-Reply-To: <20170512.122223.341607427763777325.davem@davemloft.net>
On 05/12/2017 09:22 AM, David Miller wrote:
> From: Julia Lawall <julia.lawall@lip6.fr>
> Date: Fri, 12 May 2017 22:54:23 +0800 (SGT)
>
>> Device node iterators put the previous value of the index variable, so an
>> explicit put causes a double put.
> ...
>> @@ -169,7 +169,6 @@ int mdio_mux_init(struct device *dev,
>> if (r) {
>> mdiobus_free(cb->mii_bus);
>> devm_kfree(dev, cb);
>> - of_node_put(child_bus_node);
>> } else {
>
> I think we're instead simply missing a break; statement here.
It's kind of questionable, if we have an error initializing one of our
child MDIO bus controller (child from the perspective of the MDIO mux,
boy this is getting complicated...), should we keep on going, or should
we abort entirely and rollback what we have successfully registered?
I don't think Julia's patch makes thing worse, in that if we had to
rollback, we would not be doing this correctly now anyway.
Jon, what do you think?
--
Florian
^ permalink raw reply
* [PATCH net v2] netvsc: fix net poll mode
From: Stephen Hemminger @ 2017-05-12 22:55 UTC (permalink / raw)
To: davem; +Cc: netdev, Stephen Hemminger
The ndo_poll_controller function needs to schedule NAPI to pick
up arriving packets and send completions. Otherwise no data
will ever be received.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
v2 - use rcu to ensure lower data structure is present
drivers/net/hyperv/netvsc_drv.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 4421a6d00375..c171f5bc41b9 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1158,11 +1158,22 @@ netvsc_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
}
#ifdef CONFIG_NET_POLL_CONTROLLER
-static void netvsc_poll_controller(struct net_device *net)
+static void netvsc_poll_controller(struct net_device *dev)
{
- /* As netvsc_start_xmit() works synchronous we don't have to
- * trigger anything here.
- */
+ struct net_device_context *ndc = netdev_priv(dev);
+ struct netvsc_device *ndev;
+ int i;
+
+ rcu_read_lock();
+ ndev = rcu_dereference(ndc->nvdev);
+ if (ndev) {
+ for (i = 0; i < ndev->num_chn; i++) {
+ struct netvsc_channel *nvchan = &ndev->chan_table[i];
+
+ napi_schedule(&nvchan->napi);
+ }
+ }
+ rcu_read_unlock();
}
#endif
--
2.11.0
^ permalink raw reply related
* Re: [Problem] Broadcom BCM5762 Ethernet tg3 times out with stack trace
From: Daniel Kim @ 2017-05-12 23:12 UTC (permalink / raw)
To: Michael Chan
Cc: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan,
Linux Netdev List
In-Reply-To: <CACKFLi=762N5LzmcOE4dPS_1Op8EsJ8QJBRFqZySaSx7SNb1bg@mail.gmail.com>
I believe I do have IOMMU enabled. At least the dmesg output seems to
imply that I do:
[ 1.141948] iommu: Adding device 0000:00:02.0 to group 0
[ 1.142033] iommu: Adding device 0000:00:10.0 to group 1
[ 1.142074] iommu: Adding device 0000:00:10.1 to group 1
[ 1.142119] iommu: Adding device 0000:00:11.0 to group 2
[ 1.142172] iommu: Adding device 0000:00:12.0 to group 3
[ 1.142184] iommu: Adding device 0000:00:12.2 to group 3
[ 1.142234] iommu: Adding device 0000:00:13.0 to group 4
[ 1.142247] iommu: Adding device 0000:00:13.2 to group 4
[ 1.142303] iommu: Adding device 0000:00:14.0 to group 5
[ 1.142315] iommu: Adding device 0000:00:14.2 to group 5
[ 1.142328] iommu: Adding device 0000:00:14.3 to group 5
[ 1.142373] iommu: Adding device 0000:00:14.4 to group 6
[ 1.142417] iommu: Adding device 0000:00:14.5 to group 7
[ 1.142529] iommu: Adding device 0000:00:15.0 to group 8
[ 1.142570] iommu: Adding device 0000:00:15.2 to group 8
[ 1.142639] iommu: Adding device 0000:00:18.0 to group 9
[ 1.142653] iommu: Adding device 0000:00:18.1 to group 9
[ 1.142668] iommu: Adding device 0000:00:18.2 to group 9
[ 1.142682] iommu: Adding device 0000:00:18.3 to group 9
[ 1.142695] iommu: Adding device 0000:00:18.4 to group 9
[ 1.142712] iommu: Adding device 0000:00:18.5 to group 9
[ 1.142725] iommu: Adding device 0000:01:00.0 to group 0
[ 1.142733] iommu: Adding device 0000:01:00.1 to group 0
[ 1.142840] iommu: Adding device 0000:04:00.0 to group 8
[ 1.143305] AMD-Vi: Found IOMMU at 0000:00:00.2 cap 0x40
The last kernel I used where the NIC mostly worked was 4.4.0 (x86-64).
Even when it worked, it had this issue we've experienced since v3.13
where the NIC would suddenly become unresponsive. It won't be
considered disconnected from the network, but RX/TX packets, errors,
dropped, etc. values displayed with ifconfig would show a number that
looked suspiciously close to 32-bit INT_MAX.
Since v4.8, I can trigger it much faster and it now prints out a call
stack and other messages into dmesg.
^ permalink raw reply
* [PATCH] kmod: don't load module unless req process has CAP_SYS_MODULE
From: Mahesh Bandewar @ 2017-05-12 23:22 UTC (permalink / raw)
To: Ingo Molnar, Greg Kroah-Hartman, LKML, netdev
Cc: Eric W . Biederman, Kees Cook, David Miller, Eric Dumazet,
Mahesh Bandewar, Mahesh Bandewar
From: Mahesh Bandewar <maheshb@google.com>
A process inside random user-ns should not load a module, which is
currently possible. As demonstrated in following scenario -
Create namespaces; especially a user-ns and become root inside.
$ unshare -rfUp -- unshare -unm -- bash
Try to load the bridge module. It should fail and this is expected!
# modprobe bridge
WARNING: Error inserting stp (/lib/modules/4.11.0-smp-DEV/kernel/net/802/stp.ko): Operation not permitted
FATAL: Error inserting bridge (/lib/modules/4.11.0-smp-DEV/kernel/net/bridge/bridge.ko): Operation not permitted
Verify bridge module is not loaded.
# lsmod | grep bridge
#
Now try to create a bridge inside this newly created net-ns which would
mean bridge module need to be loaded.
# ip link add br0 type bridge
# echo $?
0
# lsmod | grep bridge
bridge 110592 0
stp 16384 1 bridge
llc 16384 2 bridge,stp
#
After this patch -
# ip link add br0 type bridge
RTNETLINK answers: Operation not supported
# echo $?
2
# lsmod | grep bridge
#
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
kernel/kmod.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 563f97e2be36..ac30157169b7 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -133,6 +133,9 @@ int __request_module(bool wait, const char *fmt, ...)
#define MAX_KMOD_CONCURRENT 50 /* Completely arbitrary value - KAO */
static int kmod_loop_msg;
+ if (!capable(CAP_SYS_MODULE))
+ return -EPERM;
+
/*
* We don't allow synchronous module loading from async. Module
* init may invoke async_synchronize_full() which will end up
--
2.13.0.rc2.291.g57267f2277-goog
^ permalink raw reply related
* Re: [PATCH net v2] netvsc: fix net poll mode
From: Stephen Hemminger @ 2017-05-12 23:57 UTC (permalink / raw)
To: davem; +Cc: netdev, Stephen Hemminger
In-Reply-To: <20170512225530.2233-1-sthemmin@microsoft.com>
On Fri, 12 May 2017 15:55:30 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:
> The ndo_poll_controller function needs to schedule NAPI to pick
> up arriving packets and send completions. Otherwise no data
> will ever be received.
>
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Hold off on this. I will send v3 with a couple other RCU related fixes.
^ permalink raw reply
* [PATCH net] ipv6: avoid dad-failures for addresses with NODAD
From: Mahesh Bandewar @ 2017-05-13 0:03 UTC (permalink / raw)
To: Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, David Miller
Cc: Eric Dumazet, Mahesh Bandewar, Mahesh Bandewar
From: Mahesh Bandewar <maheshb@google.com>
Every address gets added with TENTATIVE flag even for the addresses with
IFA_F_NODAD flag and dad-work is scheduled for them. During this DAD process
we realize it's an address with NODAD and complete the process without
sending any probe. However the TENTATIVE flags stays on the
address for sometime enough to cause misinterpretation when we receive a NS.
While processing NS, if the address has TENTATIVE flag, we mark it DADFAILED
and endup with an address that was originally configured as NODAD with
DADFAILED.
We can't avoid scheduling dad_work for addresses with NODAD but we can
avoid adding TENTATIVE flag to avoid this racy situation.
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
net/ipv6/addrconf.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index b09ac38d8dc4..53f2dc092023 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1022,7 +1022,10 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
INIT_HLIST_NODE(&ifa->addr_lst);
ifa->scope = scope;
ifa->prefix_len = pfxlen;
- ifa->flags = flags | IFA_F_TENTATIVE;
+ ifa->flags = flags;
+ /* No need to add the TENTATIVE flag for addresses with NODAD */
+ if (!(flags & IFA_F_NODAD))
+ ifa->flags |= IFA_F_TENTATIVE;
ifa->valid_lft = valid_lft;
ifa->prefered_lft = prefered_lft;
ifa->cstamp = ifa->tstamp = jiffies;
--
2.13.0.rc2.291.g57267f2277-goog
^ permalink raw reply related
* Re: [PATCH v5 14/17] dt-bindings: slave-device: add current-speed property
From: Rob Herring @ 2017-05-13 0:05 UTC (permalink / raw)
To: Stefan Wahren
Cc: David S. Miller, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
linux-serial, linux-kernel, netdev, devicetree
In-Reply-To: <1494406408-31760-15-git-send-email-stefan.wahren@i2se.com>
On Wed, May 10, 2017 at 10:53:25AM +0200, Stefan Wahren wrote:
> This adds a new DT property to define the current baud rate of the
> slave device.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> Documentation/devicetree/bindings/serial/slave-device.txt | 9 +++++++++
> 1 file changed, 9 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH iproute2 -master 0/2] Two misc BPF updates
From: Daniel Borkmann @ 2017-05-13 0:32 UTC (permalink / raw)
To: stephen; +Cc: alexei.starovoitov, netdev, Daniel Borkmann
Requires header rebase with -net.
Thanks!
Daniel Borkmann (2):
bpf: update printing of generic xdp mode
bpf: dump error to the user when retrieving pinned prog fails
ip/iplink_xdp.c | 19 +++++++++++--------
lib/bpf.c | 12 +++++++++++-
2 files changed, 22 insertions(+), 9 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH iproute2 -master 2/2] bpf: dump error to the user when retrieving pinned prog fails
From: Daniel Borkmann @ 2017-05-13 0:32 UTC (permalink / raw)
To: stephen; +Cc: alexei.starovoitov, netdev, Daniel Borkmann
In-Reply-To: <cover.1494635217.git.daniel@iogearbox.net>
I noticed we currently don't dump an error message when a pinned
program couldn't be retrieved, thus add a hint to the user.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
lib/bpf.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/lib/bpf.c b/lib/bpf.c
index 04ee1ab..ae4d97d 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -648,6 +648,16 @@ static int bpf_obj_get(const char *pathname, enum bpf_prog_type type)
return bpf(BPF_OBJ_GET, &attr, sizeof(attr));
}
+static int bpf_obj_pinned(const char *pathname, enum bpf_prog_type type)
+{
+ int prog_fd = bpf_obj_get(pathname, type);
+
+ if (prog_fd < 0)
+ fprintf(stderr, "Couldn\'t retrieve pinned program \'%s\': %s\n",
+ pathname, strerror(errno));
+ return prog_fd;
+}
+
enum bpf_mode {
CBPF_BYTECODE,
CBPF_FILE,
@@ -750,7 +760,7 @@ static int bpf_parse(enum bpf_prog_type *type, enum bpf_mode *mode,
else if (*mode == EBPF_OBJECT)
ret = bpf_obj_open(file, *type, section, verbose);
else if (*mode == EBPF_PINNED)
- ret = bpf_obj_get(file, *type);
+ ret = bpf_obj_pinned(file, *type);
else
return -1;
--
1.9.3
^ permalink raw reply related
* [PATCH iproute2 -master 1/2] bpf: update printing of generic xdp mode
From: Daniel Borkmann @ 2017-05-13 0:32 UTC (permalink / raw)
To: stephen; +Cc: alexei.starovoitov, netdev, Daniel Borkmann
In-Reply-To: <cover.1494635217.git.daniel@iogearbox.net>
Follow-up to d67b9cd28c1d ("xdp: refine xdp api with regards to
generic xdp") in order to update the XDP dumping part.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
ip/iplink_xdp.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
index a1380ee..98503fa 100644
--- a/ip/iplink_xdp.c
+++ b/ip/iplink_xdp.c
@@ -79,17 +79,20 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic)
void xdp_dump(FILE *fp, struct rtattr *xdp)
{
struct rtattr *tb[IFLA_XDP_MAX + 1];
- __u32 flags = 0;
+ __u8 mode;
parse_rtattr_nested(tb, IFLA_XDP_MAX, xdp);
- if (!tb[IFLA_XDP_ATTACHED] ||
- !rta_getattr_u8(tb[IFLA_XDP_ATTACHED]))
+ if (!tb[IFLA_XDP_ATTACHED])
return;
- if (tb[IFLA_XDP_FLAGS])
- flags = rta_getattr_u32(tb[IFLA_XDP_FLAGS]);
-
- fprintf(fp, "xdp%s ",
- flags & XDP_FLAGS_SKB_MODE ? "generic" : "");
+ mode = rta_getattr_u8(tb[IFLA_XDP_ATTACHED]);
+ if (mode == XDP_ATTACHED_NONE)
+ return;
+ else if (mode == XDP_ATTACHED_DRV)
+ fprintf(fp, "xdp ");
+ else if (mode == XDP_ATTACHED_SKB)
+ fprintf(fp, "xdpgeneric ");
+ else
+ fprintf(fp, "xdp[%u] ", mode);
}
--
1.9.3
^ permalink raw reply related
* Re: [PATCH] mdio: mux: fix device_node_continue.cocci warnings
From: Julia Lawall @ 2017-05-13 0:40 UTC (permalink / raw)
To: Florian Fainelli
Cc: David Miller, jon.mason, netdev, andrew, kbuild-all, linux-kernel
In-Reply-To: <872f3980-9faa-718f-3260-9e4b22946140@gmail.com>
On Fri, 12 May 2017, Florian Fainelli wrote:
> On 05/12/2017 09:22 AM, David Miller wrote:
> > From: Julia Lawall <julia.lawall@lip6.fr>
> > Date: Fri, 12 May 2017 22:54:23 +0800 (SGT)
> >
> >> Device node iterators put the previous value of the index variable, so an
> >> explicit put causes a double put.
> > ...
> >> @@ -169,7 +169,6 @@ int mdio_mux_init(struct device *dev,
> >> if (r) {
> >> mdiobus_free(cb->mii_bus);
> >> devm_kfree(dev, cb);
> >> - of_node_put(child_bus_node);
> >> } else {
> >
> > I think we're instead simply missing a break; statement here.
>
> It's kind of questionable, if we have an error initializing one of our
> child MDIO bus controller (child from the perspective of the MDIO mux,
> boy this is getting complicated...), should we keep on going, or should
> we abort entirely and rollback what we have successfully registered?
>
> I don't think Julia's patch makes thing worse, in that if we had to
> rollback, we would not be doing this correctly now anyway.
Just to be clear, if you want the break instead, then you need to keep the
put.
julia
>
> Jon, what do you think?
> --
> Florian
>
^ permalink raw reply
* Re: [PATCH net-next] ipv6: Implement limits on hop by hop and destination options
From: Tom Herbert @ 2017-05-13 1:02 UTC (permalink / raw)
To: David Miller; +Cc: Linux Kernel Network Developers, Martin Lau
In-Reply-To: <20170512.113847.1389407948502066394.davem@davemloft.net>
On Fri, May 12, 2017 at 8:38 AM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <tom@herbertland.com>
> Date: Wed, 10 May 2017 14:33:19 -0700
>
>> RFC 2460 (IPv6) defines hop by hop options and destination options
>> extension headers. Both of these carry a list of TLVs which is
>> only limited by the maximum length of the extension header (2048
>> bytes). By the spec a host must process all the TLVs in these
>> options, however these could be used as a fairly obvious
>> denial of service attack. I think this could in fact be
>> a significant DOS vector on the Internet, one mitigating
>> factor might be that many FWs drop all packets with EH (and
>> obviously this is only IPv6) so an Internet wide attack might not
>> be so effective (yet!).
>>
>> By my calculation, the worse case packet with TLVs in a standard
>> 1500 byte MTU packet that would be processed by the stack contains
>> 1282 invidual TLVs (including pad TLVS) or 724 two byte TLVs. I
>> wrote a quick test program that floods a whole bunch of these
>> packets to a host and sure enough there is substantial time spent
>> in ip6_parse_tlv. These packets contain nothing but unknown TLVS
>> (that are ignored), TLV padding, and bogus UDP header with zero
>> payload length.
> ...
>> Default values are set to 8 for options counts, and set to INT_MAX
>> for maximum length. Note the choice to limit options to 8 is an
>> arbitrary guess (roughly based on the fact that the stack supports
>> three HBH options and just one destination option).
>
> So the maximum number of TLVs we could process is some function of the
> option count limit, and the number of padding TLVs that can be stuffed
> alongside of those 8 non-padding options?
Yes, if you count the one byte pad TLVs then maximum TLVs processed (#
times through the loop) with the default of 8 is 8 + 9*7= 71. The
padding TLVs are less expensive to process at least.
Function is:
N(max_non_pad) = (max_non_pad + 1) * 7 + max_non_pad
^ permalink raw reply
* [PATCH net] i40e: Fix state flags for bit set and clean operations of PF
From: Mauro S. M. Rodrigues @ 2017-05-13 2:26 UTC (permalink / raw)
To: netdev, intel-wired-lan
Cc: jeffrey.t.kirsher, gpiccoli, Mauro S. M. Rodrigues
Commit 0da36b9774cc ("i40e: use DECLARE_BITMAP for state fields")
introduced changes in the way i40e works with state flags converting
them to bitmaps using kernel bitmap API. This change introduced a
regression due to a mistaken substitution using __I40E_VSI_DOWN instead
of __I40E_DOWN when testing state of a PF at i40e_reset_subtask()
function. This caused a flood in the kernel log with the follow message:
[49.013] i40e 0002:01:00.0: bad reset request 0x00000020
Commit d19cb64b9222 ("i40e: separate PF and VSI state flags")
also introduced some misuse of the VSI and PF flags, so both could be
considered as the offenders.
This patch simply fixes the flags where it makes sense by changing
__I40E_VSI_DOWN to __I40E_DOWN.
Fixes: 0da36b9774cc ("i40e: use DECLARE_BITMAP for state fields")
Fixes: d19cb64b9222 ("i40e: separate PF and VSI state flags")
Reviewed-by: Guilherme G. Piccoli <gpiccoli@linux.vnet.ibm.com>
Signed-off-by: Mauro S. M. Rodrigues <maurosr@linux.vnet.ibm.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 36 ++++++++++++++---------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d5c9c9e..150caf6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -295,7 +295,7 @@ struct i40e_vsi *i40e_find_vsi_from_id(struct i40e_pf *pf, u16 id)
**/
void i40e_service_event_schedule(struct i40e_pf *pf)
{
- if (!test_bit(__I40E_VSI_DOWN, pf->state) &&
+ if (!test_bit(__I40E_DOWN, pf->state) &&
!test_bit(__I40E_RESET_RECOVERY_PENDING, pf->state))
queue_work(i40e_wq, &pf->service_task);
}
@@ -3611,7 +3611,7 @@ static irqreturn_t i40e_intr(int irq, void *data)
* this is not a performance path and napi_schedule()
* can deal with rescheduling.
*/
- if (!test_bit(__I40E_VSI_DOWN, pf->state))
+ if (!test_bit(__I40E_DOWN, pf->state))
napi_schedule_irqoff(&q_vector->napi);
}
@@ -3687,7 +3687,7 @@ static irqreturn_t i40e_intr(int irq, void *data)
enable_intr:
/* re-enable interrupt causes */
wr32(hw, I40E_PFINT_ICR0_ENA, ena_mask);
- if (!test_bit(__I40E_VSI_DOWN, pf->state)) {
+ if (!test_bit(__I40E_DOWN, pf->state)) {
i40e_service_event_schedule(pf);
i40e_irq_dynamic_enable_icr0(pf, false);
}
@@ -6203,7 +6203,7 @@ static void i40e_fdir_reinit_subtask(struct i40e_pf *pf)
{
/* if interface is down do nothing */
- if (test_bit(__I40E_VSI_DOWN, pf->state))
+ if (test_bit(__I40E_DOWN, pf->state))
return;
if (test_bit(__I40E_FD_FLUSH_REQUESTED, pf->state))
@@ -6344,7 +6344,7 @@ static void i40e_watchdog_subtask(struct i40e_pf *pf)
int i;
/* if interface is down do nothing */
- if (test_bit(__I40E_VSI_DOWN, pf->state) ||
+ if (test_bit(__I40E_DOWN, pf->state) ||
test_bit(__I40E_CONFIG_BUSY, pf->state))
return;
@@ -6399,9 +6399,9 @@ static void i40e_reset_subtask(struct i40e_pf *pf)
reset_flags |= BIT(__I40E_GLOBAL_RESET_REQUESTED);
clear_bit(__I40E_GLOBAL_RESET_REQUESTED, pf->state);
}
- if (test_bit(__I40E_VSI_DOWN_REQUESTED, pf->state)) {
- reset_flags |= BIT(__I40E_VSI_DOWN_REQUESTED);
- clear_bit(__I40E_VSI_DOWN_REQUESTED, pf->state);
+ if (test_bit(__I40E_DOWN_REQUESTED, pf->state)) {
+ reset_flags |= BIT(__I40E_DOWN_REQUESTED);
+ clear_bit(__I40E_DOWN_REQUESTED, pf->state);
}
/* If there's a recovery already waiting, it takes
@@ -6415,7 +6415,7 @@ static void i40e_reset_subtask(struct i40e_pf *pf)
/* If we're already down or resetting, just bail */
if (reset_flags &&
- !test_bit(__I40E_VSI_DOWN, pf->state) &&
+ !test_bit(__I40E_DOWN, pf->state) &&
!test_bit(__I40E_CONFIG_BUSY, pf->state)) {
rtnl_lock();
i40e_do_reset(pf, reset_flags, true);
@@ -7002,7 +7002,7 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
u32 val;
int v;
- if (test_bit(__I40E_VSI_DOWN, pf->state))
+ if (test_bit(__I40E_DOWN, pf->state))
goto clear_recovery;
dev_dbg(&pf->pdev->dev, "Rebuilding internal switch\n");
@@ -9767,7 +9767,7 @@ int i40e_vsi_release(struct i40e_vsi *vsi)
return -ENODEV;
}
if (vsi == pf->vsi[pf->lan_vsi] &&
- !test_bit(__I40E_VSI_DOWN, pf->state)) {
+ !test_bit(__I40E_DOWN, pf->state)) {
dev_info(&pf->pdev->dev, "Can't remove PF VSI\n");
return -ENODEV;
}
@@ -11003,7 +11003,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
pf->next_vsi = 0;
pf->pdev = pdev;
- set_bit(__I40E_VSI_DOWN, pf->state);
+ set_bit(__I40E_DOWN, pf->state);
hw = &pf->hw;
hw->back = pf;
@@ -11293,7 +11293,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
* before setting up the misc vector or we get a race and the vector
* ends up disabled forever.
*/
- clear_bit(__I40E_VSI_DOWN, pf->state);
+ clear_bit(__I40E_DOWN, pf->state);
/* In case of MSIX we are going to setup the misc vector right here
* to handle admin queue events etc. In case of legacy and MSI
@@ -11448,7 +11448,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
/* Unwind what we've done if something failed in the setup */
err_vsis:
- set_bit(__I40E_VSI_DOWN, pf->state);
+ set_bit(__I40E_DOWN, pf->state);
i40e_clear_interrupt_scheme(pf);
kfree(pf->vsi);
err_switch_setup:
@@ -11500,7 +11500,7 @@ static void i40e_remove(struct pci_dev *pdev)
/* no more scheduling of any task */
set_bit(__I40E_SUSPENDED, pf->state);
- set_bit(__I40E_VSI_DOWN, pf->state);
+ set_bit(__I40E_DOWN, pf->state);
if (pf->service_timer.data)
del_timer_sync(&pf->service_timer);
if (pf->service_task.func)
@@ -11740,7 +11740,7 @@ static void i40e_shutdown(struct pci_dev *pdev)
struct i40e_hw *hw = &pf->hw;
set_bit(__I40E_SUSPENDED, pf->state);
- set_bit(__I40E_VSI_DOWN, pf->state);
+ set_bit(__I40E_DOWN, pf->state);
rtnl_lock();
i40e_prep_for_reset(pf, true);
rtnl_unlock();
@@ -11789,7 +11789,7 @@ static int i40e_suspend(struct pci_dev *pdev, pm_message_t state)
int retval = 0;
set_bit(__I40E_SUSPENDED, pf->state);
- set_bit(__I40E_VSI_DOWN, pf->state);
+ set_bit(__I40E_DOWN, pf->state);
if (pf->wol_en && (pf->flags & I40E_FLAG_WOL_MC_MAGIC_PKT_WAKE))
i40e_enable_mc_magic_wake(pf);
@@ -11841,7 +11841,7 @@ static int i40e_resume(struct pci_dev *pdev)
/* handling the reset will rebuild the device state */
if (test_and_clear_bit(__I40E_SUSPENDED, pf->state)) {
- clear_bit(__I40E_VSI_DOWN, pf->state);
+ clear_bit(__I40E_DOWN, pf->state);
rtnl_lock();
i40e_reset_and_rebuild(pf, false, true);
rtnl_unlock();
--
2.7.4
^ permalink raw reply related
* [PATCH 0/3] bpf: Track MAP pointer alignment
From: David Miller @ 2017-05-13 2:28 UTC (permalink / raw)
To: daniel; +Cc: ast, alexei.starovoitov, netdev
This patch series updates the BPF verifier to properly track MAP
access alignment, just as we already do for packet pointers.
The two main elements of the implementation are putting the register
offset into a shared rather than reg type specific area of the
verifier register tracking strcutre, and also eliding packet pointer
specific checks when managing MAP pointer alignment.
More details of the implementation are in the commit message for patch
#2.
Patch #3 removes alignment restrictions from various test_verifier.c
tests which are no longer necessary with this MAP alignment
infrastructure in place. Further improvements are possible, but
I didn't want to be overly ambitious here.
In the future we can add some MAP tests to test_align.c as well.
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH 1/3] bpf: Use 1<<16 as ceiling for immediate alignment in verifier.
From: David Miller @ 2017-05-13 2:28 UTC (permalink / raw)
To: daniel; +Cc: ast, alexei.starovoitov, netdev
If we use 1<<31, then sequences like:
R1 = 0
R1 <<= 2
do silly things. Examples of this actually exist in
the MAP tests of test_verifier.c
Update test_align.c expectation strings.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
kernel/bpf/verifier.c | 2 +-
tools/testing/selftests/bpf/test_align.c | 12 ++++++------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 39f2dcb..0f33db0 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1715,7 +1715,7 @@ static void check_reg_overflow(struct bpf_reg_state *reg)
static u32 calc_align(u32 imm)
{
if (!imm)
- return 1U << 31;
+ return 1U << 16;
return imm - ((imm - 1) & imm);
}
diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
index 9644d4e..afaa297 100644
--- a/tools/testing/selftests/bpf/test_align.c
+++ b/tools/testing/selftests/bpf/test_align.c
@@ -235,12 +235,12 @@ static struct bpf_align_test tests[] = {
},
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
.matches = {
- "4: R0=imm0,min_value=0,max_value=0,min_align=2147483648 R1=ctx R2=pkt(id=0,off=0,r=0) R3=pkt_end R5=pkt(id=0,off=0,r=0) R10=fp",
- "5: R0=imm0,min_value=0,max_value=0,min_align=2147483648 R1=ctx R2=pkt(id=0,off=0,r=0) R3=pkt_end R5=pkt(id=0,off=14,r=0) R10=fp",
- "6: R0=imm0,min_value=0,max_value=0,min_align=2147483648 R1=ctx R2=pkt(id=0,off=0,r=0) R3=pkt_end R4=pkt(id=0,off=14,r=0) R5=pkt(id=0,off=14,r=0) R10=fp",
- "10: R0=imm0,min_value=0,max_value=0,min_align=2147483648 R1=ctx R2=pkt(id=0,off=0,r=18) R3=pkt_end R4=inv56 R5=pkt(id=0,off=14,r=18) R10=fp",
- "14: R0=imm0,min_value=0,max_value=0,min_align=2147483648 R1=ctx R2=pkt(id=0,off=0,r=18) R3=pkt_end R4=inv48 R5=pkt(id=0,off=14,r=18) R10=fp",
- "15: R0=imm0,min_value=0,max_value=0,min_align=2147483648 R1=ctx R2=pkt(id=0,off=0,r=18) R3=pkt_end R4=inv48 R5=pkt(id=0,off=14,r=18) R10=fp",
+ "4: R0=imm0,min_value=0,max_value=0,min_align=65536 R1=ctx R2=pkt(id=0,off=0,r=0) R3=pkt_end R5=pkt(id=0,off=0,r=0) R10=fp",
+ "5: R0=imm0,min_value=0,max_value=0,min_align=65536 R1=ctx R2=pkt(id=0,off=0,r=0) R3=pkt_end R5=pkt(id=0,off=14,r=0) R10=fp",
+ "6: R0=imm0,min_value=0,max_value=0,min_align=65536 R1=ctx R2=pkt(id=0,off=0,r=0) R3=pkt_end R4=pkt(id=0,off=14,r=0) R5=pkt(id=0,off=14,r=0) R10=fp",
+ "10: R0=imm0,min_value=0,max_value=0,min_align=65536 R1=ctx R2=pkt(id=0,off=0,r=18) R3=pkt_end R4=inv56 R5=pkt(id=0,off=14,r=18) R10=fp",
+ "14: R0=imm0,min_value=0,max_value=0,min_align=65536 R1=ctx R2=pkt(id=0,off=0,r=18) R3=pkt_end R4=inv48 R5=pkt(id=0,off=14,r=18) R10=fp",
+ "15: R0=imm0,min_value=0,max_value=0,min_align=65536 R1=ctx R2=pkt(id=0,off=0,r=18) R3=pkt_end R4=inv48 R5=pkt(id=0,off=14,r=18) R10=fp",
},
},
{
--
2.1.2.532.g19b5d50
^ permalink raw reply related
* [PATCH 2/3] bpf: Track alignment of MAP pointers in verifier.
From: David Miller @ 2017-05-13 2:28 UTC (permalink / raw)
To: daniel; +Cc: ast, alexei.starovoitov, netdev
Just like packet pointers, track the known alignment of MAP pointers.
In order to facilitate the state tracking, move the register offset
field into where there is an unused 32-bit padding slot on 64-bit.
The check logic is the same as for packet pointers, except we do not
apply NET_IP_ALIGN to the calculations.
Also, there are several restrictions that apply to packet pointers
which we do not extend to MAP pointers. For example, the
MAX_PACKET_OFF limitation and the "adding integer with < 48 upper zero
bits" thing.
When we add a variable to the MAP pointer, all of the state
transitions are identical except that we elide the reg->range clear
because it is a packet pointer specific piece of state.
This changes the string emitted when an unaligned access is trapped by
the verifier. Therefore, we need to adjust the search string used by
test_verifier.c
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/linux/bpf_verifier.h | 6 +-
kernel/bpf/verifier.c | 112 +++++++++++++++++++---------
tools/testing/selftests/bpf/test_verifier.c | 3 +-
3 files changed, 80 insertions(+), 41 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index d5093b5..acf711b 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -18,15 +18,13 @@
struct bpf_reg_state {
enum bpf_reg_type type;
+ u32 off;
union {
/* valid when type == CONST_IMM | PTR_TO_STACK | UNKNOWN_VALUE */
s64 imm;
/* valid when type == PTR_TO_PACKET* */
- struct {
- u16 off;
- u16 range;
- };
+ u32 range;
/* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE |
* PTR_TO_MAP_VALUE_OR_NULL
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0f33db0..b187815 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -231,10 +231,10 @@ static void print_verifier_state(struct bpf_verifier_state *state)
else if (t == CONST_PTR_TO_MAP || t == PTR_TO_MAP_VALUE ||
t == PTR_TO_MAP_VALUE_OR_NULL ||
t == PTR_TO_MAP_VALUE_ADJ)
- verbose("(ks=%d,vs=%d,id=%u)",
+ verbose("(ks=%d,vs=%d,id=%u,off=%u)",
reg->map_ptr->key_size,
reg->map_ptr->value_size,
- reg->id);
+ reg->id, reg->off);
if (reg->min_value != BPF_REGISTER_MIN_RANGE)
verbose(",min_value=%lld",
(long long)reg->min_value);
@@ -469,6 +469,7 @@ static void init_reg_state(struct bpf_reg_state *regs)
for (i = 0; i < MAX_BPF_REG; i++) {
regs[i].type = NOT_INIT;
+ regs[i].off = 0;
regs[i].imm = 0;
regs[i].min_value = BPF_REGISTER_MIN_RANGE;
regs[i].max_value = BPF_REGISTER_MAX_RANGE;
@@ -487,6 +488,7 @@ static void init_reg_state(struct bpf_reg_state *regs)
static void __mark_reg_unknown_value(struct bpf_reg_state *regs, u32 regno)
{
regs[regno].type = UNKNOWN_VALUE;
+ regs[regno].off = 0;
regs[regno].id = 0;
regs[regno].imm = 0;
}
@@ -823,10 +825,27 @@ static int check_pkt_ptr_alignment(const struct bpf_reg_state *reg,
}
static int check_val_ptr_alignment(const struct bpf_reg_state *reg,
- int size, bool strict)
+ int off, int size, bool strict)
{
- if (strict && size != 1) {
- verbose("Unknown alignment. Only byte-sized access allowed in value access.\n");
+ int reg_off;
+
+ /* Byte size accesses are always allowed. */
+ if (!strict || size == 1)
+ return 0;
+
+ reg_off = reg->off;
+ if (reg->id) {
+ if (reg->aux_off_align % size) {
+ verbose("Value access is only %u byte aligned, %d byte access not allowed\n",
+ reg->aux_off_align, size);
+ return -EACCES;
+ }
+ reg_off += reg->aux_off;
+ }
+
+ if ((reg_off + off) % size != 0) {
+ verbose("misaligned value access off %d+%d size %d\n",
+ reg_off, off, size);
return -EACCES;
}
@@ -846,7 +865,7 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
case PTR_TO_PACKET:
return check_pkt_ptr_alignment(reg, off, size, strict);
case PTR_TO_MAP_VALUE_ADJ:
- return check_val_ptr_alignment(reg, size, strict);
+ return check_val_ptr_alignment(reg, off, size, strict);
default:
if (off % size != 0) {
verbose("misaligned access off %d size %d\n",
@@ -1336,6 +1355,7 @@ static void clear_all_pkt_pointers(struct bpf_verifier_env *env)
reg->type != PTR_TO_PACKET_END)
continue;
reg->type = UNKNOWN_VALUE;
+ reg->off = 0;
reg->imm = 0;
}
}
@@ -1415,6 +1435,7 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
for (i = 0; i < CALLER_SAVED_REGS; i++) {
reg = regs + caller_saved[i];
reg->type = NOT_INIT;
+ reg->off = 0;
reg->imm = 0;
}
@@ -1458,8 +1479,8 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
return 0;
}
-static int check_packet_ptr_add(struct bpf_verifier_env *env,
- struct bpf_insn *insn)
+static int check_pointer_add(struct bpf_verifier_env *env,
+ struct bpf_insn *insn, bool is_packet)
{
struct bpf_reg_state *regs = env->cur_state.regs;
struct bpf_reg_state *dst_reg = ®s[insn->dst_reg];
@@ -1468,28 +1489,28 @@ static int check_packet_ptr_add(struct bpf_verifier_env *env,
s32 imm;
if (BPF_SRC(insn->code) == BPF_K) {
- /* pkt_ptr += imm */
+ /* pointer += imm */
imm = insn->imm;
add_imm:
- if (imm < 0) {
- verbose("addition of negative constant to packet pointer is not allowed\n");
- return -EACCES;
- }
- if (imm >= MAX_PACKET_OFF ||
- imm + dst_reg->off >= MAX_PACKET_OFF) {
- verbose("constant %d is too large to add to packet pointer\n",
- imm);
- return -EACCES;
+ if (is_packet) {
+ if (imm < 0) {
+ verbose("addition of negative constant to packet pointer is not allowed\n");
+ return -EACCES;
+ }
+ if (imm >= MAX_PACKET_OFF ||
+ imm + dst_reg->off >= MAX_PACKET_OFF) {
+ verbose("constant %d is too large to add to packet pointer\n",
+ imm);
+ return -EACCES;
+ }
}
- /* a constant was added to pkt_ptr.
+ /* a constant was added to the pointer.
* Remember it while keeping the same 'id'
*/
dst_reg->off += imm;
} else {
- bool had_id;
-
- if (src_reg->type == PTR_TO_PACKET) {
+ if (is_packet && src_reg->type == PTR_TO_PACKET) {
/* R6=pkt(id=0,off=0,r=62) R7=imm22; r7 += r6 */
tmp_reg = *dst_reg; /* save r7 state */
*dst_reg = *src_reg; /* copy pkt_ptr state r6 into r7 */
@@ -1503,46 +1524,62 @@ static int check_packet_ptr_add(struct bpf_verifier_env *env,
}
if (src_reg->type == CONST_IMM) {
- /* pkt_ptr += reg where reg is known constant */
+ /* pointer += reg where reg is known constant */
imm = src_reg->imm;
goto add_imm;
}
- /* disallow pkt_ptr += reg
+ /* disallow pointer += reg
* if reg is not uknown_value with guaranteed zero upper bits
- * otherwise pkt_ptr may overflow and addition will become
+ * otherwise pointer_ptr may overflow and addition will become
* subtraction which is not allowed
*/
if (src_reg->type != UNKNOWN_VALUE) {
- verbose("cannot add '%s' to ptr_to_packet\n",
+ verbose("cannot add '%s' to pointer\n",
reg_type_str[src_reg->type]);
return -EACCES;
}
- if (src_reg->imm < 48) {
+ if (is_packet && src_reg->imm < 48) {
verbose("cannot add integer value with %lld upper zero bits to ptr_to_packet\n",
src_reg->imm);
return -EACCES;
}
- had_id = (dst_reg->id != 0);
-
- /* dst_reg stays as pkt_ptr type and since some positive
+ /* dst_reg stays as the same type and since some positive
* integer value was added to the pointer, increment its 'id'
*/
dst_reg->id = ++env->id_gen;
- /* something was added to pkt_ptr, set range to zero */
dst_reg->aux_off += dst_reg->off;
dst_reg->off = 0;
- dst_reg->range = 0;
- if (had_id)
+
+ if (is_packet) {
+ /* something was added to packet ptr, set range to zero */
+ dst_reg->range = 0;
+ }
+ if (dst_reg->aux_off_align) {
dst_reg->aux_off_align = min(dst_reg->aux_off_align,
src_reg->min_align);
- else
+ } else {
dst_reg->aux_off_align = src_reg->min_align;
+ }
+ if (!dst_reg->aux_off_align)
+ dst_reg->aux_off_align = 1;
}
return 0;
}
+static int check_packet_ptr_add(struct bpf_verifier_env *env,
+ struct bpf_insn *insn)
+{
+ return check_pointer_add(env, insn, true);
+}
+
+static int check_map_ptr_add(struct bpf_verifier_env *env,
+ struct bpf_insn *insn)
+{
+ return check_pointer_add(env, insn, false);
+}
+
static int evaluate_reg_alu(struct bpf_verifier_env *env, struct bpf_insn *insn)
{
struct bpf_reg_state *regs = env->cur_state.regs;
@@ -2056,10 +2093,12 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
if (env->allow_ptr_leaks &&
BPF_CLASS(insn->code) == BPF_ALU64 && opcode == BPF_ADD &&
(dst_reg->type == PTR_TO_MAP_VALUE ||
- dst_reg->type == PTR_TO_MAP_VALUE_ADJ))
+ dst_reg->type == PTR_TO_MAP_VALUE_ADJ)) {
dst_reg->type = PTR_TO_MAP_VALUE_ADJ;
- else
+ check_map_ptr_add(env, insn);
+ } else {
mark_reg_unknown_value(regs, insn->dst_reg);
+ }
}
return 0;
@@ -2480,6 +2519,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
for (i = 0; i < CALLER_SAVED_REGS; i++) {
reg = regs + caller_saved[i];
reg->type = NOT_INIT;
+ reg->off = 0;
reg->imm = 0;
}
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 3773562..889fb5a 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -5070,7 +5070,8 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
reject_from_alignment = fd_prog < 0 &&
(test->flags & F_NEEDS_EFFICIENT_UNALIGNED_ACCESS) &&
- strstr(bpf_vlog, "Unknown alignment.");
+ (strstr(bpf_vlog, "misaligned value access") ||
+ strstr(bpf_vlog, "Value access is only"));
#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
if (reject_from_alignment) {
printf("FAIL\nFailed due to alignment despite having efficient unaligned access: '%s'!\n",
--
2.1.2.532.g19b5d50
^ permalink raw reply related
* [PATCH 3/3] bpf: Update MAP test_verifier.c tests wrt. alignment.
From: David Miller @ 2017-05-13 2:29 UTC (permalink / raw)
To: daniel; +Cc: ast, alexei.starovoitov, netdev
Now that we perform proper alignment tracking of MAP accesses in the
verifier, most of the MAP tests in test_verifier.c no longer need the
special F_NEEDS_EFFICIENT_UNALIGNED_ACCESS flag or can trivially be
made to not need it.
struct test_val is an integer count followed by an array of integers.
So for most tests, we adjust the memory access to be 32-bit rather
than 64-bit.
The exception is for tests where the 64-bit access is an important
aspect for what the test is trying to validate. For example,
the test "invalid map access into an array with a invalid max check"
is trying to do a 64-bit store at the final 4 bytes of the test
array which violates the valid access range.
We could fix this by arranging the test array size such that
the last entry is 8 byte aligned, but that is for future
consideration.
Two other tests which retain the flag are intentionally doing
unaligned accesses to a MAP element.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
tools/testing/selftests/bpf/test_verifier.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 889fb5a..73ac23a 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -3117,7 +3117,6 @@ static struct bpf_test tests[] = {
.errstr_unpriv = "R0 pointer arithmetic prohibited",
.result_unpriv = REJECT,
.result = ACCEPT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"valid map access into an array with a variable",
@@ -3133,7 +3132,7 @@ static struct bpf_test tests[] = {
BPF_JMP_IMM(BPF_JGE, BPF_REG_1, MAX_ENTRIES, 3),
BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 2),
BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
- BPF_ST_MEM(BPF_DW, BPF_REG_0, 0,
+ BPF_ST_MEM(BPF_W, BPF_REG_0, 0,
offsetof(struct test_val, foo)),
BPF_EXIT_INSN(),
},
@@ -3141,7 +3140,6 @@ static struct bpf_test tests[] = {
.errstr_unpriv = "R0 pointer arithmetic prohibited",
.result_unpriv = REJECT,
.result = ACCEPT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"valid map access into an array with a signed variable",
@@ -3161,7 +3159,7 @@ static struct bpf_test tests[] = {
BPF_MOV32_IMM(BPF_REG_1, 0),
BPF_ALU32_IMM(BPF_LSH, BPF_REG_1, 2),
BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
- BPF_ST_MEM(BPF_DW, BPF_REG_0, 0,
+ BPF_ST_MEM(BPF_W, BPF_REG_0, 0,
offsetof(struct test_val, foo)),
BPF_EXIT_INSN(),
},
@@ -3169,7 +3167,6 @@ static struct bpf_test tests[] = {
.errstr_unpriv = "R0 pointer arithmetic prohibited",
.result_unpriv = REJECT,
.result = ACCEPT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"invalid map access into an array with a constant",
@@ -3211,7 +3208,6 @@ static struct bpf_test tests[] = {
.errstr = "R0 min value is outside of the array range",
.result_unpriv = REJECT,
.result = REJECT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"invalid map access into an array with a variable",
@@ -3226,7 +3222,7 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_0, 0),
BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 2),
BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
- BPF_ST_MEM(BPF_DW, BPF_REG_0, 0,
+ BPF_ST_MEM(BPF_W, BPF_REG_0, 0,
offsetof(struct test_val, foo)),
BPF_EXIT_INSN(),
},
@@ -3235,7 +3231,6 @@ static struct bpf_test tests[] = {
.errstr = "R0 min value is negative, either use unsigned index or do a if (index >=0) check.",
.result_unpriv = REJECT,
.result = REJECT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"invalid map access into an array with no floor check",
@@ -3253,7 +3248,7 @@ static struct bpf_test tests[] = {
BPF_MOV32_IMM(BPF_REG_1, 0),
BPF_ALU32_IMM(BPF_LSH, BPF_REG_1, 2),
BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
- BPF_ST_MEM(BPF_DW, BPF_REG_0, 0,
+ BPF_ST_MEM(BPF_W, BPF_REG_0, 0,
offsetof(struct test_val, foo)),
BPF_EXIT_INSN(),
},
@@ -3262,7 +3257,6 @@ static struct bpf_test tests[] = {
.errstr = "R0 min value is negative, either use unsigned index or do a if (index >=0) check.",
.result_unpriv = REJECT,
.result = REJECT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"invalid map access into an array with a invalid max check",
@@ -3319,7 +3313,6 @@ static struct bpf_test tests[] = {
.errstr = "R0 min value is negative, either use unsigned index or do a if (index >=0) check.",
.result_unpriv = REJECT,
.result = REJECT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"multiple registers share map_lookup_elem result",
@@ -3435,7 +3428,7 @@ static struct bpf_test tests[] = {
BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 1),
BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 2),
BPF_ALU64_REG(BPF_ADD, BPF_REG_0, BPF_REG_1),
- BPF_ST_MEM(BPF_DW, BPF_REG_0, 0, offsetof(struct test_val, foo)),
+ BPF_ST_MEM(BPF_W, BPF_REG_0, 0, offsetof(struct test_val, foo)),
BPF_EXIT_INSN(),
},
.fixup_map2 = { 3 },
@@ -3443,7 +3436,6 @@ static struct bpf_test tests[] = {
.result = REJECT,
.errstr_unpriv = "R0 pointer arithmetic prohibited",
.result_unpriv = REJECT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"constant register |= constant should keep constant type",
@@ -4835,7 +4827,6 @@ static struct bpf_test tests[] = {
.errstr = "R0 min value is negative, either use unsigned index or do a if (index >=0) check.",
.result = REJECT,
.result_unpriv = REJECT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"invalid range check",
@@ -4867,7 +4858,6 @@ static struct bpf_test tests[] = {
.errstr = "R0 min value is negative, either use unsigned index or do a if (index >=0) check.",
.result = REJECT,
.result_unpriv = REJECT,
- .flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
{
"map in map access",
--
2.1.2.532.g19b5d50
^ permalink raw reply related
* Re: [PATCH] net: ipv6: Truncate single route when it doesn't fit into dump buffer.
From: David Ahern @ 2017-05-13 6:52 UTC (permalink / raw)
To: Jan Moskyto Matejka; +Cc: David Miller, mq, netdev, roopa
In-Reply-To: <20170512214107.w3q7ycl6wm7phwox@lopatka.joja.cz>
On 5/12/17 3:41 PM, Jan Moskyto Matejka wrote:
> On Fri, May 12, 2017 at 10:26:08AM -0700, David Ahern wrote:
>> On 5/12/17 8:24 AM, David Miller wrote:
>>> From: Jan Moskyto Matejka <mq@ucw.cz>
>>> Date: Fri, 12 May 2017 13:15:10 +0200
>>>
>>>> -int rt6_dump_route(struct rt6_info *rt, void *p_arg);
>>>> +int rt6_dump_route(struct rt6_info *rt, void *p_arg, int truncate);
>>>
>>> Please use "bool" and "true"/"false" for boolean values.
>>>
>>> What does ipv4 do in this situation?
>>>
>>> I'm hesitant to be OK with adding a new nlmsg flag just for this case
>>> if we solve this problem differently and using existing mechanisms
>>> elsewhere.
>>>
>>
>> I'll take a look at this later today or this weekend; we can't just
>> truncate the route returned to userspace.
>
> Agreed. My favourite would be skb realloc somewhere inside the dump loop
> ... but I don't know whether it's feasible.
>
> MQ
>
Here is what is happening: user initiates a route dump and specifies a
buffer size for receiving the message which becomes max_recvmsg_len.
This buffer size dictates the skb length allocated by netlink_dump.
The dump is interrupted when a route does not fit in the skb and
returns. Subsequent call picks up with the next route to be dumped - the
one that overflowed. All good and normal so far.
If the next route is larger than max_recvmsg_len, then the route can not
be put in the buffer, nothing is returned to the user which causes the
dump to abort showing the abbreviated output. This problem occurs with
IPv4 and IPv6. You can see this with modest size routes by just dropping
the buffer size to something really small (e.g., with iproute2, change
buf size in rtnl_dump_filter_l to say 2048).
I see 2 problems:
1. the kernel is not telling the user the supplied buffer is too small
(ie., if a single route does not fit in the skb then it should fail and
return an error code to the user),
2. multipath routes for IPv4 and IPv6 do not have a limit.
Should the kernel put a limit on the number of nexthops? I recently put
a cap on MPLS route size as 4096 bytes, but I think this should be
revisited in terms of a limit on number of nexthops to create a
consistent limit even if struct sizes change. And, the limit on the
number of nexthops should be consistent across address families (same
limit for IPv4, IPv6, and MPLS).
>From discussions I have had, 32 nexthops for a single route is on the
laughably high side, but some people do crazy things. How about a limit
of 256 nexthops?
^ permalink raw reply
* Re: [PATCH iproute2 -master 1/2] bpf: update printing of generic xdp mode
From: Sergei Shtylyov @ 2017-05-13 9:01 UTC (permalink / raw)
To: Daniel Borkmann, stephen; +Cc: alexei.starovoitov, netdev
In-Reply-To: <3c90a0b9480c61746220636ed48320da53410b6f.1494635217.git.daniel@iogearbox.net>
Hello!
On 5/13/2017 3:32 AM, Daniel Borkmann wrote:
> Follow-up to d67b9cd28c1d ("xdp: refine xdp api with regards to
> generic xdp") in order to update the XDP dumping part.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> ip/iplink_xdp.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
> index a1380ee..98503fa 100644
> --- a/ip/iplink_xdp.c
> +++ b/ip/iplink_xdp.c
> @@ -79,17 +79,20 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic)
> void xdp_dump(FILE *fp, struct rtattr *xdp)
> {
> struct rtattr *tb[IFLA_XDP_MAX + 1];
> - __u32 flags = 0;
> + __u8 mode;
>
> parse_rtattr_nested(tb, IFLA_XDP_MAX, xdp);
>
> - if (!tb[IFLA_XDP_ATTACHED] ||
> - !rta_getattr_u8(tb[IFLA_XDP_ATTACHED]))
> + if (!tb[IFLA_XDP_ATTACHED])
> return;
>
> - if (tb[IFLA_XDP_FLAGS])
> - flags = rta_getattr_u32(tb[IFLA_XDP_FLAGS]);
> -
> - fprintf(fp, "xdp%s ",
> - flags & XDP_FLAGS_SKB_MODE ? "generic" : "");
> + mode = rta_getattr_u8(tb[IFLA_XDP_ATTACHED]);
> + if (mode == XDP_ATTACHED_NONE)
> + return;
> + else if (mode == XDP_ATTACHED_DRV)
> + fprintf(fp, "xdp ");
> + else if (mode == XDP_ATTACHED_SKB)
> + fprintf(fp, "xdpgeneric ");
> + else
> + fprintf(fp, "xdp[%u] ", mode);
This is asking to be a *switch* statement.
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH iproute2 -master 1/2] bpf: update printing of generic xdp mode
From: Daniel Borkmann @ 2017-05-13 9:19 UTC (permalink / raw)
To: Sergei Shtylyov, stephen; +Cc: alexei.starovoitov, netdev
In-Reply-To: <d012ef18-8689-81ef-8f56-ef2b9c673b44@cogentembedded.com>
On 05/13/2017 11:01 AM, Sergei Shtylyov wrote:
> On 5/13/2017 3:32 AM, Daniel Borkmann wrote:
>
>> Follow-up to d67b9cd28c1d ("xdp: refine xdp api with regards to
>> generic xdp") in order to update the XDP dumping part.
>>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> ---
>> ip/iplink_xdp.c | 19 +++++++++++--------
>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
>> index a1380ee..98503fa 100644
>> --- a/ip/iplink_xdp.c
>> +++ b/ip/iplink_xdp.c
>> @@ -79,17 +79,20 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic)
>> void xdp_dump(FILE *fp, struct rtattr *xdp)
>> {
>> struct rtattr *tb[IFLA_XDP_MAX + 1];
>> - __u32 flags = 0;
>> + __u8 mode;
>>
>> parse_rtattr_nested(tb, IFLA_XDP_MAX, xdp);
>>
>> - if (!tb[IFLA_XDP_ATTACHED] ||
>> - !rta_getattr_u8(tb[IFLA_XDP_ATTACHED]))
>> + if (!tb[IFLA_XDP_ATTACHED])
>> return;
>>
>> - if (tb[IFLA_XDP_FLAGS])
>> - flags = rta_getattr_u32(tb[IFLA_XDP_FLAGS]);
>> -
>> - fprintf(fp, "xdp%s ",
>> - flags & XDP_FLAGS_SKB_MODE ? "generic" : "");
>> + mode = rta_getattr_u8(tb[IFLA_XDP_ATTACHED]);
>> + if (mode == XDP_ATTACHED_NONE)
>> + return;
>> + else if (mode == XDP_ATTACHED_DRV)
>> + fprintf(fp, "xdp ");
>> + else if (mode == XDP_ATTACHED_SKB)
>> + fprintf(fp, "xdpgeneric ");
>> + else
>> + fprintf(fp, "xdp[%u] ", mode);
>
> This is asking to be a *switch* statement.
The code is fine as-is, thank you !
^ permalink raw reply
* Re: [PATCH v1] samples/bpf: Add a .gitignore for binaries
From: Mickaël Salaün @ 2017-05-13 10:30 UTC (permalink / raw)
To: David Ahern, Alexei Starovoitov, Daniel Borkmann
Cc: linux-kernel, Arnaldo Carvalho de Melo, Wang Nan, netdev
In-Reply-To: <8277c7cc-8f44-40ec-1f7e-53c404d84292@cumulusnetworks.com>
[-- Attachment #1.1: Type: text/plain, Size: 849 bytes --]
On 13/02/2017 02:43, David Ahern wrote:
> On 2/12/17 2:23 PM, Mickaël Salaün wrote:
>> diff --git a/samples/bpf/.gitignore b/samples/bpf/.gitignore
>> new file mode 100644
>> index 000000000000..a7562a5ef4c2
>> --- /dev/null
>> +++ b/samples/bpf/.gitignore
>> @@ -0,0 +1,32 @@
>> +fds_example
>> +lathist
>
> ...
>
> Listing each target is going to be a PITA to maintain. It would be
> better to put targets into a build directory (bin?) and ignore the
> directory.
>
It would require a lot of modifications to the Makefile and more
complexity. It seems much more simple for everyone to stick to a simple
gitignore file easily maintainable:
$ awk '$1 == "hostprogs-y" { print $3 }' < Makefile > .gitignore
Alexei, Daniel, what do you think about this? Do you want me to send a
v2 with the new tests?
Mickaël
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* You Just Won Qatar Foundation Donation**Treat Urgently
From: QATAR, FOUNDATION @ 2017-05-13 10:52 UTC (permalink / raw)
To: Recipients
Dear Beneficiary,
You have been selected to receive (950,000.00 EURO) as charity donations / aid from the Qatar Foundation. Please kindly provide the
below information details mention.
1. Your Full Name:
2. Country:
3. Marital Status:
4. Age:
5. Occupation:
6. Telephone:
7. Home Address:
Note: This donation funds will be send to you Via ATM Master Card
Yours sincerely,
Engineer Saad Al Muhannadi.
President of the Qatar Foundation.
^ permalink raw reply
* Re: [PATCH] net: ipv6: Truncate single route when it doesn't fit into dump buffer.
From: Jan Moskyto Matejka @ 2017-05-13 10:54 UTC (permalink / raw)
To: David Ahern; +Cc: David Miller, mq, netdev, roopa
In-Reply-To: <6670aa0f-2f07-5afd-7c47-35b006c9b2e7@gmail.com>
On Sat, May 13, 2017 at 12:52:47AM -0600, David Ahern wrote:
> On 5/12/17 3:41 PM, Jan Moskyto Matejka wrote:
> > On Fri, May 12, 2017 at 10:26:08AM -0700, David Ahern wrote:
> >> On 5/12/17 8:24 AM, David Miller wrote:
> >>> From: Jan Moskyto Matejka <mq@ucw.cz>
> >>> Date: Fri, 12 May 2017 13:15:10 +0200
> >>>
> >>>> -int rt6_dump_route(struct rt6_info *rt, void *p_arg);
> >>>> +int rt6_dump_route(struct rt6_info *rt, void *p_arg, int truncate);
> >>>
> >>> Please use "bool" and "true"/"false" for boolean values.
> >>>
> >>> What does ipv4 do in this situation?
> >>>
> >>> I'm hesitant to be OK with adding a new nlmsg flag just for this case
> >>> if we solve this problem differently and using existing mechanisms
> >>> elsewhere.
> >>>
> >>
> >> I'll take a look at this later today or this weekend; we can't just
> >> truncate the route returned to userspace.
> >
> > Agreed. My favourite would be skb realloc somewhere inside the dump loop
> > ... but I don't know whether it's feasible.
> >
> > MQ
> >
>
> Here is what is happening: user initiates a route dump and specifies a
> buffer size for receiving the message which becomes max_recvmsg_len.
> This buffer size dictates the skb length allocated by netlink_dump.
>
> The dump is interrupted when a route does not fit in the skb and
> returns. Subsequent call picks up with the next route to be dumped - the
> one that overflowed. All good and normal so far.
>
> If the next route is larger than max_recvmsg_len, then the route can not
> be put in the buffer, nothing is returned to the user which causes the
> dump to abort showing the abbreviated output. This problem occurs with
> IPv4 and IPv6. You can see this with modest size routes by just dropping
> the buffer size to something really small (e.g., with iproute2, change
> buf size in rtnl_dump_filter_l to say 2048).
Ergo no skb realloc is possible.
> I see 2 problems:
> 1. the kernel is not telling the user the supplied buffer is too small
> (ie., if a single route does not fit in the skb then it should fail and
> return an error code to the user),
Definitely. I want just to note that this condition usually occurs
somewhere during route dump. To know it before starting output, we would
have to walk the FIB once before dump to calculate max route len.
> 2. multipath routes for IPv4 and IPv6 do not have a limit.
>
> Should the kernel put a limit on the number of nexthops? I recently put
> a cap on MPLS route size as 4096 bytes, but I think this should be
> revisited in terms of a limit on number of nexthops to create a
> consistent limit even if struct sizes change. And, the limit on the
> number of nexthops should be consistent across address families (same
> limit for IPv4, IPv6, and MPLS).
>
> From discussions I have had, 32 nexthops for a single route is on the
> laughably high side, but some people do crazy things. How about a limit
> of 256 nexthops?
256 should be OK even for a crazy developer of BIRD.
It would be nice to have if the returned error were somehow useful for
the userspace -- to know what is happening, not only something like
"impossible to add / append route".
Thanks for looking into that!
MQ
^ permalink raw reply
* Re: [PATCH V2 net 1/1] smc: switch to usage of IB_PD_UNSAFE_GLOBAL_RKEY
From: Leon Romanovsky @ 2017-05-13 11:29 UTC (permalink / raw)
To: Ursula Braun
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, hch-jcswGhMUV9g,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA,
jwi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
schwidefsky-tA70FqPdS9bQT0dZR+AlfA,
heiko.carstens-tA70FqPdS9bQT0dZR+AlfA,
raspl-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
In-Reply-To: <20170512170952.39863-2-ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 732 bytes --]
On Fri, May 12, 2017 at 07:09:52PM +0200, Ursula Braun wrote:
> This patch makes users aware of the current security implications
> when using smc.
>
> The final fix to resolve the reported security issue is worked on;
> respective patches will follow as soon as possible.
>
> Signed-off-by: Ursula Braun <ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
I'm glad that you moved to use IB_PD_UNSAFE_GLOBAL_RKEY, so the users
will see the warning in their dmesg log.
However can you please update your commit log? There is need to add
description of security issue (access to whole physical memory), clear
message that doesn't fix anything and remove mentioning unpredictable
future (... as soon as possible ...).
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 net-next 1/5] dsa: add support for Microchip KSZ tail tagging
From: Andrew Lunn @ 2017-05-13 13:03 UTC (permalink / raw)
To: Woojung.Huh
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A632AB@CHN-SV-EXMX02.mchp-main.com>
On Fri, May 12, 2017 at 08:07:54PM +0000, Woojung.Huh@microchip.com wrote:
> + padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
> +
> + if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
> + nskb = skb;
> + } else {
> + nskb = alloc_skb(NET_IP_ALIGN + skb->len +
> + padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC);
Hi Woojung
Thanks for reusing the existing skb when possible. This should help
with performance.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ 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