* [PATCH] nfp: bpf: improve handling for disabled BPF syscall
From: Arnd Bergmann @ 2016-09-23 20:23 UTC (permalink / raw)
To: Jakub Kicinski, David S. Miller
Cc: Arnd Bergmann, oss-drivers, netdev, linux-kernel
I stumbled over a new warning during randconfig testing,
with CONFIG_BPF_SYSCALL disabled:
drivers/net/ethernet/netronome/nfp/nfp_net_offload.c: In function 'nfp_net_bpf_offload':
drivers/net/ethernet/netronome/nfp/nfp_net_offload.c:263:3: error: '*((void *)&res+4)' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/net/ethernet/netronome/nfp/nfp_net_offload.c:263:3: error: 'res.n_instr' may be used uninitialized in this function [-Werror=maybe-uninitialized]
As far as I can tell, this is a false positive caused by the compiler
getting confused about a function that is partially inlined, but it's
easy to avoid while improving the code:
The nfp_bpf_jit() stub helper for that configuration is unusual as it
is defined in a header file but not marked 'static inline'. By moving
the compile-time check into the caller using the IS_ENABLED() macro,
we can remove that stub and simplify the nfp_net_bpf_offload_prepare()
function enough to unconfuse the compiler.
Fixes: 7533fdc0f77f ("nfp: bpf: add hardware bpf offload")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/netronome/nfp/nfp_bpf.h | 10 ----------
drivers/net/ethernet/netronome/nfp/nfp_net_offload.c | 3 +++
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_bpf.h b/drivers/net/ethernet/netronome/nfp/nfp_bpf.h
index fc220cd04115..87aa8a3e9112 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_bpf.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_bpf.h
@@ -192,20 +192,10 @@ struct nfp_bpf_result {
bool dense_mode;
};
-#ifdef CONFIG_BPF_SYSCALL
int
nfp_bpf_jit(struct bpf_prog *filter, void *prog, enum nfp_bpf_action_type act,
unsigned int prog_start, unsigned int prog_done,
unsigned int prog_sz, struct nfp_bpf_result *res);
-#else
-int
-nfp_bpf_jit(struct bpf_prog *filter, void *prog, enum nfp_bpf_action_type act,
- unsigned int prog_start, unsigned int prog_done,
- unsigned int prog_sz, struct nfp_bpf_result *res)
-{
- return -ENOTSUPP;
-}
-#endif
int nfp_prog_verify(struct nfp_prog *nfp_prog, struct bpf_prog *prog);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
index 43f42f842eda..8acfb631a0ea 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
@@ -148,6 +148,9 @@ nfp_net_bpf_offload_prepare(struct nfp_net *nn,
unsigned int max_mtu;
int ret;
+ if (!IS_ENABLED(CONFIG_BPF_SYSCALL))
+ return -ENOTSUPP;
+
ret = nfp_net_bpf_get_act(nn, cls_bpf);
if (ret < 0)
return ret;
--
2.9.0
^ permalink raw reply related
* Re: [RFC] net: store port/representative id in metadata_dst
From: John Fastabend @ 2016-09-23 20:25 UTC (permalink / raw)
To: Jakub Kicinski, Samudrala, Sridhar
Cc: Jiri Benc, Jiri Pirko, netdev, Thomas Graf, Roopa Prabhu,
ogerlitz, ast, daniel, simon.horman, Paolo Abeni, Pravin B Shelar,
hannes, kubakici
In-Reply-To: <20160923211728.4c2f05ab@jkicinski-Precision-T1700>
On 16-09-23 01:17 PM, Jakub Kicinski wrote:
> On Fri, 23 Sep 2016 10:22:59 -0700, Samudrala, Sridhar wrote:
>> On 9/23/2016 8:29 AM, Jakub Kicinski wrote:
>>> On Fri, 23 Sep 2016 07:23:26 -0700, John Fastabend wrote:
>>>> Yep, I like the idea in general. I had a slightly different approach in
>>>> mind though. If you look at __dev_queue_xmit() there is a void
>>>> accel_priv pointer (gather you found this based on your commit note).
>>>> My take was we could extend this a bit so it can be used by the VFR
>>>> devices and they could do a dev_queue_xmit_accel(). In this way there is
>>>> no need to touch /net/core/{filter, dst, ip_tunnel}.c etc. Maybe the
>>>> accel logic needs to be extended to push the priv pointer all the way
>>>> through the xmit routine of the target netdev though. This should look
>>>> a lot like the macvlan accelerated xmit device path without the
>>>> switching logic.
>>>>
>>>> Of course maybe the name would be extended to dev_queue_xmit_extended()
>>>> or something.
>>>>
>>>> So the flow on ingress would be,
>>>>
>>>> 1. pkt_received_by_PF_netdev
>>>> 2. PF_netdev reads some tag off packet/descriptor and sets correct
>>>> skb->dev field. This is needed so stack "sees" packets from
>>>> correct VF ports.
>>>> 3. packet passed up to stack.
>>>>
>>>> I guess it is a bit "zombie" like on the receive path because the packet
>>>> is never actually handled by VF netdev code per se and on egress can
>>>> traverse both the VFR and PF netdevs qdiscs. But on the other hand the
>>>> VFR netdevs and PF netdevs are all in the same driver. Plus using a
>>>> queue per VFR is a bit of a waste as its not needed and also hardware
>>>> may not have any mechanism to push VF traffic onto a rx queue.
>>>>
>>>> On egress,
>>>>
>>>> 1. VFR xmit is called
>>>> 2. VFR xmit calls dev_queue_xmit_accel() with some meta-data if needed
>>>> for the lower netdev
>>>> 3. lower netdev sends out the packet.
>>>>
>>>> Again we don't need to waste any queues for each VFR and the VFR can be
>>>> a LLTX device. In this scheme I think you avoid much of the changes in
>>>> your patch and keep it all contained in the driver. Any thoughts?
>>
>> The 'accel' parameter in dev_queue_xmit_accel() is currently only passed
>> to ndo_select_queue() via netdev_pick_tx() and is used to select the tx
>> queue.
>> Also, it is not passed all the way to the driver specific xmit routine.
>> Doesn't it require
>> changing all the driver xmit routines if we want to pass this parameter?
>>
>>> Goes without saying that you have a much better understanding of packet
>>> scheduling so please bear with me :) My target model is that I have
>>> n_cpus x "n_tc/prio" queues on the PF and I want to transmit the
>>> fallback traffic over those same queues. So no new HW queues are used
>>> for VFRs at all. This is a reverse of macvlan offload which AFAICT has
>>> "bastard hw queues" which actually TX for a separate software device.
>>>
>>> My understanding was that I can rework this model to have software
>>> queues for VFRs (#sw queues == #PF queues + #VFRs) but no extra HW
>>> queues (#hw queues == #PF queues) but then when the driver sees a
>>> packet on sw-only VFR queue it has to pick one of the PF queues (which
>>> one?), lock PF software queue to own it, and only then can it
>>> transmit. With the dst_metadata there is no need for extra locking or
>>> queue selection.
>>
>> Yes. The VFPR netdevs don't have any HW queues associated with them and
>> we would like
>> to use the PF queues for the xmit.
>> I was also looking into some way of passing the port id via skb
>> parameter to the
>> dev_queue_xmit() call so that the PF xmit routine can do a directed
>> transmit to a specifc VF.
>> Is skb->cb an option to pass this info?
>> dst_metadata approach would work too if it is acceptable.
>
> I don't think we can trust skb->cb to be set to anything meaningful
> when the skb is received by the lower device.
>
Agreed. I wouldn't recommend using skb->cb. How about passing it through
dev_queue_xmit_accel() through to the driver?
If you pass the metadata through the dev_queue_xmit_accel() handle tx
queue selection would work using normal mechanisms (xps, select_queue,
cls hook, etc.). If you wanted to pick some specific queue based on
policy the policy could be loaded into one of those hooks.
.John
^ permalink raw reply
* [PATCH 1/3] bpf powerpc: introduce accessors for using the tmp local stack space
From: Naveen N. Rao @ 2016-09-23 20:35 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, netdev, Michael Ellerman
Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Ananth N Mavinakayanahalli
While at it, ensure that the location of the local save area is
consistent whether or not we setup our own stackframe. This property is
utilised in the next patch that adds support for tail calls.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/net/bpf_jit64.h | 16 +++++---
arch/powerpc/net/bpf_jit_comp64.c | 79 ++++++++++++++++++++++-----------------
2 files changed, 55 insertions(+), 40 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit64.h b/arch/powerpc/net/bpf_jit64.h
index 5046d6f..a1645d7 100644
--- a/arch/powerpc/net/bpf_jit64.h
+++ b/arch/powerpc/net/bpf_jit64.h
@@ -16,22 +16,25 @@
/*
* Stack layout:
+ * Ensure the top half (upto local_tmp_var) stays consistent
+ * with our redzone usage.
*
* [ prev sp ] <-------------
* [ nv gpr save area ] 8*8 |
+ * [ tail_call_cnt ] 8 |
+ * [ local_tmp_var ] 8 |
* fp (r31) --> [ ebpf stack space ] 512 |
- * [ local/tmp var space ] 16 |
* [ frame header ] 32/112 |
* sp (r1) ---> [ stack pointer ] --------------
*/
-/* for bpf JIT code internal usage */
-#define BPF_PPC_STACK_LOCALS 16
/* for gpr non volatile registers BPG_REG_6 to 10, plus skb cache registers */
#define BPF_PPC_STACK_SAVE (8*8)
+/* for bpf JIT code internal usage */
+#define BPF_PPC_STACK_LOCALS 16
/* Ensure this is quadword aligned */
-#define BPF_PPC_STACKFRAME (STACK_FRAME_MIN_SIZE + BPF_PPC_STACK_LOCALS + \
- MAX_BPF_STACK + BPF_PPC_STACK_SAVE)
+#define BPF_PPC_STACKFRAME (STACK_FRAME_MIN_SIZE + MAX_BPF_STACK + \
+ BPF_PPC_STACK_LOCALS + BPF_PPC_STACK_SAVE)
#ifndef __ASSEMBLY__
@@ -65,6 +68,9 @@ static const int b2p[] = {
[TMP_REG_2] = 10
};
+/* PPC NVR range -- update this if we ever use NVRs below r24 */
+#define BPF_PPC_NVR_MIN 24
+
/* Assembly helpers */
#define DECLARE_LOAD_FUNC(func) u64 func(u64 r3, u64 r4); \
u64 func##_negative_offset(u64 r3, u64 r4); \
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 6073b78..5f8c91f 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -58,6 +58,35 @@ static inline bool bpf_has_stack_frame(struct codegen_context *ctx)
return ctx->seen & SEEN_FUNC || bpf_is_seen_register(ctx, BPF_REG_FP);
}
+/*
+ * When not setting up our own stackframe, the redzone usage is:
+ *
+ * [ prev sp ] <-------------
+ * [ ... ] |
+ * sp (r1) ---> [ stack pointer ] --------------
+ * [ nv gpr save area ] 8*8
+ * [ tail_call_cnt ] 8
+ * [ local_tmp_var ] 8
+ * [ unused red zone ] 208 bytes protected
+ */
+static int bpf_jit_stack_local(struct codegen_context *ctx)
+{
+ if (bpf_has_stack_frame(ctx))
+ return STACK_FRAME_MIN_SIZE + MAX_BPF_STACK;
+ else
+ return -(BPF_PPC_STACK_SAVE + 16);
+}
+
+static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
+{
+ if (reg >= BPF_PPC_NVR_MIN && reg < 32)
+ return (bpf_has_stack_frame(ctx) ? BPF_PPC_STACKFRAME : 0)
+ - (8 * (32 - reg));
+
+ pr_err("BPF JIT is asking about unknown registers");
+ BUG();
+}
+
static void bpf_jit_emit_skb_loads(u32 *image, struct codegen_context *ctx)
{
/*
@@ -100,9 +129,8 @@ static void bpf_jit_emit_func_call(u32 *image, struct codegen_context *ctx, u64
static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
{
int i;
- bool new_stack_frame = bpf_has_stack_frame(ctx);
- if (new_stack_frame) {
+ if (bpf_has_stack_frame(ctx)) {
/*
* We need a stack frame, but we don't necessarily need to
* save/restore LR unless we call other functions
@@ -122,9 +150,7 @@ static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
*/
for (i = BPF_REG_6; i <= BPF_REG_10; i++)
if (bpf_is_seen_register(ctx, i))
- PPC_BPF_STL(b2p[i], 1,
- (new_stack_frame ? BPF_PPC_STACKFRAME : 0) -
- (8 * (32 - b2p[i])));
+ PPC_BPF_STL(b2p[i], 1, bpf_jit_stack_offsetof(ctx, b2p[i]));
/*
* Save additional non-volatile regs if we cache skb
@@ -132,22 +158,21 @@ static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
*/
if (ctx->seen & SEEN_SKB) {
PPC_BPF_STL(b2p[SKB_HLEN_REG], 1,
- BPF_PPC_STACKFRAME - (8 * (32 - b2p[SKB_HLEN_REG])));
+ bpf_jit_stack_offsetof(ctx, b2p[SKB_HLEN_REG]));
PPC_BPF_STL(b2p[SKB_DATA_REG], 1,
- BPF_PPC_STACKFRAME - (8 * (32 - b2p[SKB_DATA_REG])));
+ bpf_jit_stack_offsetof(ctx, b2p[SKB_DATA_REG]));
bpf_jit_emit_skb_loads(image, ctx);
}
/* Setup frame pointer to point to the bpf stack area */
if (bpf_is_seen_register(ctx, BPF_REG_FP))
PPC_ADDI(b2p[BPF_REG_FP], 1,
- BPF_PPC_STACKFRAME - BPF_PPC_STACK_SAVE);
+ STACK_FRAME_MIN_SIZE + MAX_BPF_STACK);
}
static void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx)
{
int i;
- bool new_stack_frame = bpf_has_stack_frame(ctx);
/* Move result to r3 */
PPC_MR(3, b2p[BPF_REG_0]);
@@ -155,20 +180,18 @@ static void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx)
/* Restore NVRs */
for (i = BPF_REG_6; i <= BPF_REG_10; i++)
if (bpf_is_seen_register(ctx, i))
- PPC_BPF_LL(b2p[i], 1,
- (new_stack_frame ? BPF_PPC_STACKFRAME : 0) -
- (8 * (32 - b2p[i])));
+ PPC_BPF_LL(b2p[i], 1, bpf_jit_stack_offsetof(ctx, b2p[i]));
/* Restore non-volatile registers used for skb cache */
if (ctx->seen & SEEN_SKB) {
PPC_BPF_LL(b2p[SKB_HLEN_REG], 1,
- BPF_PPC_STACKFRAME - (8 * (32 - b2p[SKB_HLEN_REG])));
+ bpf_jit_stack_offsetof(ctx, b2p[SKB_HLEN_REG]));
PPC_BPF_LL(b2p[SKB_DATA_REG], 1,
- BPF_PPC_STACKFRAME - (8 * (32 - b2p[SKB_DATA_REG])));
+ bpf_jit_stack_offsetof(ctx, b2p[SKB_DATA_REG]));
}
/* Tear down our stack frame */
- if (new_stack_frame) {
+ if (bpf_has_stack_frame(ctx)) {
PPC_ADDI(1, 1, BPF_PPC_STACKFRAME);
if (ctx->seen & SEEN_FUNC) {
PPC_BPF_LL(0, 1, PPC_LR_STKOFF);
@@ -200,7 +223,6 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
u64 imm64;
u8 *func;
u32 true_cond;
- int stack_local_off;
/*
* addrs[] maps a BPF bytecode address into a real offset from
@@ -219,9 +241,9 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
* optimization but everything else should work without
* any issues.
*/
- if (dst_reg >= 24 && dst_reg <= 31)
+ if (dst_reg >= BPF_PPC_NVR_MIN && dst_reg < 32)
bpf_set_seen_register(ctx, insn[i].dst_reg);
- if (src_reg >= 24 && src_reg <= 31)
+ if (src_reg >= BPF_PPC_NVR_MIN && src_reg < 32)
bpf_set_seen_register(ctx, insn[i].src_reg);
switch (code) {
@@ -490,25 +512,12 @@ bpf_alu32_trunc:
* Way easier and faster(?) to store the value
* into stack and then use ldbrx
*
- * First, determine where in stack we can store
- * this:
- * - if we have allotted a stack frame, then we
- * will utilize the area set aside by
- * BPF_PPC_STACK_LOCALS
- * - else, we use the area beneath the NV GPR
- * save area
- *
* ctx->seen will be reliable in pass2, but
* the instructions generated will remain the
* same across all passes
*/
- if (bpf_has_stack_frame(ctx))
- stack_local_off = STACK_FRAME_MIN_SIZE;
- else
- stack_local_off = -(BPF_PPC_STACK_SAVE + 8);
-
- PPC_STD(dst_reg, 1, stack_local_off);
- PPC_ADDI(b2p[TMP_REG_1], 1, stack_local_off);
+ PPC_STD(dst_reg, 1, bpf_jit_stack_local(ctx));
+ PPC_ADDI(b2p[TMP_REG_1], 1, bpf_jit_stack_local(ctx));
PPC_LDBRX(dst_reg, 0, b2p[TMP_REG_1]);
break;
}
@@ -668,7 +677,7 @@ emit_clear:
/* Save skb pointer if we need to re-cache skb data */
if (bpf_helper_changes_skb_data(func))
- PPC_BPF_STL(3, 1, STACK_FRAME_MIN_SIZE);
+ PPC_BPF_STL(3, 1, bpf_jit_stack_local(ctx));
bpf_jit_emit_func_call(image, ctx, (u64)func);
@@ -678,7 +687,7 @@ emit_clear:
/* refresh skb cache */
if (bpf_helper_changes_skb_data(func)) {
/* reload skb pointer to r3 */
- PPC_BPF_LL(3, 1, STACK_FRAME_MIN_SIZE);
+ PPC_BPF_LL(3, 1, bpf_jit_stack_local(ctx));
bpf_jit_emit_skb_loads(image, ctx);
}
break;
--
2.9.3
^ permalink raw reply related
* [PATCH 2/3] bpf powerpc: implement support for tail calls
From: Naveen N. Rao @ 2016-09-23 20:35 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, netdev, Michael Ellerman
Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Ananth N Mavinakayanahalli
In-Reply-To: <40b65ab2bb3a48837ab047a70887de3ccd70c56b.1474661927.git.naveen.n.rao@linux.vnet.ibm.com>
Tail calls allow JIT'ed eBPF programs to call into other JIT'ed eBPF
programs. This can be achieved either by:
(1) retaining the stack setup by the first eBPF program and having all
subsequent eBPF programs re-using it, or,
(2) by unwinding/tearing down the stack and having each eBPF program
deal with its own stack as it sees fit.
To ensure that this does not create loops, there is a limit to how many
tail calls can be done (currently 32). This requires the JIT'ed code to
maintain a count of the number of tail calls done so far.
Approach (1) is simple, but requires every eBPF program to have (almost)
the same prologue/epilogue, regardless of whether they need it. This is
inefficient for small eBPF programs which may not sometimes need a
prologue at all. As such, to minimize impact of tail call
implementation, we use approach (2) here which needs each eBPF program
in the chain to use its own prologue/epilogue. This is not ideal when
many tail calls are involved and when all the eBPF programs in the chain
have similar prologue/epilogue. However, the impact is restricted to
programs that do tail calls. Individual eBPF programs are not affected.
We maintain the tail call count in a fixed location on the stack and
updated tail call count values are passed in through this. The very
first eBPF program in a chain sets this up to 0 (the first 2
instructions). Subsequent tail calls skip the first two eBPF JIT
instructions to maintain the count. For programs that don't do tail
calls themselves, the first two instructions are NOPs.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/ppc-opcode.h | 2 +
arch/powerpc/net/bpf_jit.h | 2 +
arch/powerpc/net/bpf_jit64.h | 1 +
arch/powerpc/net/bpf_jit_comp64.c | 149 +++++++++++++++++++++++++++-------
4 files changed, 126 insertions(+), 28 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 127ebf5..54ff8ce 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -236,6 +236,7 @@
#define PPC_INST_STWU 0x94000000
#define PPC_INST_MFLR 0x7c0802a6
#define PPC_INST_MTLR 0x7c0803a6
+#define PPC_INST_MTCTR 0x7c0903a6
#define PPC_INST_CMPWI 0x2c000000
#define PPC_INST_CMPDI 0x2c200000
#define PPC_INST_CMPW 0x7c000000
@@ -250,6 +251,7 @@
#define PPC_INST_SUB 0x7c000050
#define PPC_INST_BLR 0x4e800020
#define PPC_INST_BLRL 0x4e800021
+#define PPC_INST_BCTR 0x4e800420
#define PPC_INST_MULLD 0x7c0001d2
#define PPC_INST_MULLW 0x7c0001d6
#define PPC_INST_MULHWU 0x7c000016
diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index d5301b6..89f7007 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -40,6 +40,8 @@
#define PPC_BLR() EMIT(PPC_INST_BLR)
#define PPC_BLRL() EMIT(PPC_INST_BLRL)
#define PPC_MTLR(r) EMIT(PPC_INST_MTLR | ___PPC_RT(r))
+#define PPC_BCTR() EMIT(PPC_INST_BCTR)
+#define PPC_MTCTR(r) EMIT(PPC_INST_MTCTR | ___PPC_RT(r))
#define PPC_ADDI(d, a, i) EMIT(PPC_INST_ADDI | ___PPC_RT(d) | \
___PPC_RA(a) | IMM_L(i))
#define PPC_MR(d, a) PPC_OR(d, a, a)
diff --git a/arch/powerpc/net/bpf_jit64.h b/arch/powerpc/net/bpf_jit64.h
index a1645d7..038e00b 100644
--- a/arch/powerpc/net/bpf_jit64.h
+++ b/arch/powerpc/net/bpf_jit64.h
@@ -88,6 +88,7 @@ DECLARE_LOAD_FUNC(sk_load_byte);
#define SEEN_FUNC 0x1000 /* might call external helpers */
#define SEEN_STACK 0x2000 /* uses BPF stack */
#define SEEN_SKB 0x4000 /* uses sk_buff */
+#define SEEN_TAILCALL 0x8000 /* uses tail calls */
struct codegen_context {
/*
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 5f8c91f..3ec29d6 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -17,6 +17,7 @@
#include <linux/filter.h>
#include <linux/if_vlan.h>
#include <asm/kprobes.h>
+#include <linux/bpf.h>
#include "bpf_jit64.h"
@@ -77,6 +78,11 @@ static int bpf_jit_stack_local(struct codegen_context *ctx)
return -(BPF_PPC_STACK_SAVE + 16);
}
+static int bpf_jit_stack_tailcallcnt(struct codegen_context *ctx)
+{
+ return bpf_jit_stack_local(ctx) + 8;
+}
+
static int bpf_jit_stack_offsetof(struct codegen_context *ctx, int reg)
{
if (reg >= BPF_PPC_NVR_MIN && reg < 32)
@@ -102,33 +108,25 @@ static void bpf_jit_emit_skb_loads(u32 *image, struct codegen_context *ctx)
PPC_BPF_LL(b2p[SKB_DATA_REG], 3, offsetof(struct sk_buff, data));
}
-static void bpf_jit_emit_func_call(u32 *image, struct codegen_context *ctx, u64 func)
+static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
{
-#ifdef PPC64_ELF_ABI_v1
- /* func points to the function descriptor */
- PPC_LI64(b2p[TMP_REG_2], func);
- /* Load actual entry point from function descriptor */
- PPC_BPF_LL(b2p[TMP_REG_1], b2p[TMP_REG_2], 0);
- /* ... and move it to LR */
- PPC_MTLR(b2p[TMP_REG_1]);
+ int i;
+
/*
- * Load TOC from function descriptor at offset 8.
- * We can clobber r2 since we get called through a
- * function pointer (so caller will save/restore r2)
- * and since we don't use a TOC ourself.
+ * Initialize tail_call_cnt if we do tail calls.
+ * Otherwise, put in NOPs so that it can be skipped when we are
+ * invoked through a tail call.
*/
- PPC_BPF_LL(2, b2p[TMP_REG_2], 8);
-#else
- /* We can clobber r12 */
- PPC_FUNC_ADDR(12, func);
- PPC_MTLR(12);
-#endif
- PPC_BLRL();
-}
+ if (ctx->seen & SEEN_TAILCALL) {
+ PPC_LI(b2p[TMP_REG_1], 0);
+ /* this goes in the redzone */
+ PPC_BPF_STL(b2p[TMP_REG_1], 1, -(BPF_PPC_STACK_SAVE + 8));
+ } else {
+ PPC_NOP();
+ PPC_NOP();
+ }
-static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
-{
- int i;
+#define BPF_TAILCALL_PROLOGUE_SIZE 8
if (bpf_has_stack_frame(ctx)) {
/*
@@ -170,13 +168,10 @@ static void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
STACK_FRAME_MIN_SIZE + MAX_BPF_STACK);
}
-static void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx)
+static void bpf_jit_emit_common_epilogue(u32 *image, struct codegen_context *ctx)
{
int i;
- /* Move result to r3 */
- PPC_MR(3, b2p[BPF_REG_0]);
-
/* Restore NVRs */
for (i = BPF_REG_6; i <= BPF_REG_10; i++)
if (bpf_is_seen_register(ctx, i))
@@ -198,10 +193,105 @@ static void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx)
PPC_MTLR(0);
}
}
+}
+
+static void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx)
+{
+ bpf_jit_emit_common_epilogue(image, ctx);
+
+ /* Move result to r3 */
+ PPC_MR(3, b2p[BPF_REG_0]);
PPC_BLR();
}
+static void bpf_jit_emit_func_call(u32 *image, struct codegen_context *ctx, u64 func)
+{
+#ifdef PPC64_ELF_ABI_v1
+ /* func points to the function descriptor */
+ PPC_LI64(b2p[TMP_REG_2], func);
+ /* Load actual entry point from function descriptor */
+ PPC_BPF_LL(b2p[TMP_REG_1], b2p[TMP_REG_2], 0);
+ /* ... and move it to LR */
+ PPC_MTLR(b2p[TMP_REG_1]);
+ /*
+ * Load TOC from function descriptor at offset 8.
+ * We can clobber r2 since we get called through a
+ * function pointer (so caller will save/restore r2)
+ * and since we don't use a TOC ourself.
+ */
+ PPC_BPF_LL(2, b2p[TMP_REG_2], 8);
+#else
+ /* We can clobber r12 */
+ PPC_FUNC_ADDR(12, func);
+ PPC_MTLR(12);
+#endif
+ PPC_BLRL();
+}
+
+static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32 out)
+{
+ /*
+ * By now, the eBPF program has already setup parameters in r3, r4 and r5
+ * r3/BPF_REG_1 - pointer to ctx -- passed as is to the next bpf program
+ * r4/BPF_REG_2 - pointer to bpf_array
+ * r5/BPF_REG_3 - index in bpf_array
+ */
+ int b2p_bpf_array = b2p[BPF_REG_2];
+ int b2p_index = b2p[BPF_REG_3];
+
+ /*
+ * if (index >= array->map.max_entries)
+ * goto out;
+ */
+ PPC_LWZ(b2p[TMP_REG_1], b2p_bpf_array, offsetof(struct bpf_array, map.max_entries));
+ PPC_CMPLW(b2p_index, b2p[TMP_REG_1]);
+ PPC_BCC(COND_GE, out);
+
+ /*
+ * if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+ * goto out;
+ */
+ PPC_LD(b2p[TMP_REG_1], 1, bpf_jit_stack_tailcallcnt(ctx));
+ PPC_CMPLWI(b2p[TMP_REG_1], MAX_TAIL_CALL_CNT);
+ PPC_BCC(COND_GT, out);
+
+ /*
+ * tail_call_cnt++;
+ */
+ PPC_ADDI(b2p[TMP_REG_1], b2p[TMP_REG_1], 1);
+ PPC_BPF_STL(b2p[TMP_REG_1], 1, bpf_jit_stack_tailcallcnt(ctx));
+
+ /* prog = array->ptrs[index]; */
+ PPC_MULI(b2p[TMP_REG_1], b2p_index, 8);
+ PPC_ADD(b2p[TMP_REG_1], b2p[TMP_REG_1], b2p_bpf_array);
+ PPC_LD(b2p[TMP_REG_1], b2p[TMP_REG_1], offsetof(struct bpf_array, ptrs));
+
+ /*
+ * if (prog == NULL)
+ * goto out;
+ */
+ PPC_CMPLDI(b2p[TMP_REG_1], 0);
+ PPC_BCC(COND_EQ, out);
+
+ /* goto *(prog->bpf_func + prologue_size); */
+ PPC_LD(b2p[TMP_REG_1], b2p[TMP_REG_1], offsetof(struct bpf_prog, bpf_func));
+#ifdef PPC64_ELF_ABI_v1
+ /* skip past the function descriptor */
+ PPC_ADDI(b2p[TMP_REG_1], b2p[TMP_REG_1],
+ FUNCTION_DESCR_SIZE + BPF_TAILCALL_PROLOGUE_SIZE);
+#else
+ PPC_ADDI(b2p[TMP_REG_1], b2p[TMP_REG_1], BPF_TAILCALL_PROLOGUE_SIZE);
+#endif
+ PPC_MTCTR(b2p[TMP_REG_1]);
+
+ /* tear down stack, restore NVRs, ... */
+ bpf_jit_emit_common_epilogue(image, ctx);
+
+ PPC_BCTR();
+ /* out: */
+}
+
/* Assemble the body code between the prologue & epilogue */
static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
struct codegen_context *ctx,
@@ -846,9 +936,12 @@ common_load:
break;
/*
- * TODO: Tail call
+ * Tail call
*/
case BPF_JMP | BPF_CALL | BPF_X:
+ ctx->seen |= SEEN_TAILCALL;
+ bpf_jit_emit_tail_call(image, ctx, addrs[i + 1]);
+ break;
default:
/*
--
2.9.3
^ permalink raw reply related
* [PATCH 3/3] bpf powerpc: add support for bpf constant blinding
From: Naveen N. Rao @ 2016-09-23 20:35 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, netdev, Michael Ellerman
Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Ananth N Mavinakayanahalli
In-Reply-To: <40b65ab2bb3a48837ab047a70887de3ccd70c56b.1474661927.git.naveen.n.rao@linux.vnet.ibm.com>
In line with similar support for other architectures by Daniel Borkmann.
'MOD Default X' from test_bpf without constant blinding:
84 bytes emitted from JIT compiler (pass:3, flen:7)
d0000000058a4688 + <x>:
0: nop
4: nop
8: std r27,-40(r1)
c: std r28,-32(r1)
10: xor r8,r8,r8
14: xor r28,r28,r28
18: mr r27,r3
1c: li r8,66
20: cmpwi r28,0
24: bne 0x0000000000000030
28: li r8,0
2c: b 0x0000000000000044
30: divwu r9,r8,r28
34: mullw r9,r28,r9
38: subf r8,r9,r8
3c: rotlwi r8,r8,0
40: li r8,66
44: ld r27,-40(r1)
48: ld r28,-32(r1)
4c: mr r3,r8
50: blr
... and with constant blinding:
140 bytes emitted from JIT compiler (pass:3, flen:11)
d00000000bd6ab24 + <x>:
0: nop
4: nop
8: std r27,-40(r1)
c: std r28,-32(r1)
10: xor r8,r8,r8
14: xor r28,r28,r28
18: mr r27,r3
1c: lis r2,-22834
20: ori r2,r2,36083
24: rotlwi r2,r2,0
28: xori r2,r2,36017
2c: xoris r2,r2,42702
30: rotlwi r2,r2,0
34: mr r8,r2
38: rotlwi r8,r8,0
3c: cmpwi r28,0
40: bne 0x000000000000004c
44: li r8,0
48: b 0x000000000000007c
4c: divwu r9,r8,r28
50: mullw r9,r28,r9
54: subf r8,r9,r8
58: rotlwi r8,r8,0
5c: lis r2,-17137
60: ori r2,r2,39065
64: rotlwi r2,r2,0
68: xori r2,r2,39131
6c: xoris r2,r2,48399
70: rotlwi r2,r2,0
74: mr r8,r2
78: rotlwi r8,r8,0
7c: ld r27,-40(r1)
80: ld r28,-32(r1)
84: mr r3,r8
88: blr
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/net/bpf_jit64.h | 9 +++++----
arch/powerpc/net/bpf_jit_comp64.c | 36 +++++++++++++++++++++++++++++-------
2 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit64.h b/arch/powerpc/net/bpf_jit64.h
index 038e00b..62fa758 100644
--- a/arch/powerpc/net/bpf_jit64.h
+++ b/arch/powerpc/net/bpf_jit64.h
@@ -39,10 +39,10 @@
#ifndef __ASSEMBLY__
/* BPF register usage */
-#define SKB_HLEN_REG (MAX_BPF_REG + 0)
-#define SKB_DATA_REG (MAX_BPF_REG + 1)
-#define TMP_REG_1 (MAX_BPF_REG + 2)
-#define TMP_REG_2 (MAX_BPF_REG + 3)
+#define SKB_HLEN_REG (MAX_BPF_JIT_REG + 0)
+#define SKB_DATA_REG (MAX_BPF_JIT_REG + 1)
+#define TMP_REG_1 (MAX_BPF_JIT_REG + 2)
+#define TMP_REG_2 (MAX_BPF_JIT_REG + 3)
/* BPF to ppc register mappings */
static const int b2p[] = {
@@ -62,6 +62,7 @@ static const int b2p[] = {
/* frame pointer aka BPF_REG_10 */
[BPF_REG_FP] = 31,
/* eBPF jit internal registers */
+ [BPF_REG_AX] = 2,
[SKB_HLEN_REG] = 25,
[SKB_DATA_REG] = 26,
[TMP_REG_1] = 9,
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 3ec29d6..0fe98a5 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -974,21 +974,37 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
int pass;
int flen;
struct bpf_binary_header *bpf_hdr;
+ struct bpf_prog *org_fp = fp;
+ struct bpf_prog *tmp_fp;
+ bool bpf_blinded = false;
if (!bpf_jit_enable)
- return fp;
+ return org_fp;
+
+ tmp_fp = bpf_jit_blind_constants(org_fp);
+ if (IS_ERR(tmp_fp))
+ return org_fp;
+
+ if (tmp_fp != org_fp) {
+ bpf_blinded = true;
+ fp = tmp_fp;
+ }
flen = fp->len;
addrs = kzalloc((flen+1) * sizeof(*addrs), GFP_KERNEL);
- if (addrs == NULL)
- return fp;
+ if (addrs == NULL) {
+ fp = org_fp;
+ goto out;
+ }
+
+ memset(&cgctx, 0, sizeof(struct codegen_context));
- cgctx.idx = 0;
- cgctx.seen = 0;
/* Scouting faux-generate pass 0 */
- if (bpf_jit_build_body(fp, 0, &cgctx, addrs))
+ if (bpf_jit_build_body(fp, 0, &cgctx, addrs)) {
/* We hit something illegal or unsupported. */
+ fp = org_fp;
goto out;
+ }
/*
* Pretend to build prologue, given the features we've seen. This will
@@ -1003,8 +1019,10 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
bpf_hdr = bpf_jit_binary_alloc(alloclen, &image, 4,
bpf_jit_fill_ill_insns);
- if (!bpf_hdr)
+ if (!bpf_hdr) {
+ fp = org_fp;
goto out;
+ }
code_base = (u32 *)(image + FUNCTION_DESCR_SIZE);
@@ -1041,6 +1059,10 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
out:
kfree(addrs);
+
+ if (bpf_blinded)
+ bpf_jit_prog_release_other(fp, fp == org_fp ? tmp_fp : org_fp);
+
return fp;
}
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net v2] ip6_gre: fix flowi6_proto value in ip6gre_xmit_other()
From: Lance Richardson @ 2016-09-23 20:37 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev, shmulik ladkani, jbenc
In-Reply-To: <53fca902-c7e6-c216-6366-ba3f483854a0@cogentembedded.com>
> From: "Sergei Shtylyov" <sergei.shtylyov@cogentembedded.com>
> To: "Lance Richardson" <lrichard@redhat.com>, netdev@vger.kernel.org
> Cc: "shmulik ladkani" <shmulik.ladkani@gmail.com>, jbenc@redhat.com
> Sent: Friday, September 23, 2016 4:01:15 PM
> Subject: Re: [PATCH net v2] ip6_gre: fix flowi6_proto value in ip6gre_xmit_other()
>
> Hello.
>
> On 09/23/2016 10:50 PM, Lance Richardson wrote:
>
> > Similar to commit 3be07244b733 ("ip6_gre: fix flowi6_proto value in
> > xmit path"), set flowi6_proto to IPPROTO_GRE for output route lookup.
> >
> > Up until now, ip6gre_xmit_other() has set flowi6_proto to a bogus value.
> > This affected output route lookup for packets sent on an ip6gretap device
> > in cases where routing was dependent on the value of flowi6_proto.
> >
> > Since the correct proto is already set in the tunnel flowi6 template via
> > commit 252f3f5a1189 ("ip6_gre: Set flowi6_proto as IPPROTO_GRE in xmit
> > path."), simply delete the line setting the incorrect flowi6_proto value.
> >
> > Suggested-by: Jiri Benc <jbenc@redhat.com>
> > Fixes: commit c12b395a4664 ("gre: Support GRE over IPv6")
>
> That "commit" isn't needed here, this tag has a standardized format.
> Hopefully, can be fixed while applying...
Thanks for pointing that out, I mistakenly added that "commit" after
checkpatch.pl complained about not having "commit" before the hashes
in the log. Hoping it can be fixed when applying as well.
>
> > Reviewed-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> > Signed-off-by: Lance Richardson <lrichard@redhat.com>
> [...]
>
> MBR, Sergei
>
>
^ permalink raw reply
* [PATCH 1/2] bpf samples: fix compiler errors with sockex2 and sockex3
From: Naveen N. Rao @ 2016-09-23 20:40 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, netdev, David S. Miller
Cc: Michael Ellerman, Alexei Starovoitov, Daniel Borkmann,
Ananth N Mavinakayanahalli
These samples fail to compile as 'struct flow_keys' conflicts with
definition in net/flow_dissector.h. Fix the same by renaming the
structure used in the sample.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
samples/bpf/sockex2_kern.c | 10 +++++-----
samples/bpf/sockex3_kern.c | 8 ++++----
samples/bpf/sockex3_user.c | 4 ++--
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/samples/bpf/sockex2_kern.c b/samples/bpf/sockex2_kern.c
index ba0e177..44e5846 100644
--- a/samples/bpf/sockex2_kern.c
+++ b/samples/bpf/sockex2_kern.c
@@ -14,7 +14,7 @@ struct vlan_hdr {
__be16 h_vlan_encapsulated_proto;
};
-struct flow_keys {
+struct bpf_flow_keys {
__be32 src;
__be32 dst;
union {
@@ -59,7 +59,7 @@ static inline __u32 ipv6_addr_hash(struct __sk_buff *ctx, __u64 off)
}
static inline __u64 parse_ip(struct __sk_buff *skb, __u64 nhoff, __u64 *ip_proto,
- struct flow_keys *flow)
+ struct bpf_flow_keys *flow)
{
__u64 verlen;
@@ -83,7 +83,7 @@ static inline __u64 parse_ip(struct __sk_buff *skb, __u64 nhoff, __u64 *ip_proto
}
static inline __u64 parse_ipv6(struct __sk_buff *skb, __u64 nhoff, __u64 *ip_proto,
- struct flow_keys *flow)
+ struct bpf_flow_keys *flow)
{
*ip_proto = load_byte(skb,
nhoff + offsetof(struct ipv6hdr, nexthdr));
@@ -96,7 +96,7 @@ static inline __u64 parse_ipv6(struct __sk_buff *skb, __u64 nhoff, __u64 *ip_pro
return nhoff;
}
-static inline bool flow_dissector(struct __sk_buff *skb, struct flow_keys *flow)
+static inline bool flow_dissector(struct __sk_buff *skb, struct bpf_flow_keys *flow)
{
__u64 nhoff = ETH_HLEN;
__u64 ip_proto;
@@ -198,7 +198,7 @@ struct bpf_map_def SEC("maps") hash_map = {
SEC("socket2")
int bpf_prog2(struct __sk_buff *skb)
{
- struct flow_keys flow;
+ struct bpf_flow_keys flow;
struct pair *value;
u32 key;
diff --git a/samples/bpf/sockex3_kern.c b/samples/bpf/sockex3_kern.c
index 41ae2fd..95907f8 100644
--- a/samples/bpf/sockex3_kern.c
+++ b/samples/bpf/sockex3_kern.c
@@ -61,7 +61,7 @@ struct vlan_hdr {
__be16 h_vlan_encapsulated_proto;
};
-struct flow_keys {
+struct bpf_flow_keys {
__be32 src;
__be32 dst;
union {
@@ -88,7 +88,7 @@ static inline __u32 ipv6_addr_hash(struct __sk_buff *ctx, __u64 off)
}
struct globals {
- struct flow_keys flow;
+ struct bpf_flow_keys flow;
};
struct bpf_map_def SEC("maps") percpu_map = {
@@ -114,14 +114,14 @@ struct pair {
struct bpf_map_def SEC("maps") hash_map = {
.type = BPF_MAP_TYPE_HASH,
- .key_size = sizeof(struct flow_keys),
+ .key_size = sizeof(struct bpf_flow_keys),
.value_size = sizeof(struct pair),
.max_entries = 1024,
};
static void update_stats(struct __sk_buff *skb, struct globals *g)
{
- struct flow_keys key = g->flow;
+ struct bpf_flow_keys key = g->flow;
struct pair *value;
value = bpf_map_lookup_elem(&hash_map, &key);
diff --git a/samples/bpf/sockex3_user.c b/samples/bpf/sockex3_user.c
index d4184ab..3fcfd8c4 100644
--- a/samples/bpf/sockex3_user.c
+++ b/samples/bpf/sockex3_user.c
@@ -7,7 +7,7 @@
#include <arpa/inet.h>
#include <sys/resource.h>
-struct flow_keys {
+struct bpf_flow_keys {
__be32 src;
__be32 dst;
union {
@@ -49,7 +49,7 @@ int main(int argc, char **argv)
(void) f;
for (i = 0; i < 5; i++) {
- struct flow_keys key = {}, next_key;
+ struct bpf_flow_keys key = {}, next_key;
struct pair value;
sleep(1);
--
2.9.3
^ permalink raw reply related
* [PATCH 2/2] bpf samples: update tracex5 sample to use __seccomp_filter
From: Naveen N. Rao @ 2016-09-23 20:40 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, netdev, David S. Miller
Cc: Michael Ellerman, Alexei Starovoitov, Daniel Borkmann,
Ananth N Mavinakayanahalli
In-Reply-To: <7b29fba6073924dc4c9e7d639eefb0b15a98660d.1474661952.git.naveen.n.rao@linux.vnet.ibm.com>
seccomp_phase1() does not exist anymore. Instead, update sample to use
__seccomp_filter(). While at it, set max locked memory to unlimited.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
I am not completely sure if __seccomp_filter is the right place to hook
in. This works for me though. Please review.
Thanks,
Naveen
samples/bpf/tracex5_kern.c | 16 +++++++---------
samples/bpf/tracex5_user.c | 3 +++
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/samples/bpf/tracex5_kern.c b/samples/bpf/tracex5_kern.c
index f95f232..fd12d71 100644
--- a/samples/bpf/tracex5_kern.c
+++ b/samples/bpf/tracex5_kern.c
@@ -19,20 +19,18 @@ struct bpf_map_def SEC("maps") progs = {
.max_entries = 1024,
};
-SEC("kprobe/seccomp_phase1")
+SEC("kprobe/__seccomp_filter")
int bpf_prog1(struct pt_regs *ctx)
{
- struct seccomp_data sd;
-
- bpf_probe_read(&sd, sizeof(sd), (void *)PT_REGS_PARM1(ctx));
+ int sc_nr = (int)PT_REGS_PARM1(ctx);
/* dispatch into next BPF program depending on syscall number */
- bpf_tail_call(ctx, &progs, sd.nr);
+ bpf_tail_call(ctx, &progs, sc_nr);
/* fall through -> unknown syscall */
- if (sd.nr >= __NR_getuid && sd.nr <= __NR_getsid) {
+ if (sc_nr >= __NR_getuid && sc_nr <= __NR_getsid) {
char fmt[] = "syscall=%d (one of get/set uid/pid/gid)\n";
- bpf_trace_printk(fmt, sizeof(fmt), sd.nr);
+ bpf_trace_printk(fmt, sizeof(fmt), sc_nr);
}
return 0;
}
@@ -42,7 +40,7 @@ PROG(__NR_write)(struct pt_regs *ctx)
{
struct seccomp_data sd;
- bpf_probe_read(&sd, sizeof(sd), (void *)PT_REGS_PARM1(ctx));
+ bpf_probe_read(&sd, sizeof(sd), (void *)PT_REGS_PARM2(ctx));
if (sd.args[2] == 512) {
char fmt[] = "write(fd=%d, buf=%p, size=%d)\n";
bpf_trace_printk(fmt, sizeof(fmt),
@@ -55,7 +53,7 @@ PROG(__NR_read)(struct pt_regs *ctx)
{
struct seccomp_data sd;
- bpf_probe_read(&sd, sizeof(sd), (void *)PT_REGS_PARM1(ctx));
+ bpf_probe_read(&sd, sizeof(sd), (void *)PT_REGS_PARM2(ctx));
if (sd.args[2] > 128 && sd.args[2] <= 1024) {
char fmt[] = "read(fd=%d, buf=%p, size=%d)\n";
bpf_trace_printk(fmt, sizeof(fmt),
diff --git a/samples/bpf/tracex5_user.c b/samples/bpf/tracex5_user.c
index a04dd3c..36b5925 100644
--- a/samples/bpf/tracex5_user.c
+++ b/samples/bpf/tracex5_user.c
@@ -6,6 +6,7 @@
#include <sys/prctl.h>
#include "libbpf.h"
#include "bpf_load.h"
+#include <sys/resource.h>
/* install fake seccomp program to enable seccomp code path inside the kernel,
* so that our kprobe attached to seccomp_phase1() can be triggered
@@ -27,8 +28,10 @@ int main(int ac, char **argv)
{
FILE *f;
char filename[256];
+ struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+ setrlimit(RLIMIT_MEMLOCK, &r);
if (load_bpf_file(filename)) {
printf("%s", bpf_log_buf);
--
2.9.3
^ permalink raw reply related
* Re: [RFC] net: store port/representative id in metadata_dst
From: Jakub Kicinski @ 2016-09-23 20:45 UTC (permalink / raw)
To: John Fastabend
Cc: Jakub Kicinski, Samudrala, Sridhar, Jiri Benc, Jiri Pirko, netdev,
Thomas Graf, Roopa Prabhu, ogerlitz, ast, daniel, simon.horman,
Paolo Abeni, Pravin B Shelar, hannes
In-Reply-To: <57E58FA6.3050001@gmail.com>
On Fri, 23 Sep 2016 13:25:10 -0700, John Fastabend wrote:
> On 16-09-23 01:17 PM, Jakub Kicinski wrote:
> > On Fri, 23 Sep 2016 10:22:59 -0700, Samudrala, Sridhar wrote:
> >> On 9/23/2016 8:29 AM, Jakub Kicinski wrote:
> [...]
> [...]
> >>
> >> The 'accel' parameter in dev_queue_xmit_accel() is currently only passed
> >> to ndo_select_queue() via netdev_pick_tx() and is used to select the tx
> >> queue.
> >> Also, it is not passed all the way to the driver specific xmit routine.
> >> Doesn't it require
> >> changing all the driver xmit routines if we want to pass this parameter?
> >>
> [...]
> >>
> >> Yes. The VFPR netdevs don't have any HW queues associated with them and
> >> we would like
> >> to use the PF queues for the xmit.
> >> I was also looking into some way of passing the port id via skb
> >> parameter to the
> >> dev_queue_xmit() call so that the PF xmit routine can do a directed
> >> transmit to a specifc VF.
> >> Is skb->cb an option to pass this info?
> >> dst_metadata approach would work too if it is acceptable.
> >
> > I don't think we can trust skb->cb to be set to anything meaningful
> > when the skb is received by the lower device.
>
> Agreed. I wouldn't recommend using skb->cb. How about passing it through
> dev_queue_xmit_accel() through to the driver?
>
> If you pass the metadata through the dev_queue_xmit_accel() handle tx
> queue selection would work using normal mechanisms (xps, select_queue,
> cls hook, etc.). If you wanted to pick some specific queue based on
> policy the policy could be loaded into one of those hooks.
Do you mean without extending how accel is handled by
dev_queue_xmit_accel() today? If my goal is to not have extra HW
queues then I don't see how I could mux in the lower dev without extra
locking (as I tried to explain two emails ago). Sorry for being slow
here :(
^ permalink raw reply
* Re: [PATCH] net: bcmgenet: Fix EPHY reset in power up
From: Jaedon Shin @ 2016-09-23 20:55 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, David Miller, Philippe Reynes, netdev
In-Reply-To: <f68f0b8c-df49-1326-c05f-ddd7f95e4209@gmail.com>
Hi Florian,
> On 24 Sep 2016, at 1:54 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 09/23/2016 08:04 AM, Jaedon Shin wrote:
>> Hi Andrew,
>>
>> On 23 Sep 2016, at 11:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>>>
>>> On Fri, Sep 23, 2016 at 10:20:04PM +0900, Jaedon Shin wrote:
>>>> The bcmgenet_mii_reset() is always not running in power up sequence
>>>> after 'commit 62469c76007e ("net: ethernet: bcmgenet: use phydev from
>>>> struct net_device")'. This'll show extremely high latency and duplicate
>>>> packets while interface down and up repeatedly.
>>>>
>>>> For now, adds again a private phydev for mii reset when runs power up to
>>>> open interface.
>>>
>>> Hi Jaedon
>>>
>>> How does this fix the issue? It sounds like you are papering over the
>>> crack, not truly fixing it.
>>>
>>> Andrew
>>
>> Yes, It feel like a workaround, but I think it must need v4.8 stable
>> version. If we find better way that fixes internal PHY to initialize
>> after re-open interface, this patch will be dropped.
>
> I can observe the faulting behavior with 4.8-rc7 that the link below
> fixed initially:
>
> # ping fainelli-linux
> PING fainelli-linux (10.112.156.244): 56 data bytes
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.352 ms
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.472 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.496 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.517 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.536 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=1.557 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=752.448 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=2 ttl=61 time=1.291 ms
> 64 bytes from 10.112.156.244: seq=2 ttl=61 time=1.421 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=2 ttl=61 time=1.444 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=2 ttl=61 time=1.464 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=2 ttl=61 time=1.483 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=2 ttl=61 time=1.505 ms (DUP!)
> 64 bytes from 10.112.156.244: seq=2 ttl=61 time=24.964 ms (DUP!)
>
> If we revert this patch, we indeed get the normal and expected behavior
> back:
>
> # ping fainelli-linux
> PING fainelli-linux (10.112.156.244): 56 data bytes
> 64 bytes from 10.112.156.244: seq=0 ttl=61 time=0.417 ms
> 64 bytes from 10.112.156.244: seq=1 ttl=61 time=0.415 ms
> 64 bytes from 10.112.156.244: seq=2 ttl=61 time=0.424 ms
>
> Actually, the key thing is this:
>
> - without Philippe's patch we call twice bcmgenet_mii_reset, and that is
> intended:
> - first time from bcmgenet_power_up() to make sure the PHY is
> initialized *before* we get to initialize the UniMAC, this is critical
> - second time from bcmgenet_mii_probe(), through the normal phy_init_hw()
>
> - with Philippe's patch, we only get to call bcmgenet_mii_reset once, in
> bcmgenet_mii_probe() because the first time in bcmgenet_power_up(),
> dev->phydev is NULL, because of a prior call to phy_disconnect() in
> bcmgenet_close(), unfortunately, there has been MAC activity, so the PHY
> gets in a bad state
>
> Jaedon, feel free to use the explanation above, and send a plain revert
> of commit 62469c76007e11428e2ee3c6de90cbe74b588d44.
>
Will send revert patch.
Thanks,
Jaedon
> Thanks!
>
> Thanks!
> --
> Florian
^ permalink raw reply
* Re: [net-next 5/5] PCI: disable FLR for 82579 device
From: Jeff Kirsher @ 2016-09-23 21:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: davem, bhelgaas, Sasha Neftin, netdev, nhorman, sassmann,
jogreene, guru.anbalagane, linux-pci
In-Reply-To: <20160923140136.GC1514@localhost>
[-- Attachment #1: Type: text/plain, Size: 3107 bytes --]
On Fri, 2016-09-23 at 09:01 -0500, Bjorn Helgaas wrote:
> On Thu, Sep 22, 2016 at 11:39:01PM -0700, Jeff Kirsher wrote:
> >
> > From: Sasha Neftin <sasha.neftin@intel.com>
> >
> > 82579 has a problem reattaching itself after the device is detached.
> > The bug was reported by Redhat. The suggested fix is to disable
> > FLR capability in PCIe configuration space.
> >
> > Reproduction:
> > Attach the device to a VM, then detach and try to attach again.
> >
> > Fix:
> > Disable FLR capability to prevent the 82579 from hanging.
>
> Is there a bugzilla or other reference URL to include here? Should
> this be marked for stable?
So the author is in Israel, meaning it is their weekend now. I do not
believe Sasha monitors email over the weekend, so a response to your
questions won't happen for a few days.
I tried searching my archives for more information, but had no luck finding
any additional information.
> > Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
> > Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> > drivers/pci/quirks.c | 21 +++++++++++++++++++++
> > 1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 44e0ff3..59fba6e 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -4431,3 +4431,24 @@ static void quirk_intel_qat_vf_cap(struct
> > pci_dev *pdev)
> > }
> > }
> > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443,
> > quirk_intel_qat_vf_cap);
> > +/*
> > + * Workaround FLR issues for 82579
> > + * This code disables the FLR (Function Level Reset) via PCIe, in
> > order
> > + * to workaround a bug found while using device passthrough, where the
> > + * interface would become non-responsive.
> > + * NOTE: the FLR bit is Read/Write Once (RWO) in config space, so if
> > + * the BIOS or kernel writes this register * then this workaround will
> > + * not work.
>
> This doesn't sound like a root cause. Is the issue a hardware
> erratum? Linux PCI core bug? VFIO bug? Device firmware bug?
>
> The changelog suggests that the problem only affects passthrough,
> which suggests some sort of kernel bug related to how passthrough is
> implemented.
>
> >
> > + */
> > +static void quirk_intel_flr_cap_dis(struct pci_dev *dev)
> > +{
> > + int pos = pci_find_capability(dev, PCI_CAP_ID_AF);
> > + if (pos) {
> > + u8 cap;
> > + pci_read_config_byte(dev, pos + PCI_AF_CAP, &cap);
> > + cap = cap & (~PCI_AF_CAP_FLR);
> > + pci_write_config_byte(dev, pos + PCI_AF_CAP, cap);
> > + }
> > +}
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1502,
> > quirk_intel_flr_cap_dis);
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1503,
> > quirk_intel_flr_cap_dis);
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] Revert "net: ethernet: bcmgenet: use phydev from struct net_device"
From: Jaedon Shin @ 2016-09-23 21:08 UTC (permalink / raw)
To: Florian Fainelli, David S . Miller
Cc: Philippe Reynes, Andrew Lunn, netdev, Jaedon Shin
This reverts commit 62469c76007e ("net: ethernet: bcmgenet: use phydev
from struct net_device")
without this patch, we call twice bcmgenet_mii_reset, and that is intended:
- first time from bcmgenet_power_up() to make sure the PHY is initialized
*before* we get to initialize the UniMAC, this is critical
- second time from bcmgenet_mii_probe(), through the normal phy_init_hw()
with this patch, we only get to call bcmgenet_mii_reset once, in
bcmgenet_mii_probe() because the first time in bcmgenet_power_up(),
dev->phydev is NULL, because of a prior call to phy_disconnect() in
bcmgenet_close(), unfortunately, there has been MAC activity, so the PHY
gets in a bad state
Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 45 ++++++++++++++------------
drivers/net/ethernet/broadcom/genet/bcmgenet.h | 1 +
drivers/net/ethernet/broadcom/genet/bcmmii.c | 24 +++++++-------
3 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 8d4f8495dbb3..541456398dfb 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -453,25 +453,29 @@ static inline void bcmgenet_rdma_ring_writel(struct bcmgenet_priv *priv,
static int bcmgenet_get_settings(struct net_device *dev,
struct ethtool_cmd *cmd)
{
+ struct bcmgenet_priv *priv = netdev_priv(dev);
+
if (!netif_running(dev))
return -EINVAL;
- if (!dev->phydev)
+ if (!priv->phydev)
return -ENODEV;
- return phy_ethtool_gset(dev->phydev, cmd);
+ return phy_ethtool_gset(priv->phydev, cmd);
}
static int bcmgenet_set_settings(struct net_device *dev,
struct ethtool_cmd *cmd)
{
+ struct bcmgenet_priv *priv = netdev_priv(dev);
+
if (!netif_running(dev))
return -EINVAL;
- if (!dev->phydev)
+ if (!priv->phydev)
return -ENODEV;
- return phy_ethtool_sset(dev->phydev, cmd);
+ return phy_ethtool_sset(priv->phydev, cmd);
}
static int bcmgenet_set_rx_csum(struct net_device *dev,
@@ -937,7 +941,7 @@ static int bcmgenet_get_eee(struct net_device *dev, struct ethtool_eee *e)
e->eee_active = p->eee_active;
e->tx_lpi_timer = bcmgenet_umac_readl(priv, UMAC_EEE_LPI_TIMER);
- return phy_ethtool_get_eee(dev->phydev, e);
+ return phy_ethtool_get_eee(priv->phydev, e);
}
static int bcmgenet_set_eee(struct net_device *dev, struct ethtool_eee *e)
@@ -954,7 +958,7 @@ static int bcmgenet_set_eee(struct net_device *dev, struct ethtool_eee *e)
if (!p->eee_enabled) {
bcmgenet_eee_enable_set(dev, false);
} else {
- ret = phy_init_eee(dev->phydev, 0);
+ ret = phy_init_eee(priv->phydev, 0);
if (ret) {
netif_err(priv, hw, dev, "EEE initialization failed\n");
return ret;
@@ -964,12 +968,14 @@ static int bcmgenet_set_eee(struct net_device *dev, struct ethtool_eee *e)
bcmgenet_eee_enable_set(dev, true);
}
- return phy_ethtool_set_eee(dev->phydev, e);
+ return phy_ethtool_set_eee(priv->phydev, e);
}
static int bcmgenet_nway_reset(struct net_device *dev)
{
- return genphy_restart_aneg(dev->phydev);
+ struct bcmgenet_priv *priv = netdev_priv(dev);
+
+ return genphy_restart_aneg(priv->phydev);
}
/* standard ethtool support functions. */
@@ -996,13 +1002,12 @@ static struct ethtool_ops bcmgenet_ethtool_ops = {
static int bcmgenet_power_down(struct bcmgenet_priv *priv,
enum bcmgenet_power_mode mode)
{
- struct net_device *ndev = priv->dev;
int ret = 0;
u32 reg;
switch (mode) {
case GENET_POWER_CABLE_SENSE:
- phy_detach(ndev->phydev);
+ phy_detach(priv->phydev);
break;
case GENET_POWER_WOL_MAGIC:
@@ -1063,6 +1068,7 @@ static void bcmgenet_power_up(struct bcmgenet_priv *priv,
/* ioctl handle special commands that are not present in ethtool. */
static int bcmgenet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
{
+ struct bcmgenet_priv *priv = netdev_priv(dev);
int val = 0;
if (!netif_running(dev))
@@ -1072,10 +1078,10 @@ static int bcmgenet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
case SIOCGMIIPHY:
case SIOCGMIIREG:
case SIOCSMIIREG:
- if (!dev->phydev)
+ if (!priv->phydev)
val = -ENODEV;
else
- val = phy_mii_ioctl(dev->phydev, rq, cmd);
+ val = phy_mii_ioctl(priv->phydev, rq, cmd);
break;
default:
@@ -2458,7 +2464,6 @@ static void bcmgenet_irq_task(struct work_struct *work)
{
struct bcmgenet_priv *priv = container_of(
work, struct bcmgenet_priv, bcmgenet_irq_work);
- struct net_device *ndev = priv->dev;
netif_dbg(priv, intr, priv->dev, "%s\n", __func__);
@@ -2471,7 +2476,7 @@ static void bcmgenet_irq_task(struct work_struct *work)
/* Link UP/DOWN event */
if (priv->irq0_stat & UMAC_IRQ_LINK_EVENT) {
- phy_mac_interrupt(ndev->phydev,
+ phy_mac_interrupt(priv->phydev,
!!(priv->irq0_stat & UMAC_IRQ_LINK_UP));
priv->irq0_stat &= ~UMAC_IRQ_LINK_EVENT;
}
@@ -2833,7 +2838,7 @@ static void bcmgenet_netif_start(struct net_device *dev)
/* Monitor link interrupts now */
bcmgenet_link_intr_enable(priv);
- phy_start(dev->phydev);
+ phy_start(priv->phydev);
}
static int bcmgenet_open(struct net_device *dev)
@@ -2932,7 +2937,7 @@ static void bcmgenet_netif_stop(struct net_device *dev)
struct bcmgenet_priv *priv = netdev_priv(dev);
netif_tx_stop_all_queues(dev);
- phy_stop(dev->phydev);
+ phy_stop(priv->phydev);
bcmgenet_intr_disable(priv);
bcmgenet_disable_rx_napi(priv);
bcmgenet_disable_tx_napi(priv);
@@ -2958,7 +2963,7 @@ static int bcmgenet_close(struct net_device *dev)
bcmgenet_netif_stop(dev);
/* Really kill the PHY state machine and disconnect from it */
- phy_disconnect(dev->phydev);
+ phy_disconnect(priv->phydev);
/* Disable MAC receive */
umac_enable_set(priv, CMD_RX_EN, false);
@@ -3517,7 +3522,7 @@ static int bcmgenet_suspend(struct device *d)
bcmgenet_netif_stop(dev);
- phy_suspend(dev->phydev);
+ phy_suspend(priv->phydev);
netif_device_detach(dev);
@@ -3581,7 +3586,7 @@ static int bcmgenet_resume(struct device *d)
if (priv->wolopts)
clk_disable_unprepare(priv->clk_wol);
- phy_init_hw(dev->phydev);
+ phy_init_hw(priv->phydev);
/* Speed settings must be restored */
bcmgenet_mii_config(priv->dev);
@@ -3614,7 +3619,7 @@ static int bcmgenet_resume(struct device *d)
netif_device_attach(dev);
- phy_resume(dev->phydev);
+ phy_resume(priv->phydev);
if (priv->eee.eee_enabled)
bcmgenet_eee_enable_set(dev, true);
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 0f0868c56f05..1e2dc34d331a 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -597,6 +597,7 @@ struct bcmgenet_priv {
/* MDIO bus variables */
wait_queue_head_t wq;
+ struct phy_device *phydev;
bool internal_phy;
struct device_node *phy_dn;
struct device_node *mdio_dn;
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index e907acd81da9..457c3bc8cfff 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -86,7 +86,7 @@ static int bcmgenet_mii_write(struct mii_bus *bus, int phy_id,
void bcmgenet_mii_setup(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
- struct phy_device *phydev = dev->phydev;
+ struct phy_device *phydev = priv->phydev;
u32 reg, cmd_bits = 0;
bool status_changed = false;
@@ -183,9 +183,9 @@ void bcmgenet_mii_reset(struct net_device *dev)
if (GENET_IS_V4(priv))
return;
- if (dev->phydev) {
- phy_init_hw(dev->phydev);
- phy_start_aneg(dev->phydev);
+ if (priv->phydev) {
+ phy_init_hw(priv->phydev);
+ phy_start_aneg(priv->phydev);
}
}
@@ -236,7 +236,6 @@ static void bcmgenet_internal_phy_setup(struct net_device *dev)
static void bcmgenet_moca_phy_setup(struct bcmgenet_priv *priv)
{
- struct net_device *ndev = priv->dev;
u32 reg;
/* Speed settings are set in bcmgenet_mii_setup() */
@@ -245,14 +244,14 @@ static void bcmgenet_moca_phy_setup(struct bcmgenet_priv *priv)
bcmgenet_sys_writel(priv, reg, SYS_PORT_CTRL);
if (priv->hw_params->flags & GENET_HAS_MOCA_LINK_DET)
- fixed_phy_set_link_update(ndev->phydev,
+ fixed_phy_set_link_update(priv->phydev,
bcmgenet_fixed_phy_link_update);
}
int bcmgenet_mii_config(struct net_device *dev)
{
struct bcmgenet_priv *priv = netdev_priv(dev);
- struct phy_device *phydev = dev->phydev;
+ struct phy_device *phydev = priv->phydev;
struct device *kdev = &priv->pdev->dev;
const char *phy_name = NULL;
u32 id_mode_dis = 0;
@@ -303,7 +302,7 @@ int bcmgenet_mii_config(struct net_device *dev)
* capabilities, use that knowledge to also configure the
* Reverse MII interface correctly.
*/
- if ((phydev->supported & PHY_BASIC_FEATURES) ==
+ if ((priv->phydev->supported & PHY_BASIC_FEATURES) ==
PHY_BASIC_FEATURES)
port_ctrl = PORT_MODE_EXT_RVMII_25;
else
@@ -372,7 +371,7 @@ int bcmgenet_mii_probe(struct net_device *dev)
return -ENODEV;
}
} else {
- phydev = dev->phydev;
+ phydev = priv->phydev;
phydev->dev_flags = phy_flags;
ret = phy_connect_direct(dev, phydev, bcmgenet_mii_setup,
@@ -383,6 +382,8 @@ int bcmgenet_mii_probe(struct net_device *dev)
}
}
+ priv->phydev = phydev;
+
/* Configure port multiplexer based on what the probed PHY device since
* reading the 'max-speed' property determines the maximum supported
* PHY speed which is needed for bcmgenet_mii_config() to configure
@@ -390,7 +391,7 @@ int bcmgenet_mii_probe(struct net_device *dev)
*/
ret = bcmgenet_mii_config(dev);
if (ret) {
- phy_disconnect(phydev);
+ phy_disconnect(priv->phydev);
return ret;
}
@@ -400,7 +401,7 @@ int bcmgenet_mii_probe(struct net_device *dev)
* Ethernet MAC ISRs
*/
if (priv->internal_phy)
- phydev->irq = PHY_IGNORE_INTERRUPT;
+ priv->phydev->irq = PHY_IGNORE_INTERRUPT;
return 0;
}
@@ -605,6 +606,7 @@ static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv)
}
+ priv->phydev = phydev;
priv->phy_interface = pd->phy_interface;
return 0;
--
2.10.0
^ permalink raw reply related
* Re: [PATCH] softirq: let ksoftirqd do its job
From: Peter Zijlstra @ 2016-09-23 21:16 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Daniel Borkmann, David Miller, eric.dumazet, riel, pabeni, hannes,
linux-kernel, netdev, corbet, Ingo Molnar
In-Reply-To: <20160923185104.535813b4@redhat.com>
On Fri, Sep 23, 2016 at 06:51:04PM +0200, Jesper Dangaard Brouer wrote:
> This is your git tree, right:
> https://git.kernel.org/cgit/linux/kernel/git/peterz/queue.git/
>
> Doesn't look like you pushed it yet, or do I need to look at a specific
> branch?
I mainly work from a local quilt queue which I feed to mingo. I
occasionally push out to get build-bot coverage or have people look at
bits I poked together.
That said, I'll try and do a push later tonight.
Do note however, that git tree is a complete wipe and rebuild, don't
expect any kind of continuity from it.
^ permalink raw reply
* Re: [RFC] net: store port/representative id in metadata_dst
From: John Fastabend @ 2016-09-23 21:20 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jakub Kicinski, Samudrala, Sridhar, Jiri Benc, Jiri Pirko, netdev,
Thomas Graf, Roopa Prabhu, ogerlitz, ast, daniel, simon.horman,
Paolo Abeni, Pravin B Shelar, hannes
In-Reply-To: <20160923214544.6a3cd0ff@laptop>
On 16-09-23 01:45 PM, Jakub Kicinski wrote:
> On Fri, 23 Sep 2016 13:25:10 -0700, John Fastabend wrote:
>> On 16-09-23 01:17 PM, Jakub Kicinski wrote:
>>> On Fri, 23 Sep 2016 10:22:59 -0700, Samudrala, Sridhar wrote:
>>>> On 9/23/2016 8:29 AM, Jakub Kicinski wrote:
>> [...]
>> [...]
>>>>
>>>> The 'accel' parameter in dev_queue_xmit_accel() is currently only passed
>>>> to ndo_select_queue() via netdev_pick_tx() and is used to select the tx
>>>> queue.
>>>> Also, it is not passed all the way to the driver specific xmit routine.
>>>> Doesn't it require
>>>> changing all the driver xmit routines if we want to pass this parameter?
>>>>
>> [...]
>>>>
>>>> Yes. The VFPR netdevs don't have any HW queues associated with them and
>>>> we would like
>>>> to use the PF queues for the xmit.
>>>> I was also looking into some way of passing the port id via skb
>>>> parameter to the
>>>> dev_queue_xmit() call so that the PF xmit routine can do a directed
>>>> transmit to a specifc VF.
>>>> Is skb->cb an option to pass this info?
>>>> dst_metadata approach would work too if it is acceptable.
>>>
>>> I don't think we can trust skb->cb to be set to anything meaningful
>>> when the skb is received by the lower device.
>>
>> Agreed. I wouldn't recommend using skb->cb. How about passing it through
>> dev_queue_xmit_accel() through to the driver?
>>
>> If you pass the metadata through the dev_queue_xmit_accel() handle tx
>> queue selection would work using normal mechanisms (xps, select_queue,
>> cls hook, etc.). If you wanted to pick some specific queue based on
>> policy the policy could be loaded into one of those hooks.
>
> Do you mean without extending how accel is handled by
> dev_queue_xmit_accel() today? If my goal is to not have extra HW
> queues then I don't see how I could mux in the lower dev without extra
> locking (as I tried to explain two emails ago). Sorry for being slow
> here :(
>
Not slow here I think I was overly optimistic...
Yeh let me try this, roughly the current flow is,
dev_queue_xmit_accel(struct sk_buff *skb, void *accel_priv);
__dev_queue_xmit(skb, accel_priv);
netdev_pick_tx(dev, skb, accel_priv);
ndo_select_queue(dev, skb, accel_priv, ...);
[...]
q->enqueue();
[...]
dev_hard_start_xmit();
[...]
<driver code here>
So in this flow the VFR netdev driver handles its xmit routine by
calling dev_queue_xmit_accel after setting skb->dev to the physical
device and passing a cookie via accel that the select_queue() routine
can use to pick a tx queue. The rest of the stack q->enqueue() and
friends will ensure that locking and qdisc is handled correctly.
But accel_priv was lost at queue selection and so its not being passed
down to the driver so no way to set your descriptor bits or whatever
needed to push to the VF. I was sort of thinking we could map it from
the select_queue routine but I can't figure out how to do that either.
The metadata idea doesn't seem that bad now that I've spent some more
time going through it. Either that or hijack some field in the skb but
I think that might be worse than the proposal here.
I'm trying to think up some other alternative now and will let you know
if I think of anything clever but got nothing at the moment.
.John
^ permalink raw reply
* Re: [PATCH net] i40e: fix call of ndo_dflt_bridge_getlink()
From: Jeff Kirsher @ 2016-09-23 21:37 UTC (permalink / raw)
To: nicolas.dichtel
Cc: davem, netdev, Huaibin Wang, Scott Feldman, Carolyn Wyborny,
Catherine Sullivan
In-Reply-To: <60c90efd-fab3-84cd-f6e1-f6bc710c5ab7@6wind.com>
[-- Attachment #1: Type: text/plain, Size: 1259 bytes --]
On Fri, 2016-09-23 at 11:12 +0200, Nicolas Dichtel wrote:
> Le 19/09/2016 à 18:14, Nicolas Dichtel a écrit :
> > From: Huaibin Wang <huaibin.wang@6wind.com>
> >
> > Order of arguments is wrong.
> > The wrong code has been introduced by commit 7d4f8d871ab1, but is
> compiled
> > only since commit 9df70b66418e.
> >
> > Note that this may break netlink dumps.
> >
> > Fixes: 9df70b66418e ("i40e: Remove incorrect #ifdef's")
> > Fixes: 7d4f8d871ab1 ("switchdev; add VLAN support for port's
> bridge_getlink")
> > CC: Scott Feldman <sfeldma@gmail.com>
> > CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
> > CC: Catherine Sullivan <catherine.sullivan@intel.com>
> > Signed-off-by: Huaibin Wang <huaibin.wang@6wind.com>
> > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Hi Jeff,
>
> any news about this patch? David has marked it "awaiting upstream" on the
> patchwork, so I understand it should go to your tree.
Yes, it needs to go through my tree. Please send it to intel-wired-lan@li
sts.osuosl.org mailing list, that way I can track it through our patchwork
projects. Also you can trim Scott Feldman and Catherine Sullivan from the
CC list.
http://patchwork.ozlabs.org/project/intel-wired-lan/list/
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] bpf powerpc: add support for bpf constant blinding
From: Daniel Borkmann @ 2016-09-23 21:40 UTC (permalink / raw)
To: Naveen N. Rao, linux-kernel, linuxppc-dev, netdev,
Michael Ellerman
Cc: Alexei Starovoitov, David S. Miller, Ananth N Mavinakayanahalli
In-Reply-To: <0ecead168c80b1c3d8a8101595e689ff7c7a735f.1474661927.git.naveen.n.rao@linux.vnet.ibm.com>
On 09/23/2016 10:35 PM, Naveen N. Rao wrote:
> In line with similar support for other architectures by Daniel Borkmann.
>
> 'MOD Default X' from test_bpf without constant blinding:
> 84 bytes emitted from JIT compiler (pass:3, flen:7)
> d0000000058a4688 + <x>:
> 0: nop
> 4: nop
> 8: std r27,-40(r1)
> c: std r28,-32(r1)
> 10: xor r8,r8,r8
> 14: xor r28,r28,r28
> 18: mr r27,r3
> 1c: li r8,66
> 20: cmpwi r28,0
> 24: bne 0x0000000000000030
> 28: li r8,0
> 2c: b 0x0000000000000044
> 30: divwu r9,r8,r28
> 34: mullw r9,r28,r9
> 38: subf r8,r9,r8
> 3c: rotlwi r8,r8,0
> 40: li r8,66
> 44: ld r27,-40(r1)
> 48: ld r28,-32(r1)
> 4c: mr r3,r8
> 50: blr
>
> ... and with constant blinding:
> 140 bytes emitted from JIT compiler (pass:3, flen:11)
> d00000000bd6ab24 + <x>:
> 0: nop
> 4: nop
> 8: std r27,-40(r1)
> c: std r28,-32(r1)
> 10: xor r8,r8,r8
> 14: xor r28,r28,r28
> 18: mr r27,r3
> 1c: lis r2,-22834
> 20: ori r2,r2,36083
> 24: rotlwi r2,r2,0
> 28: xori r2,r2,36017
> 2c: xoris r2,r2,42702
> 30: rotlwi r2,r2,0
> 34: mr r8,r2
> 38: rotlwi r8,r8,0
> 3c: cmpwi r28,0
> 40: bne 0x000000000000004c
> 44: li r8,0
> 48: b 0x000000000000007c
> 4c: divwu r9,r8,r28
> 50: mullw r9,r28,r9
> 54: subf r8,r9,r8
> 58: rotlwi r8,r8,0
> 5c: lis r2,-17137
> 60: ori r2,r2,39065
> 64: rotlwi r2,r2,0
> 68: xori r2,r2,39131
> 6c: xoris r2,r2,48399
> 70: rotlwi r2,r2,0
> 74: mr r8,r2
> 78: rotlwi r8,r8,0
> 7c: ld r27,-40(r1)
> 80: ld r28,-32(r1)
> 84: mr r3,r8
> 88: blr
>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net-next v2] Documentation: devicetree: revise ethernet device-tree binding about TRGMII
From: Rob Herring @ 2016-09-23 21:48 UTC (permalink / raw)
To: sean.wang-NuS5LvNUpcJWk0Htik3J/w
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8,
nbd-p3rKhJxN3npAfugRpC6u6w, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
john-Pj+rj9U5foFAfugRpC6u6w, keyhaede-Re5JQEeQqe8AvxtiuMwx3w,
objelf-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1474610649-18582-1-git-send-email-sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
On Fri, Sep 23, 2016 at 02:04:09PM +0800, sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
> add phy-mode "trgmii" to
> Documentation/devicetree/bindings/net/ethernet.txt
>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Reported-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
> Signed-off-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/net/ethernet.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next] Documentation: devicetree: fix typo in MediaTek ethernet device-tree binding
From: Rob Herring @ 2016-09-23 21:48 UTC (permalink / raw)
To: sean.wang
Cc: davem, sergei.shtylyov, nbd, netdev, linux-kernel, devicetree,
linux-mediatek, john, keyhaede, objelf
In-Reply-To: <1474610972-14974-1-git-send-email-sean.wang@mediatek.com>
On Fri, Sep 23, 2016 at 02:09:32PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> fix typo in
> Documentation/devicetree/bindings/net/mediatek-net.txt
>
> Cc: devicetree@vger.kernel.org
> Reported-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
> Documentation/devicetree/bindings/net/mediatek-net.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH] hv_netvsc: fix comments
From: sthemmin @ 2016-09-24 0:08 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, davem; +Cc: netdev, Stephen Hemminger
From: Stephen Hemminger <sthemmin@microsoft.com>
Typo's and spelling errors. Also remove old comment from staging era.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/net/hyperv/hyperv_net.h | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 284b97b..d7c1cc6 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -433,7 +433,7 @@ struct nvsp_1_message_revoke_send_buffer {
*/
struct nvsp_1_message_send_rndis_packet {
/*
- * This field is specified by RNIDS. They assume there's two different
+ * This field is specified by RNDIS. They assume there's two different
* channels of communication. However, the Network VSP only has one.
* Therefore, the channel travels with the RNDIS packet.
*/
@@ -578,7 +578,7 @@ struct nvsp_5_send_indirect_table {
/* The number of entries in the send indirection table */
u32 count;
- /* The offset of the send indireciton table from top of this struct.
+ /* The offset of the send indirection table from top of this struct.
* The send indirection table tells which channel to put the send
* traffic on. Each entry is a channel number.
*/
@@ -733,7 +733,6 @@ struct netvsc_device {
struct nvsp_message channel_init_pkt;
struct nvsp_message revoke_packet;
- /* unsigned char HwMacAddr[HW_MACADDR_LEN]; */
struct vmbus_channel *chn_table[VRSS_CHANNEL_MAX];
u32 send_table[VRSS_SEND_TAB_SIZE];
@@ -1238,7 +1237,7 @@ struct rndis_message {
u32 ndis_msg_type;
/* Total length of this message, from the beginning */
- /* of the sruct rndis_message, in bytes. */
+ /* of the struct rndis_message, in bytes. */
u32 msg_len;
/* Actual message */
--
1.7.4.1
^ permalink raw reply related
* [PATCH] netfilter: don't permit unprivileged writes to global state via sysctls
From: Jann Horn @ 2016-09-23 22:21 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI
Cc: netdev, netfilter-devel
This prevents the modification of nf_conntrack_max in unprivileged network
namespaces. For unprivileged network namespaces, ip_conntrack_max is kept
as a readonly sysctl in order to minimize potential compatibility issues.
This patch should apply cleanly to the net tree.
Signed-off-by: Jann Horn <jann@thejh.net>
---
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index ae1a71a..a639e94 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -358,6 +358,9 @@ static int ipv4_init_net(struct net *net)
if (!in->ctl_table)
return -ENOMEM;
+ if (net->user_ns != &init_user_ns)
+ in->ctl_table[0].mode = 0444;
+
in->ctl_table[0].data = &nf_conntrack_max;
in->ctl_table[1].data = &net->ct.count;
in->ctl_table[2].data = &nf_conntrack_htable_size;
--
2.1.4
^ permalink raw reply related
* [PATCH] Net Driver: Add Cypress GX3 VID=04b4 PID=3610.
From: chris.roth-/KKvz3x1pcI @ 2016-09-23 22:24 UTC (permalink / raw)
To: linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Chris Roth, Allan Chou
From: Chris Roth <chris.roth-/KKvz3x1pcI@public.gmane.org>
From: Allan Chou <allan-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
Add support for Cypress GX3 SuperSpeed to Gigabit Ethernet
Bridge Controller (Vendor=04b4 ProdID=3610).
Patch verified on x64 linux kernel 4.7.4 system with the
Kensington SD4600P USB-C Universal Dock with Power, which uses the
Cypress GX3 SuperSpeed to Gigabit Ethernet Bridge Controller.
A similar patch was signed-off and tested-by Allan Chou
<allan-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org> on 2015-12-01.
Allan verified his similar patch on x86 Linux kernel 4.1.6 system
with Cypress GX3 SuperSpeed to Gigabit Ethernet Bridge Controller.
Tested-by: Allan Chou <allan-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
Tested-by: Chris Roth <chris.roth-/KKvz3x1pcI@public.gmane.org>
Signed-off-by: Allan Chou <allan-knRN6Y/kmf1NUHwG+Fw1Kw@public.gmane.org>
Signed-off-by: Chris Roth <chris.roth-/KKvz3x1pcI@public.gmane.org>
---
drivers/net/usb/ax88179_178a.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index e6338c1..8a6675d 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1656,6 +1656,19 @@ static const struct driver_info ax88178a_info = {
.tx_fixup = ax88179_tx_fixup,
};
+static const struct driver_info cypress_GX3_info = {
+ .description = "Cypress GX3 SuperSpeed to Gigabit Ethernet Controller",
+ .bind = ax88179_bind,
+ .unbind = ax88179_unbind,
+ .status = ax88179_status,
+ .link_reset = ax88179_link_reset,
+ .reset = ax88179_reset,
+ .stop = ax88179_stop,
+ .flags = FLAG_ETHER | FLAG_FRAMING_AX,
+ .rx_fixup = ax88179_rx_fixup,
+ .tx_fixup = ax88179_tx_fixup,
+};
+
static const struct driver_info dlink_dub1312_info = {
.description = "D-Link DUB-1312 USB 3.0 to Gigabit Ethernet Adapter",
.bind = ax88179_bind,
@@ -1718,6 +1731,10 @@ static const struct usb_device_id products[] = {
USB_DEVICE(0x0b95, 0x178a),
.driver_info = (unsigned long)&ax88178a_info,
}, {
+ /* Cypress GX3 SuperSpeed to Gigabit Ethernet Bridge Controller */
+ USB_DEVICE(0x04b4, 0x3610),
+ .driver_info = (unsigned long)&cypress_GX3_info,
+}, {
/* D-Link DUB-1312 USB 3.0 to Gigabit Ethernet Adapter */
USB_DEVICE(0x2001, 0x4a00),
.driver_info = (unsigned long)&dlink_dub1312_info,
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: [PATCH 2/3] bpf powerpc: implement support for tail calls
From: Daniel Borkmann @ 2016-09-23 22:33 UTC (permalink / raw)
To: Naveen N. Rao, linux-kernel, linuxppc-dev, netdev,
Michael Ellerman
Cc: Alexei Starovoitov, David S. Miller, Ananth N Mavinakayanahalli
In-Reply-To: <c7c16b999694b2851705d79da939988e521bda3a.1474661927.git.naveen.n.rao@linux.vnet.ibm.com>
On 09/23/2016 10:35 PM, Naveen N. Rao wrote:
> Tail calls allow JIT'ed eBPF programs to call into other JIT'ed eBPF
> programs. This can be achieved either by:
> (1) retaining the stack setup by the first eBPF program and having all
> subsequent eBPF programs re-using it, or,
> (2) by unwinding/tearing down the stack and having each eBPF program
> deal with its own stack as it sees fit.
>
> To ensure that this does not create loops, there is a limit to how many
> tail calls can be done (currently 32). This requires the JIT'ed code to
> maintain a count of the number of tail calls done so far.
>
> Approach (1) is simple, but requires every eBPF program to have (almost)
> the same prologue/epilogue, regardless of whether they need it. This is
> inefficient for small eBPF programs which may not sometimes need a
> prologue at all. As such, to minimize impact of tail call
> implementation, we use approach (2) here which needs each eBPF program
> in the chain to use its own prologue/epilogue. This is not ideal when
> many tail calls are involved and when all the eBPF programs in the chain
> have similar prologue/epilogue. However, the impact is restricted to
> programs that do tail calls. Individual eBPF programs are not affected.
>
> We maintain the tail call count in a fixed location on the stack and
> updated tail call count values are passed in through this. The very
> first eBPF program in a chain sets this up to 0 (the first 2
> instructions). Subsequent tail calls skip the first two eBPF JIT
> instructions to maintain the count. For programs that don't do tail
> calls themselves, the first two instructions are NOPs.
>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Thanks for adding support, Naveen, that's really great! I think 2) seems
fine as well in this context as prologue size can vary quite a bit here,
and depending on program types likelihood of tail call usage as well (but
I wouldn't expect deep nesting). Thanks a lot!
^ permalink raw reply
* Re: Modification to skb->queue_mapping affecting performance
From: Michael Ma @ 2016-09-23 23:21 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <CAAmHdhwZUeKd_GKj8JdbXtdAZxn8sfbaGx+1Mjzn08rT3yf=+g@mail.gmail.com>
2016-09-16 15:00 GMT-07:00 Michael Ma <make0818@gmail.com>:
> 2016-09-16 12:53 GMT-07:00 Eric Dumazet <eric.dumazet@gmail.com>:
>> On Fri, 2016-09-16 at 10:57 -0700, Michael Ma wrote:
>>
>>> This is actually the problem - if flows from different RX queues are
>>> switched to the same RX queue in IFB, they'll use different processor
>>> context with the same tasklet, and the processor context of different
>>> tasklets might be the same. So multiple tasklets in IFB competes for
>>> the same core when queue is switched.
>>>
>>> The following simple fix proved this - with this change even switching
>>> the queue won't affect small packet bandwidth/latency anymore:
>>>
>>> in ifb.c:
>>>
>>> - struct ifb_q_private *txp = dp->tx_private + skb_get_queue_mapping(skb);
>>> + struct ifb_q_private *txp = dp->tx_private +
>>> (smp_processor_id() % dev->num_tx_queues);
>>>
>>> This should be more efficient since we're not sending the task to a
>>> different processor, instead we try to queue the packet to an
>>> appropriate tasklet based on the processor ID. Will this cause any
>>> packet out-of-order problem? If packets from the same flow are queued
>>> to the same RX queue due to RSS, and processor affinity is set for RX
>>> queues, I assume packets from the same flow will end up in the same
>>> core when tasklet is scheduled. But I might have missed some uncommon
>>> cases here... Would appreciate if anyone can provide more insights.
>>
>> Wait, don't you have proper smp affinity for the RX queues on your NIC ?
>>
>> ( Documentation/networking/scaling.txt RSS IRQ Configuration )
>>
> Yes - what I was trying to say is that this change will be more
> efficient than using smp_call_function_single() to schedule the
> tasklet to a different processor.
>
> RSS IRQ should be set properly already. The issue here is that I'll
> need to switch the queue mapping for NIC RX to a different TXQ on IFB,
> which allows me to classify the flows at the IFB TXQ layer and avoid
> qdisc lock contention.
>
> When that switch happens, ideally processor core shouldn't be switched
> because all the thread context isn't changed. The work in tasklet
> should be scheduled to the same processor as well. That's why I tried
> this change. Also conceptually IFB is a software device which should
> be able to schedule its workload independent from how NIC is
> configured for the interrupt handling.
>
>> A driver ndo_start_xmit() MUST use skb_get_queue_mapping(skb), because
>> the driver queue is locked before ndo_start_xmit()) (for non
>> NETIF_F_LLTX drivers at least)
>>
>
> Thanks a lot for pointing out this! I was expecting this kind of
> guidance... Then the options would be:
>
> 1. Use smp_call_function_single() to schedule the tasklet to a core
> statically mapped to the IFB TXQ, which is very similar to how TX/RX
> IRQ is configured.
This actually won't help with the throughput because ultimately load
will still be concentrated to some particular cores after packets are
concentrated to a TXQ due to queue level classification.
> 2. As you suggested below add some additional action to do the
> rescheduling before entering IFB - for example when receiving the
> packet we could just use RSS to redirect to the desired RXQ, however
> this doesn't seem to be easy, especially compared with the way how
> mqprio chooses the queue. The challenge here is that IFB queue
> selection is based on queue_mapping when skb arrives at IFB and core
> selection is based on RXQ on NIC and so it's also based on
> queue_mapping when skb arrives at NIC. Then these two queue_mappings
> must be the same so that there is no core conflict of processing two
> TXQs of IFB. Then this essentially means we have to change queue
> mapping of the NIC on the receiver side which can't be achieved using
> TC.
>
I tried to explore this further - there is actually XPS on ifb which
can be used to specify the processor cores that will be used to
process each TXQ of ifb, however the problem is similar as above -
eventually I'll have a few cores processing these queues instead of
having all the cores processing together with relatively light
contention. And this again reduces the throughput. So there isn't a
good place to do this. The ultimate problem is that we're trying to
workaround the qdisc spin lock problem by leveraging the independence
of TXQs, but at the same time after qdisc phase we also want to
maximize the utilization of cores across whatever TXQs that are used.
>> In case of ifb, __skb_queue_tail(&txp->rq, skb); could corrupt the skb
>> list.
>>
>> In any case, you could have an action to do this before reaching IFB.
>>
>>
>>
So here is another solution - for packets coming from the NIC ingress
path the context is already a tasklet and there is no need of starting
another tasklet based on the queue selected, right? All the RQ
handling and netif_tx_stop/wakeup stuff in ifb module is unnecessary
in this case. Then we can just do transmit/receive in ifb_xmit()
directly to simplify the problem here - there is no synchronization
issue for the RQ anymore since there is no RQ indeed.
Eric, would appreciate if you can share your thoughts here.
^ permalink raw reply
* Re: [PATCH net-next] net/vxlan: Avoid unaligned access in vxlan_build_skb()
From: Sowmini Varadhan @ 2016-09-23 23:41 UTC (permalink / raw)
To: Alexander Duyck
Cc: David Miller, Jiri Benc, Netdev, Hannes Frederic Sowa,
Alexander Duyck, Daniel Borkmann, Paolo Abeni
In-Reply-To: <CAKgT0UeB5n3g4f1v_2nenjcayAi=eOPFSC7132ihXTZOztHRkw@mail.gmail.com>
On (09/23/16 10:38), Alexander Duyck wrote:
>
> So basically what I was thinking is we do something like reserving
> NET_IP_ALIGN and continue writing headers to skb->data, but we force
> the tracking for the inner headers into frag[0] so that we can keep
> the inner headers aligned without messing up the alignment for outer
> headers. In theory the inner offset and all that would still be
> functional but might need a few tweaks. You could probably even use
> the skb->encapsulation bit to indicate you are doing this. You could
> almost think of it as us doing something like the inverse of
> pskb_pull_tail. The general idea here is we want to actually leave
> the data in skb->data, but just reference it from frag[0] so that we
> don't accidentally pull in the 2 byte padding for alignment when
> transmitting the frame.
yes, I think something along this line could do the trick.. I tried
hacking it a bit today for vxlan, and it could be extended for all
these encaps protocols. Let me fix/test this more next week, maybe
we can discuss in Tokyo.
--Sowmini
^ permalink raw reply
* Re: [PATCH net-next] net/vxlan: Avoid unaligned access in vxlan_build_skb()
From: Alexander Duyck @ 2016-09-24 0:43 UTC (permalink / raw)
To: Sowmini Varadhan
Cc: David Miller, Jiri Benc, Netdev, Hannes Frederic Sowa,
Alexander Duyck, Daniel Borkmann, Paolo Abeni
In-Reply-To: <20160923234134.GA6397@oracle.com>
On Fri, Sep 23, 2016 at 4:41 PM, Sowmini Varadhan
<sowmini.varadhan@oracle.com> wrote:
> On (09/23/16 10:38), Alexander Duyck wrote:
>>
>> So basically what I was thinking is we do something like reserving
>> NET_IP_ALIGN and continue writing headers to skb->data, but we force
>> the tracking for the inner headers into frag[0] so that we can keep
>> the inner headers aligned without messing up the alignment for outer
>> headers. In theory the inner offset and all that would still be
>> functional but might need a few tweaks. You could probably even use
>> the skb->encapsulation bit to indicate you are doing this. You could
>> almost think of it as us doing something like the inverse of
>> pskb_pull_tail. The general idea here is we want to actually leave
>> the data in skb->data, but just reference it from frag[0] so that we
>> don't accidentally pull in the 2 byte padding for alignment when
>> transmitting the frame.
>
> yes, I think something along this line could do the trick.. I tried
> hacking it a bit today for vxlan, and it could be extended for all
> these encaps protocols. Let me fix/test this more next week, maybe
> we can discuss in Tokyo.
Agreed. Keep in mind we only really need it for the architectures
that need to set NET_IP_ALIGN so we may want to end up wrapping the
code in ifndef checks for HAVE_EFFICIENT_UNALIGNED_ACCESS.
- Alex
^ 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