* Re: [PATCH] bpf: verifier: BPF_MOV don't mark dst reg if src == dst
From: Arthur Fabre @ 2018-07-30 10:58 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Alexei Starovoitov, Network Development
In-Reply-To: <7b0ce11d-ad54-9ba8-740c-58fd2675cc69@iogearbox.net>
On Mon, Jul 30, 2018 at 10:10 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 07/30/2018 09:44 AM, Arthur Fabre wrote:
>> On Sun, Jul 29, 2018 at 4:59 PM, Alexei Starovoitov
>> <alexei.starovoitov@gmail.com> wrote:
>>> On Thu, Jul 26, 2018 at 1:08 AM, Arthur Fabre <afabre@cloudflare.com> wrote:
>>>> When check_alu_op() handles a BPF_MOV between two registers,
>>>> it calls check_reg_arg() on the dst register, marking it as unbounded.
>>>> If the src and dst register are the same, this marks the src as
>>>> unbounded, which can lead to unexpected errors for further checks that
>>>> rely on bounds info.
>>>>
>>>> check_alu_op() now only marks the dst register as unbounded if it
>>>> different from the src register.
>>>>
>>>> Signed-off-by: Arthur Fabre <afabre@cloudflare.com>
>>>> ---
>>>> kernel/bpf/verifier.c | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>>> index 63aaac52a265..ddfe3c544a80 100644
>>>> --- a/kernel/bpf/verifier.c
>>>> +++ b/kernel/bpf/verifier.c
>>>> @@ -3238,8 +3238,9 @@ static int check_alu_op(struct bpf_verifier_env
>>>> *env, struct bpf_insn *insn)
>>>> }
>>>> }
>>>>
>>>> - /* check dest operand */
>>>> - err = check_reg_arg(env, insn->dst_reg, DST_OP);
>>>> + /* check dest operand, only mark if dest != src */
>>>> + err = check_reg_arg(env, insn->dst_reg,
>>>> + insn->dst_reg == insn->src_reg ?
>>>> DST_OP_NO_MARK : DST_OP);
>>>
>>> that doesn't look correct for 32-bit mov.
>>> Is that the case you're trying to improve?
>>
>> The patch was originally for 64-bit mov only
>
> Hmm, I'm not sure that is infact the case. The check_alu_op() is handled for
> 32 and 64 bit alu op case. So in the opcode == BPF_MOV case the check_reg_arg()
> on the dst register is done for both at that point, whereas retaining any
> current state should only be valid in 64 bit mov case, e.g. think of pointer
> types, these really need to be scratched here. I think it would make sense that
> after checking src operand we hold a temporary copy of its state and use that
> for setting regs[insn->dst_reg] later on under BPF_ALU64.
The check_alu_op() call handles 32bit and 64bit cases, but then in the
32bit case
mark_reg_unknown() is called, discarding all the dst register state.
I think this is equivalent to keeping a copy of dst and always marking
dst as unknown.
I think we could actually always use check_reg_arg() with DST_OP_NO_MARK:
In the 32bit case, we call mark_reg_unknown() anyways.
In the 64bit case, we copy src to dst, so marking dst as unknown is pointless.
For plain BPF, we call __mark_reg_known() anyways.
^ permalink raw reply
* Re: [PATCH net-next 1/2] be2net: Collect the transmit queue data in Tx timeout
From: Suresh Kumar Reddy Reddygari @ 2018-07-30 10:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <CAN0M_G5CNBP7V==84og-xR+sCQ2dRcXJcopWBpNWnD_xJTLD1g@mail.gmail.com>
On Wed, Jul 25, 2018 at 6:14 PM, Suresh Kumar Reddy Reddygari
<suresh.reddy@broadcom.com> wrote:
> On Mon, Jul 23, 2018 at 11:53 PM, David Miller <davem@davemloft.net> wrote:
>> From: Suresh Reddy <suresh.reddy@broadcom.com>
>> Date: Mon, 23 Jul 2018 10:25:23 -0400
>>
>>> Driver dumps tx_queue, tx_compl, pending SKBs information in tx_timeout.
>>> This debug data used to idenfiy the cause of the time out.
>>>
>>> Also reset Lancer chip in tx_timeout.
>>>
...
>>
>> The purpose of the tx timeout NDO operation is to do whatever is
>> necessary to handle the TX timeout.
>>
>> Outputting debugging information is useful, but is secondary.
>>
>> I see that you do reset the Lancer, but that is far from what really
>> needs to happen here.
>>
>> When you get a TX timeout, the hardware is not processing TX ring
>> entries, nor signalling completion any longer.
>>
>> Therefore the only way to get things going again is to reset all of
>> the TX side data structure and logic. This means shutting down the TX
>> engine, freeing up all of the TX SKBs in the ring, resetting the TX
>> ring software state, and then finally reprogramming the head/tail
>> pointer registers and re-enabling TX DMA processing.
Hi David,
I am clarifying again about the patch (Lancer reset) as I didnt see a reply
from you after my clarification.
> +static void be_tx_timeout(struct net_device *netdev)
> +{
...
> +
> + if (lancer_chip(adapter)) {
> + dev_info(dev, "Initiating reset due to tx timeout\n");
> + dev_info(dev, "Resetting adapter\n");
> + status = lancer_physdev_ctrl(adapter,
> + PHYSDEV_CONTROL_FW_RESET_MASK);
This patch does recover from a TX-timeout by resetting the chip itself
that *includes* resetting the TX block. Freeing up TX SKBs and rings and
re-creating/registering them is also being done.
Driver and Firmware does the following in chip reset path.
1. When driver sets the PHYSDEV_RESET_MASK bit, Lancer firmware
goes into an error state and indicates this back to the driver via a bit in
a doorbell register.
2. Driver detects this and calls be_err_recover().
be_close() and be_clear() are called in this error recovery path.
a) In be_close(), we cleanup all pending TX queue entries and SKBs.
b) In be_clear(), we destroy TX, RX and all other queues.
3. be_resume() is called after be_cleanup() in error recover path.
In this routine, we create TX, RX queues and other resources
which were destroyed in cleanup path.
Now, this recovery is supported only on Lancer NICs (and not in BE3
and Skyhawk NICs). That's why this patch only gathers diagnostic information
in the case of BE3 and Skyhawk NICs, but on Lancer NICs does the extra step
of resetting and recovering.
Please let me know if this makes sense. I send a v2 patch with this
explanation in the
commit message.
Regards,
Suresh.
^ permalink raw reply
* [PATCH v6 bpf-next 9/9] veth: Support per queue XDP ring
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
In-Reply-To: <1532947431-2737-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
Move XDP and napi related fields in veth_priv to newly created veth_rq
structure.
When xdp_frames are enqueued from ndo_xdp_xmit and XDP_TX, rxq is
selected by current cpu.
When skbs are enqueued from the peer device, rxq is one to one mapping
of its peer txq. This way we have a restriction that the number of rxqs
must not less than the number of peer txqs, but leave the possibility to
achieve bulk skb xmit in the future because txq lock would make it
possible to remove rxq ptr_ring lock.
v3:
- Add extack messages.
- Fix array overrun in veth_xmit.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/net/veth.c | 278 ++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 188 insertions(+), 90 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index b1ce3691..c276a72 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -42,20 +42,24 @@ struct pcpu_vstats {
struct u64_stats_sync syncp;
};
-struct veth_priv {
+struct veth_rq {
struct napi_struct xdp_napi;
struct net_device *dev;
struct bpf_prog __rcu *xdp_prog;
- struct bpf_prog *_xdp_prog;
- struct net_device __rcu *peer;
- atomic64_t dropped;
struct xdp_mem_info xdp_mem;
- unsigned requested_headroom;
bool rx_notify_masked;
struct ptr_ring xdp_ring;
struct xdp_rxq_info xdp_rxq;
};
+struct veth_priv {
+ struct net_device __rcu *peer;
+ atomic64_t dropped;
+ struct bpf_prog *_xdp_prog;
+ struct veth_rq *rq;
+ unsigned int requested_headroom;
+};
+
/*
* ethtool interface
*/
@@ -144,19 +148,19 @@ static void veth_ptr_free(void *ptr)
kfree_skb(ptr);
}
-static void __veth_xdp_flush(struct veth_priv *priv)
+static void __veth_xdp_flush(struct veth_rq *rq)
{
/* Write ptr_ring before reading rx_notify_masked */
smp_mb();
- if (!priv->rx_notify_masked) {
- priv->rx_notify_masked = true;
- napi_schedule(&priv->xdp_napi);
+ if (!rq->rx_notify_masked) {
+ rq->rx_notify_masked = true;
+ napi_schedule(&rq->xdp_napi);
}
}
-static int veth_xdp_rx(struct veth_priv *priv, struct sk_buff *skb)
+static int veth_xdp_rx(struct veth_rq *rq, struct sk_buff *skb)
{
- if (unlikely(ptr_ring_produce(&priv->xdp_ring, skb))) {
+ if (unlikely(ptr_ring_produce(&rq->xdp_ring, skb))) {
dev_kfree_skb_any(skb);
return NET_RX_DROP;
}
@@ -164,21 +168,22 @@ static int veth_xdp_rx(struct veth_priv *priv, struct sk_buff *skb)
return NET_RX_SUCCESS;
}
-static int veth_forward_skb(struct net_device *dev, struct sk_buff *skb, bool xdp)
+static int veth_forward_skb(struct net_device *dev, struct sk_buff *skb,
+ struct veth_rq *rq, bool xdp)
{
- struct veth_priv *priv = netdev_priv(dev);
-
return __dev_forward_skb(dev, skb) ?: xdp ?
- veth_xdp_rx(priv, skb) :
+ veth_xdp_rx(rq, skb) :
netif_rx(skb);
}
static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
+ struct veth_rq *rq = NULL;
struct net_device *rcv;
int length = skb->len;
bool rcv_xdp = false;
+ int rxq;
rcu_read_lock();
rcv = rcu_dereference(priv->peer);
@@ -188,9 +193,15 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
}
rcv_priv = netdev_priv(rcv);
- rcv_xdp = rcu_access_pointer(rcv_priv->xdp_prog);
+ rxq = skb_get_queue_mapping(skb);
+ if (rxq < rcv->real_num_rx_queues) {
+ rq = &rcv_priv->rq[rxq];
+ rcv_xdp = rcu_access_pointer(rq->xdp_prog);
+ if (rcv_xdp)
+ skb_record_rx_queue(skb, rxq);
+ }
- if (likely(veth_forward_skb(rcv, skb, rcv_xdp) == NET_RX_SUCCESS)) {
+ if (likely(veth_forward_skb(rcv, skb, rq, rcv_xdp) == NET_RX_SUCCESS)) {
struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
u64_stats_update_begin(&stats->syncp);
@@ -203,7 +214,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
}
if (rcv_xdp)
- __veth_xdp_flush(rcv_priv);
+ __veth_xdp_flush(rq);
rcu_read_unlock();
@@ -278,12 +289,18 @@ static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
return skb;
}
+static int veth_select_rxq(struct net_device *dev)
+{
+ return smp_processor_id() % dev->real_num_rx_queues;
+}
+
static int veth_xdp_xmit(struct net_device *dev, int n,
struct xdp_frame **frames, u32 flags)
{
struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
struct net_device *rcv;
unsigned int max_len;
+ struct veth_rq *rq;
int i, drops = 0;
if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
@@ -294,30 +311,31 @@ static int veth_xdp_xmit(struct net_device *dev, int n,
return -ENXIO;
rcv_priv = netdev_priv(rcv);
+ rq = &rcv_priv->rq[veth_select_rxq(rcv)];
/* Non-NULL xdp_prog ensures that xdp_ring is initialized on receive
* side. This means an XDP program is loaded on the peer and the peer
* device is up.
*/
- if (!rcu_access_pointer(rcv_priv->xdp_prog))
+ if (!rcu_access_pointer(rq->xdp_prog))
return -ENXIO;
max_len = rcv->mtu + rcv->hard_header_len + VLAN_HLEN;
- spin_lock(&rcv_priv->xdp_ring.producer_lock);
+ spin_lock(&rq->xdp_ring.producer_lock);
for (i = 0; i < n; i++) {
struct xdp_frame *frame = frames[i];
void *ptr = veth_xdp_to_ptr(frame);
if (unlikely(frame->len > max_len ||
- __ptr_ring_produce(&rcv_priv->xdp_ring, ptr))) {
+ __ptr_ring_produce(&rq->xdp_ring, ptr))) {
xdp_return_frame_rx_napi(frame);
drops++;
}
}
- spin_unlock(&rcv_priv->xdp_ring.producer_lock);
+ spin_unlock(&rq->xdp_ring.producer_lock);
if (flags & XDP_XMIT_FLUSH)
- __veth_xdp_flush(rcv_priv);
+ __veth_xdp_flush(rq);
return n - drops;
}
@@ -326,6 +344,7 @@ static void veth_xdp_flush(struct net_device *dev)
{
struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
struct net_device *rcv;
+ struct veth_rq *rq;
rcu_read_lock();
rcv = rcu_dereference(priv->peer);
@@ -333,11 +352,12 @@ static void veth_xdp_flush(struct net_device *dev)
goto out;
rcv_priv = netdev_priv(rcv);
+ rq = &rcv_priv->rq[veth_select_rxq(rcv)];
/* xdp_ring is initialized on receive side? */
- if (unlikely(!rcu_access_pointer(rcv_priv->xdp_prog)))
+ if (unlikely(!rcu_access_pointer(rq->xdp_prog)))
goto out;
- __veth_xdp_flush(rcv_priv);
+ __veth_xdp_flush(rq);
out:
rcu_read_unlock();
}
@@ -352,7 +372,7 @@ static int veth_xdp_tx(struct net_device *dev, struct xdp_buff *xdp)
return veth_xdp_xmit(dev, 1, &frame, 0);
}
-static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
+static struct sk_buff *veth_xdp_rcv_one(struct veth_rq *rq,
struct xdp_frame *frame,
unsigned int *xdp_xmit)
{
@@ -363,7 +383,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
struct sk_buff *skb;
rcu_read_lock();
- xdp_prog = rcu_dereference(priv->xdp_prog);
+ xdp_prog = rcu_dereference(rq->xdp_prog);
if (likely(xdp_prog)) {
struct xdp_buff xdp;
u32 act;
@@ -372,7 +392,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
xdp.data = frame->data;
xdp.data_end = frame->data + frame->len;
xdp.data_meta = frame->data - frame->metasize;
- xdp.rxq = &priv->xdp_rxq;
+ xdp.rxq = &rq->xdp_rxq;
act = bpf_prog_run_xdp(xdp_prog, &xdp);
@@ -385,8 +405,8 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
orig_frame = *frame;
xdp.data_hard_start = frame;
xdp.rxq->mem = frame->mem;
- if (unlikely(veth_xdp_tx(priv->dev, &xdp) < 0)) {
- trace_xdp_exception(priv->dev, xdp_prog, act);
+ if (unlikely(veth_xdp_tx(rq->dev, &xdp) < 0)) {
+ trace_xdp_exception(rq->dev, xdp_prog, act);
frame = &orig_frame;
goto err_xdp;
}
@@ -397,7 +417,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
orig_frame = *frame;
xdp.data_hard_start = frame;
xdp.rxq->mem = frame->mem;
- if (xdp_do_redirect(priv->dev, &xdp, xdp_prog)) {
+ if (xdp_do_redirect(rq->dev, &xdp, xdp_prog)) {
frame = &orig_frame;
goto err_xdp;
}
@@ -407,7 +427,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
- trace_xdp_exception(priv->dev, xdp_prog, act);
+ trace_xdp_exception(rq->dev, xdp_prog, act);
case XDP_DROP:
goto err_xdp;
}
@@ -422,7 +442,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
}
memset(frame, 0, sizeof(*frame));
- skb->protocol = eth_type_trans(skb, priv->dev);
+ skb->protocol = eth_type_trans(skb, rq->dev);
err:
return skb;
err_xdp:
@@ -432,8 +452,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
return NULL;
}
-static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
- struct sk_buff *skb,
+static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, struct sk_buff *skb,
unsigned int *xdp_xmit)
{
u32 pktlen, headroom, act, metalen;
@@ -443,7 +462,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
struct xdp_buff xdp;
rcu_read_lock();
- xdp_prog = rcu_dereference(priv->xdp_prog);
+ xdp_prog = rcu_dereference(rq->xdp_prog);
if (unlikely(!xdp_prog)) {
rcu_read_unlock();
goto out;
@@ -497,7 +516,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
xdp.data = skb_mac_header(skb);
xdp.data_end = xdp.data + pktlen;
xdp.data_meta = xdp.data;
- xdp.rxq = &priv->xdp_rxq;
+ xdp.rxq = &rq->xdp_rxq;
orig_data = xdp.data;
orig_data_end = xdp.data_end;
@@ -509,9 +528,9 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
case XDP_TX:
get_page(virt_to_page(xdp.data));
consume_skb(skb);
- xdp.rxq->mem = priv->xdp_mem;
- if (unlikely(veth_xdp_tx(priv->dev, &xdp) < 0)) {
- trace_xdp_exception(priv->dev, xdp_prog, act);
+ xdp.rxq->mem = rq->xdp_mem;
+ if (unlikely(veth_xdp_tx(rq->dev, &xdp) < 0)) {
+ trace_xdp_exception(rq->dev, xdp_prog, act);
goto err_xdp;
}
*xdp_xmit |= VETH_XDP_TX;
@@ -520,8 +539,8 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
case XDP_REDIRECT:
get_page(virt_to_page(xdp.data));
consume_skb(skb);
- xdp.rxq->mem = priv->xdp_mem;
- if (xdp_do_redirect(priv->dev, &xdp, xdp_prog))
+ xdp.rxq->mem = rq->xdp_mem;
+ if (xdp_do_redirect(rq->dev, &xdp, xdp_prog))
goto err_xdp;
*xdp_xmit |= VETH_XDP_REDIR;
rcu_read_unlock();
@@ -529,7 +548,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
- trace_xdp_exception(priv->dev, xdp_prog, act);
+ trace_xdp_exception(rq->dev, xdp_prog, act);
case XDP_DROP:
goto drop;
}
@@ -545,7 +564,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
off = xdp.data_end - orig_data_end;
if (off != 0)
__skb_put(skb, off);
- skb->protocol = eth_type_trans(skb, priv->dev);
+ skb->protocol = eth_type_trans(skb, rq->dev);
metalen = xdp.data - xdp.data_meta;
if (metalen)
@@ -563,27 +582,26 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
return NULL;
}
-static int veth_xdp_rcv(struct veth_priv *priv, int budget,
- unsigned int *xdp_xmit)
+static int veth_xdp_rcv(struct veth_rq *rq, int budget, unsigned int *xdp_xmit)
{
int i, done = 0;
for (i = 0; i < budget; i++) {
- void *ptr = __ptr_ring_consume(&priv->xdp_ring);
+ void *ptr = __ptr_ring_consume(&rq->xdp_ring);
struct sk_buff *skb;
if (!ptr)
break;
if (veth_is_xdp_frame(ptr)) {
- skb = veth_xdp_rcv_one(priv, veth_ptr_to_xdp(ptr),
+ skb = veth_xdp_rcv_one(rq, veth_ptr_to_xdp(ptr),
xdp_xmit);
} else {
- skb = veth_xdp_rcv_skb(priv, ptr, xdp_xmit);
+ skb = veth_xdp_rcv_skb(rq, ptr, xdp_xmit);
}
if (skb)
- napi_gro_receive(&priv->xdp_napi, skb);
+ napi_gro_receive(&rq->xdp_napi, skb);
done++;
}
@@ -593,25 +611,25 @@ static int veth_xdp_rcv(struct veth_priv *priv, int budget,
static int veth_poll(struct napi_struct *napi, int budget)
{
- struct veth_priv *priv =
- container_of(napi, struct veth_priv, xdp_napi);
+ struct veth_rq *rq =
+ container_of(napi, struct veth_rq, xdp_napi);
unsigned int xdp_xmit = 0;
int done;
xdp_set_return_frame_no_direct();
- done = veth_xdp_rcv(priv, budget, &xdp_xmit);
+ done = veth_xdp_rcv(rq, budget, &xdp_xmit);
if (done < budget && napi_complete_done(napi, done)) {
/* Write rx_notify_masked before reading ptr_ring */
- smp_store_mb(priv->rx_notify_masked, false);
- if (unlikely(!__ptr_ring_empty(&priv->xdp_ring))) {
- priv->rx_notify_masked = true;
- napi_schedule(&priv->xdp_napi);
+ smp_store_mb(rq->rx_notify_masked, false);
+ if (unlikely(!__ptr_ring_empty(&rq->xdp_ring))) {
+ rq->rx_notify_masked = true;
+ napi_schedule(&rq->xdp_napi);
}
}
if (xdp_xmit & VETH_XDP_TX)
- veth_xdp_flush(priv->dev);
+ veth_xdp_flush(rq->dev);
if (xdp_xmit & VETH_XDP_REDIR)
xdp_do_flush_map();
xdp_clear_return_frame_no_direct();
@@ -622,56 +640,90 @@ static int veth_poll(struct napi_struct *napi, int budget)
static int veth_napi_add(struct net_device *dev)
{
struct veth_priv *priv = netdev_priv(dev);
- int err;
+ int err, i;
- err = ptr_ring_init(&priv->xdp_ring, VETH_RING_SIZE, GFP_KERNEL);
- if (err)
- return err;
+ for (i = 0; i < dev->real_num_rx_queues; i++) {
+ struct veth_rq *rq = &priv->rq[i];
+
+ err = ptr_ring_init(&rq->xdp_ring, VETH_RING_SIZE, GFP_KERNEL);
+ if (err)
+ goto err_xdp_ring;
+ }
- netif_napi_add(dev, &priv->xdp_napi, veth_poll, NAPI_POLL_WEIGHT);
- napi_enable(&priv->xdp_napi);
+ for (i = 0; i < dev->real_num_rx_queues; i++) {
+ struct veth_rq *rq = &priv->rq[i];
+
+ netif_napi_add(dev, &rq->xdp_napi, veth_poll, NAPI_POLL_WEIGHT);
+ napi_enable(&rq->xdp_napi);
+ }
return 0;
+err_xdp_ring:
+ for (i--; i >= 0; i--)
+ ptr_ring_cleanup(&priv->rq[i].xdp_ring, veth_ptr_free);
+
+ return err;
}
static void veth_napi_del(struct net_device *dev)
{
struct veth_priv *priv = netdev_priv(dev);
+ int i;
- napi_disable(&priv->xdp_napi);
- netif_napi_del(&priv->xdp_napi);
- priv->rx_notify_masked = false;
- ptr_ring_cleanup(&priv->xdp_ring, veth_ptr_free);
+ for (i = 0; i < dev->real_num_rx_queues; i++) {
+ struct veth_rq *rq = &priv->rq[i];
+
+ napi_disable(&rq->xdp_napi);
+ napi_hash_del(&rq->xdp_napi);
+ }
+ synchronize_net();
+
+ for (i = 0; i < dev->real_num_rx_queues; i++) {
+ struct veth_rq *rq = &priv->rq[i];
+
+ netif_napi_del(&rq->xdp_napi);
+ rq->rx_notify_masked = false;
+ ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
+ }
}
static int veth_enable_xdp(struct net_device *dev)
{
struct veth_priv *priv = netdev_priv(dev);
- int err;
+ int err, i;
- if (!xdp_rxq_info_is_reg(&priv->xdp_rxq)) {
- err = xdp_rxq_info_reg(&priv->xdp_rxq, dev, 0);
- if (err < 0)
- return err;
+ if (!xdp_rxq_info_is_reg(&priv->rq[0].xdp_rxq)) {
+ for (i = 0; i < dev->real_num_rx_queues; i++) {
+ struct veth_rq *rq = &priv->rq[i];
- err = xdp_rxq_info_reg_mem_model(&priv->xdp_rxq,
- MEM_TYPE_PAGE_SHARED, NULL);
- if (err < 0)
- goto err;
+ err = xdp_rxq_info_reg(&rq->xdp_rxq, dev, i);
+ if (err < 0)
+ goto err_rxq_reg;
+
+ err = xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,
+ MEM_TYPE_PAGE_SHARED,
+ NULL);
+ if (err < 0)
+ goto err_reg_mem;
+
+ /* Save original mem info as it can be overwritten */
+ rq->xdp_mem = rq->xdp_rxq.mem;
+ }
err = veth_napi_add(dev);
if (err)
- goto err;
-
- /* Save original mem info as it can be overwritten */
- priv->xdp_mem = priv->xdp_rxq.mem;
+ goto err_rxq_reg;
}
- rcu_assign_pointer(priv->xdp_prog, priv->_xdp_prog);
+ for (i = 0; i < dev->real_num_rx_queues; i++)
+ rcu_assign_pointer(priv->rq[i].xdp_prog, priv->_xdp_prog);
return 0;
-err:
- xdp_rxq_info_unreg(&priv->xdp_rxq);
+err_reg_mem:
+ xdp_rxq_info_unreg(&priv->rq[i].xdp_rxq);
+err_rxq_reg:
+ for (i--; i >= 0; i--)
+ xdp_rxq_info_unreg(&priv->rq[i].xdp_rxq);
return err;
}
@@ -679,11 +731,17 @@ static int veth_enable_xdp(struct net_device *dev)
static void veth_disable_xdp(struct net_device *dev)
{
struct veth_priv *priv = netdev_priv(dev);
+ int i;
- rcu_assign_pointer(priv->xdp_prog, NULL);
+ for (i = 0; i < dev->real_num_rx_queues; i++)
+ rcu_assign_pointer(priv->rq[i].xdp_prog, NULL);
veth_napi_del(dev);
- priv->xdp_rxq.mem = priv->xdp_mem;
- xdp_rxq_info_unreg(&priv->xdp_rxq);
+ for (i = 0; i < dev->real_num_rx_queues; i++) {
+ struct veth_rq *rq = &priv->rq[i];
+
+ rq->xdp_rxq.mem = rq->xdp_mem;
+ xdp_rxq_info_unreg(&rq->xdp_rxq);
+ }
}
static int veth_open(struct net_device *dev)
@@ -840,6 +898,12 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
goto err;
}
+ if (dev->real_num_rx_queues < peer->real_num_tx_queues) {
+ NL_SET_ERR_MSG_MOD(extack, "XDP expects number of rx queues not less than peer tx queues");
+ err = -ENOSPC;
+ goto err;
+ }
+
if (dev->flags & IFF_UP) {
err = veth_enable_xdp(dev);
if (err) {
@@ -974,13 +1038,31 @@ static int veth_validate(struct nlattr *tb[], struct nlattr *data[],
return 0;
}
+static int veth_alloc_queues(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+
+ priv->rq = kcalloc(dev->num_rx_queues, sizeof(*priv->rq), GFP_KERNEL);
+ if (!priv->rq)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void veth_free_queues(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+
+ kfree(priv->rq);
+}
+
static struct rtnl_link_ops veth_link_ops;
static int veth_newlink(struct net *src_net, struct net_device *dev,
struct nlattr *tb[], struct nlattr *data[],
struct netlink_ext_ack *extack)
{
- int err;
+ int err, i;
struct net_device *peer;
struct veth_priv *priv;
char ifname[IFNAMSIZ];
@@ -1033,6 +1115,12 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
return PTR_ERR(peer);
}
+ err = veth_alloc_queues(peer);
+ if (err) {
+ put_net(net);
+ goto err_peer_alloc_queues;
+ }
+
if (!ifmp || !tbp[IFLA_ADDRESS])
eth_hw_addr_random(peer);
@@ -1061,6 +1149,10 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
* should be re-allocated
*/
+ err = veth_alloc_queues(dev);
+ if (err)
+ goto err_alloc_queues;
+
if (tb[IFLA_ADDRESS] == NULL)
eth_hw_addr_random(dev);
@@ -1080,22 +1172,28 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
*/
priv = netdev_priv(dev);
- priv->dev = dev;
+ for (i = 0; i < dev->real_num_rx_queues; i++)
+ priv->rq[i].dev = dev;
rcu_assign_pointer(priv->peer, peer);
priv = netdev_priv(peer);
- priv->dev = peer;
+ for (i = 0; i < peer->real_num_rx_queues; i++)
+ priv->rq[i].dev = peer;
rcu_assign_pointer(priv->peer, dev);
return 0;
err_register_dev:
+ veth_free_queues(dev);
+err_alloc_queues:
/* nothing to do */
err_configure_peer:
unregister_netdevice(peer);
return err;
err_register_peer:
+ veth_free_queues(peer);
+err_peer_alloc_queues:
free_netdev(peer);
return err;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 bpf-next 8/9] veth: Add XDP TX and REDIRECT
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
In-Reply-To: <1532947431-2737-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
This allows further redirection of xdp_frames like
NIC -> veth--veth -> veth--veth
(XDP) (XDP) (XDP)
The intermediate XDP, redirecting packets from NIC to the other veth,
reuses xdp_mem_info from NIC so that page recycling of the NIC works on
the destination veth's XDP.
In this way return_frame is not fully guarded by NAPI, since another
NAPI handler on another cpu may use the same xdp_mem_info concurrently.
Thus disable napi_direct by xdp_set_return_frame_no_direct() during the
NAPI context.
v4:
- Use xdp_[set|clear]_return_frame_no_direct() instead of a flag in
xdp_mem_info.
v3:
- Fix double free when veth_xdp_tx() returns a positive value.
- Convert xdp_xmit and xdp_redir variables into flags.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/net/veth.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 110 insertions(+), 9 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index c13f7a4..b1ce3691 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -32,6 +32,10 @@
#define VETH_RING_SIZE 256
#define VETH_XDP_HEADROOM (XDP_PACKET_HEADROOM + NET_IP_ALIGN)
+/* Separating two types of XDP xmit */
+#define VETH_XDP_TX BIT(0)
+#define VETH_XDP_REDIR BIT(1)
+
struct pcpu_vstats {
u64 packets;
u64 bytes;
@@ -45,6 +49,7 @@ struct veth_priv {
struct bpf_prog *_xdp_prog;
struct net_device __rcu *peer;
atomic64_t dropped;
+ struct xdp_mem_info xdp_mem;
unsigned requested_headroom;
bool rx_notify_masked;
struct ptr_ring xdp_ring;
@@ -317,10 +322,42 @@ static int veth_xdp_xmit(struct net_device *dev, int n,
return n - drops;
}
+static void veth_xdp_flush(struct net_device *dev)
+{
+ struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
+ struct net_device *rcv;
+
+ rcu_read_lock();
+ rcv = rcu_dereference(priv->peer);
+ if (unlikely(!rcv))
+ goto out;
+
+ rcv_priv = netdev_priv(rcv);
+ /* xdp_ring is initialized on receive side? */
+ if (unlikely(!rcu_access_pointer(rcv_priv->xdp_prog)))
+ goto out;
+
+ __veth_xdp_flush(rcv_priv);
+out:
+ rcu_read_unlock();
+}
+
+static int veth_xdp_tx(struct net_device *dev, struct xdp_buff *xdp)
+{
+ struct xdp_frame *frame = convert_to_xdp_frame(xdp);
+
+ if (unlikely(!frame))
+ return -EOVERFLOW;
+
+ return veth_xdp_xmit(dev, 1, &frame, 0);
+}
+
static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
- struct xdp_frame *frame)
+ struct xdp_frame *frame,
+ unsigned int *xdp_xmit)
{
int len = frame->len, delta = 0;
+ struct xdp_frame orig_frame;
struct bpf_prog *xdp_prog;
unsigned int headroom;
struct sk_buff *skb;
@@ -344,6 +381,29 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
delta = frame->data - xdp.data;
len = xdp.data_end - xdp.data;
break;
+ case XDP_TX:
+ orig_frame = *frame;
+ xdp.data_hard_start = frame;
+ xdp.rxq->mem = frame->mem;
+ if (unlikely(veth_xdp_tx(priv->dev, &xdp) < 0)) {
+ trace_xdp_exception(priv->dev, xdp_prog, act);
+ frame = &orig_frame;
+ goto err_xdp;
+ }
+ *xdp_xmit |= VETH_XDP_TX;
+ rcu_read_unlock();
+ goto xdp_xmit;
+ case XDP_REDIRECT:
+ orig_frame = *frame;
+ xdp.data_hard_start = frame;
+ xdp.rxq->mem = frame->mem;
+ if (xdp_do_redirect(priv->dev, &xdp, xdp_prog)) {
+ frame = &orig_frame;
+ goto err_xdp;
+ }
+ *xdp_xmit |= VETH_XDP_REDIR;
+ rcu_read_unlock();
+ goto xdp_xmit;
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
@@ -368,12 +428,13 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
err_xdp:
rcu_read_unlock();
xdp_return_frame(frame);
-
+xdp_xmit:
return NULL;
}
static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
- struct sk_buff *skb)
+ struct sk_buff *skb,
+ unsigned int *xdp_xmit)
{
u32 pktlen, headroom, act, metalen;
void *orig_data, *orig_data_end;
@@ -445,6 +506,26 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
switch (act) {
case XDP_PASS:
break;
+ case XDP_TX:
+ get_page(virt_to_page(xdp.data));
+ consume_skb(skb);
+ xdp.rxq->mem = priv->xdp_mem;
+ if (unlikely(veth_xdp_tx(priv->dev, &xdp) < 0)) {
+ trace_xdp_exception(priv->dev, xdp_prog, act);
+ goto err_xdp;
+ }
+ *xdp_xmit |= VETH_XDP_TX;
+ rcu_read_unlock();
+ goto xdp_xmit;
+ case XDP_REDIRECT:
+ get_page(virt_to_page(xdp.data));
+ consume_skb(skb);
+ xdp.rxq->mem = priv->xdp_mem;
+ if (xdp_do_redirect(priv->dev, &xdp, xdp_prog))
+ goto err_xdp;
+ *xdp_xmit |= VETH_XDP_REDIR;
+ rcu_read_unlock();
+ goto xdp_xmit;
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
@@ -475,9 +556,15 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
rcu_read_unlock();
kfree_skb(skb);
return NULL;
+err_xdp:
+ rcu_read_unlock();
+ page_frag_free(xdp.data);
+xdp_xmit:
+ return NULL;
}
-static int veth_xdp_rcv(struct veth_priv *priv, int budget)
+static int veth_xdp_rcv(struct veth_priv *priv, int budget,
+ unsigned int *xdp_xmit)
{
int i, done = 0;
@@ -488,10 +575,12 @@ static int veth_xdp_rcv(struct veth_priv *priv, int budget)
if (!ptr)
break;
- if (veth_is_xdp_frame(ptr))
- skb = veth_xdp_rcv_one(priv, veth_ptr_to_xdp(ptr));
- else
- skb = veth_xdp_rcv_skb(priv, ptr);
+ if (veth_is_xdp_frame(ptr)) {
+ skb = veth_xdp_rcv_one(priv, veth_ptr_to_xdp(ptr),
+ xdp_xmit);
+ } else {
+ skb = veth_xdp_rcv_skb(priv, ptr, xdp_xmit);
+ }
if (skb)
napi_gro_receive(&priv->xdp_napi, skb);
@@ -506,9 +595,11 @@ static int veth_poll(struct napi_struct *napi, int budget)
{
struct veth_priv *priv =
container_of(napi, struct veth_priv, xdp_napi);
+ unsigned int xdp_xmit = 0;
int done;
- done = veth_xdp_rcv(priv, budget);
+ xdp_set_return_frame_no_direct();
+ done = veth_xdp_rcv(priv, budget, &xdp_xmit);
if (done < budget && napi_complete_done(napi, done)) {
/* Write rx_notify_masked before reading ptr_ring */
@@ -519,6 +610,12 @@ static int veth_poll(struct napi_struct *napi, int budget)
}
}
+ if (xdp_xmit & VETH_XDP_TX)
+ veth_xdp_flush(priv->dev);
+ if (xdp_xmit & VETH_XDP_REDIR)
+ xdp_do_flush_map();
+ xdp_clear_return_frame_no_direct();
+
return done;
}
@@ -565,6 +662,9 @@ static int veth_enable_xdp(struct net_device *dev)
err = veth_napi_add(dev);
if (err)
goto err;
+
+ /* Save original mem info as it can be overwritten */
+ priv->xdp_mem = priv->xdp_rxq.mem;
}
rcu_assign_pointer(priv->xdp_prog, priv->_xdp_prog);
@@ -582,6 +682,7 @@ static void veth_disable_xdp(struct net_device *dev)
rcu_assign_pointer(priv->xdp_prog, NULL);
veth_napi_del(dev);
+ priv->xdp_rxq.mem = priv->xdp_mem;
xdp_rxq_info_unreg(&priv->xdp_rxq);
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 bpf-next 7/9] xdp: Helpers for disabling napi_direct of xdp_return_frame
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
In-Reply-To: <1532947431-2737-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
We need some mechanism to disable napi_direct on calling
xdp_return_frame_rx_napi() from some context.
When veth gets support of XDP_REDIRECT, it will redirects packets which
are redirected from other devices. On redirection veth will reuse
xdp_mem_info of the redirection source device to make return_frame work.
But in this case .ndo_xdp_xmit() called from veth redirection uses
xdp_mem_info which is not guarded by NAPI, because the .ndo_xdp_xmit()
is not called directly from the rxq which owns the xdp_mem_info.
This approach introduces a flag in bpf_redirect_info to indicate that
napi_direct should be disabled even when _rx_napi variant is used as
well as helper functions to use it.
A NAPI handler who wants to use this flag needs to call
xdp_set_return_frame_no_direct() before processing packets, and call
xdp_clear_return_frame_no_direct() after xdp_do_flush_map() before
exiting NAPI.
v4:
- Use bpf_redirect_info for storing the flag instead of xdp_mem_info to
avoid per-frame copy cost.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
include/linux/filter.h | 25 +++++++++++++++++++++++++
net/core/xdp.c | 6 ++++--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 4717af8..2b072da 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -543,10 +543,14 @@ struct bpf_redirect_info {
struct bpf_map *map;
struct bpf_map *map_to_flush;
unsigned long map_owner;
+ u32 kern_flags;
};
DECLARE_PER_CPU(struct bpf_redirect_info, bpf_redirect_info);
+/* flags for bpf_redirect_info kern_flags */
+#define BPF_RI_F_RF_NO_DIRECT BIT(0) /* no napi_direct on return_frame */
+
/* Compute the linear packet data range [data, data_end) which
* will be accessed by various program types (cls_bpf, act_bpf,
* lwt, ...). Subsystems allowing direct data access must (!)
@@ -775,6 +779,27 @@ static inline bool bpf_dump_raw_ok(void)
struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off,
const struct bpf_insn *patch, u32 len);
+static inline bool xdp_return_frame_no_direct(void)
+{
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
+
+ return ri->kern_flags & BPF_RI_F_RF_NO_DIRECT;
+}
+
+static inline void xdp_set_return_frame_no_direct(void)
+{
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
+
+ ri->kern_flags |= BPF_RI_F_RF_NO_DIRECT;
+}
+
+static inline void xdp_clear_return_frame_no_direct(void)
+{
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
+
+ ri->kern_flags &= ~BPF_RI_F_RF_NO_DIRECT;
+}
+
static inline int xdp_ok_fwd_dev(const struct net_device *fwd,
unsigned int pktlen)
{
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 5728538..3dd99e1 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -330,10 +330,12 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
/* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
page = virt_to_head_page(data);
- if (xa)
+ if (xa) {
+ napi_direct &= !xdp_return_frame_no_direct();
page_pool_put_page(xa->page_pool, page, napi_direct);
- else
+ } else {
put_page(page);
+ }
rcu_read_unlock();
break;
case MEM_TYPE_PAGE_SHARED:
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 bpf-next 6/9] bpf: Make redirect_info accessible from modules
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
In-Reply-To: <1532947431-2737-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
We are going to add kern_flags field in redirect_info for kernel
internal use.
In order to avoid function call to access the flags, make redirect_info
accessible from modules. Also as it is now non-static, add prefix bpf_
to redirect_info.
v6:
- Fix sparse warning around EXPORT_SYMBOL.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
include/linux/filter.h | 10 ++++++++++
net/core/filter.c | 29 +++++++++++------------------
2 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index c73dd73..4717af8 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -537,6 +537,16 @@ struct sk_msg_buff {
struct list_head list;
};
+struct bpf_redirect_info {
+ u32 ifindex;
+ u32 flags;
+ struct bpf_map *map;
+ struct bpf_map *map_to_flush;
+ unsigned long map_owner;
+};
+
+DECLARE_PER_CPU(struct bpf_redirect_info, bpf_redirect_info);
+
/* Compute the linear packet data range [data, data_end) which
* will be accessed by various program types (cls_bpf, act_bpf,
* lwt, ...). Subsystems allowing direct data access must (!)
diff --git a/net/core/filter.c b/net/core/filter.c
index 104d560..2766a55 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2080,19 +2080,12 @@ static int __bpf_redirect(struct sk_buff *skb, struct net_device *dev,
.arg3_type = ARG_ANYTHING,
};
-struct redirect_info {
- u32 ifindex;
- u32 flags;
- struct bpf_map *map;
- struct bpf_map *map_to_flush;
- unsigned long map_owner;
-};
-
-static DEFINE_PER_CPU(struct redirect_info, redirect_info);
+DEFINE_PER_CPU(struct bpf_redirect_info, bpf_redirect_info);
+EXPORT_PER_CPU_SYMBOL_GPL(bpf_redirect_info);
BPF_CALL_2(bpf_redirect, u32, ifindex, u64, flags)
{
- struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
if (unlikely(flags & ~(BPF_F_INGRESS)))
return TC_ACT_SHOT;
@@ -2105,7 +2098,7 @@ struct redirect_info {
int skb_do_redirect(struct sk_buff *skb)
{
- struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
struct net_device *dev;
dev = dev_get_by_index_rcu(dev_net(skb->dev), ri->ifindex);
@@ -3198,7 +3191,7 @@ static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
void xdp_do_flush_map(void)
{
- struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
struct bpf_map *map = ri->map_to_flush;
ri->map_to_flush = NULL;
@@ -3243,7 +3236,7 @@ static inline bool xdp_map_invalid(const struct bpf_prog *xdp_prog,
static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
- struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
unsigned long map_owner = ri->map_owner;
struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
@@ -3283,7 +3276,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
- struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
struct net_device *fwd;
u32 index = ri->ifindex;
int err;
@@ -3315,7 +3308,7 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
- struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
unsigned long map_owner = ri->map_owner;
struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
@@ -3366,7 +3359,7 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
struct xdp_buff *xdp, struct bpf_prog *xdp_prog)
{
- struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
u32 index = ri->ifindex;
struct net_device *fwd;
int err = 0;
@@ -3397,7 +3390,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
BPF_CALL_2(bpf_xdp_redirect, u32, ifindex, u64, flags)
{
- struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
if (unlikely(flags))
return XDP_ABORTED;
@@ -3421,7 +3414,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
BPF_CALL_4(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags,
unsigned long, map_owner)
{
- struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+ struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
if (unlikely(flags))
return XDP_ABORTED;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 bpf-next 5/9] veth: Add ndo_xdp_xmit
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
In-Reply-To: <1532947431-2737-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
This allows NIC's XDP to redirect packets to veth. The destination veth
device enqueues redirected packets to the napi ring of its peer, then
they are processed by XDP on its peer veth device.
This can be thought as calling another XDP program by XDP program using
REDIRECT, when the peer enables driver XDP.
Note that when the peer veth device does not set driver xdp, redirected
packets will be dropped because the peer is not ready for NAPI.
v4:
- Don't use xdp_ok_fwd_dev() because checking IFF_UP is not necessary.
Add comments about it and check only MTU.
v2:
- Drop the part converting xdp_frame into skb when XDP is not enabled.
- Implement bulk interface of ndo_xdp_xmit.
- Implement XDP_XMIT_FLUSH bit and drop ndo_xdp_flush.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: John Fastabend <john.fastabend@gmail.com>
---
drivers/net/veth.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 9de0e90..c13f7a4 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -17,6 +17,7 @@
#include <net/rtnetlink.h>
#include <net/dst.h>
#include <net/xfrm.h>
+#include <net/xdp.h>
#include <linux/veth.h>
#include <linux/module.h>
#include <linux/bpf.h>
@@ -125,6 +126,11 @@ static void *veth_ptr_to_xdp(void *ptr)
return (void *)((unsigned long)ptr & ~VETH_XDP_FLAG);
}
+static void *veth_xdp_to_ptr(void *ptr)
+{
+ return (void *)((unsigned long)ptr | VETH_XDP_FLAG);
+}
+
static void veth_ptr_free(void *ptr)
{
if (veth_is_xdp_frame(ptr))
@@ -267,6 +273,50 @@ static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
return skb;
}
+static int veth_xdp_xmit(struct net_device *dev, int n,
+ struct xdp_frame **frames, u32 flags)
+{
+ struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
+ struct net_device *rcv;
+ unsigned int max_len;
+ int i, drops = 0;
+
+ if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
+ return -EINVAL;
+
+ rcv = rcu_dereference(priv->peer);
+ if (unlikely(!rcv))
+ return -ENXIO;
+
+ rcv_priv = netdev_priv(rcv);
+ /* Non-NULL xdp_prog ensures that xdp_ring is initialized on receive
+ * side. This means an XDP program is loaded on the peer and the peer
+ * device is up.
+ */
+ if (!rcu_access_pointer(rcv_priv->xdp_prog))
+ return -ENXIO;
+
+ max_len = rcv->mtu + rcv->hard_header_len + VLAN_HLEN;
+
+ spin_lock(&rcv_priv->xdp_ring.producer_lock);
+ for (i = 0; i < n; i++) {
+ struct xdp_frame *frame = frames[i];
+ void *ptr = veth_xdp_to_ptr(frame);
+
+ if (unlikely(frame->len > max_len ||
+ __ptr_ring_produce(&rcv_priv->xdp_ring, ptr))) {
+ xdp_return_frame_rx_napi(frame);
+ drops++;
+ }
+ }
+ spin_unlock(&rcv_priv->xdp_ring.producer_lock);
+
+ if (flags & XDP_XMIT_FLUSH)
+ __veth_xdp_flush(rcv_priv);
+
+ return n - drops;
+}
+
static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
struct xdp_frame *frame)
{
@@ -767,6 +817,7 @@ static int veth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
.ndo_features_check = passthru_features_check,
.ndo_set_rx_headroom = veth_set_rx_headroom,
.ndo_bpf = veth_xdp,
+ .ndo_xdp_xmit = veth_xdp_xmit,
};
#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 bpf-next 4/9] veth: Handle xdp_frames in xdp napi ring
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
In-Reply-To: <1532947431-2737-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
This is preparation for XDP TX and ndo_xdp_xmit.
This allows napi handler to handle xdp_frames through xdp ring as well
as sk_buff.
v3:
- Revert v2 change around rings and use a flag to differentiate skb and
xdp_frame, since bulk skb xmit makes little performance difference
for now.
v2:
- Use another ring instead of using flag to differentiate skb and
xdp_frame. This approach makes bulk skb transmit possible in
veth_xmit later.
- Clear xdp_frame feilds in skb->head.
- Implement adjust_tail.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: John Fastabend <john.fastabend@gmail.com>
---
drivers/net/veth.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 82 insertions(+), 5 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 9edf104..9de0e90 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -22,12 +22,12 @@
#include <linux/bpf.h>
#include <linux/filter.h>
#include <linux/ptr_ring.h>
-#include <linux/skb_array.h>
#include <linux/bpf_trace.h>
#define DRV_NAME "veth"
#define DRV_VERSION "1.0"
+#define VETH_XDP_FLAG BIT(0)
#define VETH_RING_SIZE 256
#define VETH_XDP_HEADROOM (XDP_PACKET_HEADROOM + NET_IP_ALIGN)
@@ -115,6 +115,24 @@ static void veth_get_ethtool_stats(struct net_device *dev,
/* general routines */
+static bool veth_is_xdp_frame(void *ptr)
+{
+ return (unsigned long)ptr & VETH_XDP_FLAG;
+}
+
+static void *veth_ptr_to_xdp(void *ptr)
+{
+ return (void *)((unsigned long)ptr & ~VETH_XDP_FLAG);
+}
+
+static void veth_ptr_free(void *ptr)
+{
+ if (veth_is_xdp_frame(ptr))
+ xdp_return_frame(veth_ptr_to_xdp(ptr));
+ else
+ kfree_skb(ptr);
+}
+
static void __veth_xdp_flush(struct veth_priv *priv)
{
/* Write ptr_ring before reading rx_notify_masked */
@@ -249,6 +267,61 @@ static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
return skb;
}
+static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
+ struct xdp_frame *frame)
+{
+ int len = frame->len, delta = 0;
+ struct bpf_prog *xdp_prog;
+ unsigned int headroom;
+ struct sk_buff *skb;
+
+ rcu_read_lock();
+ xdp_prog = rcu_dereference(priv->xdp_prog);
+ if (likely(xdp_prog)) {
+ struct xdp_buff xdp;
+ u32 act;
+
+ xdp.data_hard_start = frame->data - frame->headroom;
+ xdp.data = frame->data;
+ xdp.data_end = frame->data + frame->len;
+ xdp.data_meta = frame->data - frame->metasize;
+ xdp.rxq = &priv->xdp_rxq;
+
+ act = bpf_prog_run_xdp(xdp_prog, &xdp);
+
+ switch (act) {
+ case XDP_PASS:
+ delta = frame->data - xdp.data;
+ len = xdp.data_end - xdp.data;
+ break;
+ default:
+ bpf_warn_invalid_xdp_action(act);
+ case XDP_ABORTED:
+ trace_xdp_exception(priv->dev, xdp_prog, act);
+ case XDP_DROP:
+ goto err_xdp;
+ }
+ }
+ rcu_read_unlock();
+
+ headroom = frame->data - delta - (void *)frame;
+ skb = veth_build_skb(frame, headroom, len, 0);
+ if (!skb) {
+ xdp_return_frame(frame);
+ goto err;
+ }
+
+ memset(frame, 0, sizeof(*frame));
+ skb->protocol = eth_type_trans(skb, priv->dev);
+err:
+ return skb;
+err_xdp:
+ rcu_read_unlock();
+ xdp_return_frame(frame);
+
+ return NULL;
+}
+
static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
struct sk_buff *skb)
{
@@ -359,12 +432,16 @@ static int veth_xdp_rcv(struct veth_priv *priv, int budget)
int i, done = 0;
for (i = 0; i < budget; i++) {
- struct sk_buff *skb = __ptr_ring_consume(&priv->xdp_ring);
+ void *ptr = __ptr_ring_consume(&priv->xdp_ring);
+ struct sk_buff *skb;
- if (!skb)
+ if (!ptr)
break;
- skb = veth_xdp_rcv_skb(priv, skb);
+ if (veth_is_xdp_frame(ptr))
+ skb = veth_xdp_rcv_one(priv, veth_ptr_to_xdp(ptr));
+ else
+ skb = veth_xdp_rcv_skb(priv, ptr);
if (skb)
napi_gro_receive(&priv->xdp_napi, skb);
@@ -417,7 +494,7 @@ static void veth_napi_del(struct net_device *dev)
napi_disable(&priv->xdp_napi);
netif_napi_del(&priv->xdp_napi);
priv->rx_notify_masked = false;
- ptr_ring_cleanup(&priv->xdp_ring, __skb_array_destroy_skb);
+ ptr_ring_cleanup(&priv->xdp_ring, veth_ptr_free);
}
static int veth_enable_xdp(struct net_device *dev)
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 bpf-next 3/9] veth: Avoid drops by oversized packets when XDP is enabled
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
In-Reply-To: <1532947431-2737-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
Oversized packets including GSO packets can be dropped if XDP is
enabled on receiver side, so don't send such packets from peer.
Drop TSO and SCTP fragmentation features so that veth devices themselves
segment packets with XDP enabled. Also cap MTU accordingly.
v4:
- Don't auto-adjust MTU but cap max MTU.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/net/veth.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index d3b9f10..9edf104 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -543,6 +543,23 @@ static int veth_get_iflink(const struct net_device *dev)
return iflink;
}
+static netdev_features_t veth_fix_features(struct net_device *dev,
+ netdev_features_t features)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ struct net_device *peer;
+
+ peer = rtnl_dereference(priv->peer);
+ if (peer) {
+ struct veth_priv *peer_priv = netdev_priv(peer);
+
+ if (peer_priv->_xdp_prog)
+ features &= ~NETIF_F_GSO_SOFTWARE;
+ }
+
+ return features;
+}
+
static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
{
struct veth_priv *peer_priv, *priv = netdev_priv(dev);
@@ -572,6 +589,7 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
struct veth_priv *priv = netdev_priv(dev);
struct bpf_prog *old_prog;
struct net_device *peer;
+ unsigned int max_mtu;
int err;
old_prog = priv->_xdp_prog;
@@ -585,6 +603,15 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
goto err;
}
+ max_mtu = PAGE_SIZE - VETH_XDP_HEADROOM -
+ peer->hard_header_len -
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ if (peer->mtu > max_mtu) {
+ NL_SET_ERR_MSG_MOD(extack, "Peer MTU is too large to set XDP");
+ err = -ERANGE;
+ goto err;
+ }
+
if (dev->flags & IFF_UP) {
err = veth_enable_xdp(dev);
if (err) {
@@ -592,14 +619,29 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
goto err;
}
}
+
+ if (!old_prog) {
+ peer->hw_features &= ~NETIF_F_GSO_SOFTWARE;
+ peer->max_mtu = max_mtu;
+ }
}
if (old_prog) {
- if (!prog && dev->flags & IFF_UP)
- veth_disable_xdp(dev);
+ if (!prog) {
+ if (dev->flags & IFF_UP)
+ veth_disable_xdp(dev);
+
+ if (peer) {
+ peer->hw_features |= NETIF_F_GSO_SOFTWARE;
+ peer->max_mtu = ETH_MAX_MTU;
+ }
+ }
bpf_prog_put(old_prog);
}
+ if ((!!old_prog ^ !!prog) && peer)
+ netdev_update_features(peer);
+
return 0;
err:
priv->_xdp_prog = old_prog;
@@ -644,6 +686,7 @@ static int veth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
.ndo_poll_controller = veth_poll_controller,
#endif
.ndo_get_iflink = veth_get_iflink,
+ .ndo_fix_features = veth_fix_features,
.ndo_features_check = passthru_features_check,
.ndo_set_rx_headroom = veth_set_rx_headroom,
.ndo_bpf = veth_xdp,
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 bpf-next 2/9] veth: Add driver XDP
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
In-Reply-To: <1532947431-2737-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
This is the basic implementation of veth driver XDP.
Incoming packets are sent from the peer veth device in the form of skb,
so this is generally doing the same thing as generic XDP.
This itself is not so useful, but a starting point to implement other
useful veth XDP features like TX and REDIRECT.
This introduces NAPI when XDP is enabled, because XDP is now heavily
relies on NAPI context. Use ptr_ring to emulate NIC ring. Tx function
enqueues packets to the ring and peer NAPI handler drains the ring.
Currently only one ring is allocated for each veth device, so it does
not scale on multiqueue env. This can be resolved by allocating rings
on the per-queue basis later.
Note that NAPI is not used but netif_rx is used when XDP is not loaded,
so this does not change the default behaviour.
v6:
- Check skb->len only when allocation is needed.
- Add __GFP_NOWARN to alloc_page() as it can be triggered by external
events.
v3:
- Fix race on closing the device.
- Add extack messages in ndo_bpf.
v2:
- Squashed with the patch adding NAPI.
- Implement adjust_tail.
- Don't acquire consumer lock because it is guarded by NAPI.
- Make poll_controller noop since it is unnecessary.
- Register rxq_info on enabling XDP rather than on opening the device.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/net/veth.c | 374 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 367 insertions(+), 7 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index a69ad39..d3b9f10 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -19,10 +19,18 @@
#include <net/xfrm.h>
#include <linux/veth.h>
#include <linux/module.h>
+#include <linux/bpf.h>
+#include <linux/filter.h>
+#include <linux/ptr_ring.h>
+#include <linux/skb_array.h>
+#include <linux/bpf_trace.h>
#define DRV_NAME "veth"
#define DRV_VERSION "1.0"
+#define VETH_RING_SIZE 256
+#define VETH_XDP_HEADROOM (XDP_PACKET_HEADROOM + NET_IP_ALIGN)
+
struct pcpu_vstats {
u64 packets;
u64 bytes;
@@ -30,9 +38,16 @@ struct pcpu_vstats {
};
struct veth_priv {
+ struct napi_struct xdp_napi;
+ struct net_device *dev;
+ struct bpf_prog __rcu *xdp_prog;
+ struct bpf_prog *_xdp_prog;
struct net_device __rcu *peer;
atomic64_t dropped;
unsigned requested_headroom;
+ bool rx_notify_masked;
+ struct ptr_ring xdp_ring;
+ struct xdp_rxq_info xdp_rxq;
};
/*
@@ -98,11 +113,43 @@ static void veth_get_ethtool_stats(struct net_device *dev,
.get_link_ksettings = veth_get_link_ksettings,
};
-static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
+/* general routines */
+
+static void __veth_xdp_flush(struct veth_priv *priv)
+{
+ /* Write ptr_ring before reading rx_notify_masked */
+ smp_mb();
+ if (!priv->rx_notify_masked) {
+ priv->rx_notify_masked = true;
+ napi_schedule(&priv->xdp_napi);
+ }
+}
+
+static int veth_xdp_rx(struct veth_priv *priv, struct sk_buff *skb)
+{
+ if (unlikely(ptr_ring_produce(&priv->xdp_ring, skb))) {
+ dev_kfree_skb_any(skb);
+ return NET_RX_DROP;
+ }
+
+ return NET_RX_SUCCESS;
+}
+
+static int veth_forward_skb(struct net_device *dev, struct sk_buff *skb, bool xdp)
{
struct veth_priv *priv = netdev_priv(dev);
+
+ return __dev_forward_skb(dev, skb) ?: xdp ?
+ veth_xdp_rx(priv, skb) :
+ netif_rx(skb);
+}
+
+static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
struct net_device *rcv;
int length = skb->len;
+ bool rcv_xdp = false;
rcu_read_lock();
rcv = rcu_dereference(priv->peer);
@@ -111,7 +158,10 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
goto drop;
}
- if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
+ rcv_priv = netdev_priv(rcv);
+ rcv_xdp = rcu_access_pointer(rcv_priv->xdp_prog);
+
+ if (likely(veth_forward_skb(rcv, skb, rcv_xdp) == NET_RX_SUCCESS)) {
struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
u64_stats_update_begin(&stats->syncp);
@@ -122,14 +172,15 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
drop:
atomic64_inc(&priv->dropped);
}
+
+ if (rcv_xdp)
+ __veth_xdp_flush(rcv_priv);
+
rcu_read_unlock();
+
return NETDEV_TX_OK;
}
-/*
- * general routines
- */
-
static u64 veth_stats_one(struct pcpu_vstats *result, struct net_device *dev)
{
struct veth_priv *priv = netdev_priv(dev);
@@ -179,18 +230,254 @@ static void veth_set_multicast_list(struct net_device *dev)
{
}
+static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
+ int buflen)
+{
+ struct sk_buff *skb;
+
+ if (!buflen) {
+ buflen = SKB_DATA_ALIGN(headroom + len) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ }
+ skb = build_skb(head, buflen);
+ if (!skb)
+ return NULL;
+
+ skb_reserve(skb, headroom);
+ skb_put(skb, len);
+
+ return skb;
+}
+
+static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
+ struct sk_buff *skb)
+{
+ u32 pktlen, headroom, act, metalen;
+ void *orig_data, *orig_data_end;
+ struct bpf_prog *xdp_prog;
+ int mac_len, delta, off;
+ struct xdp_buff xdp;
+
+ rcu_read_lock();
+ xdp_prog = rcu_dereference(priv->xdp_prog);
+ if (unlikely(!xdp_prog)) {
+ rcu_read_unlock();
+ goto out;
+ }
+
+ mac_len = skb->data - skb_mac_header(skb);
+ pktlen = skb->len + mac_len;
+ headroom = skb_headroom(skb) - mac_len;
+
+ if (skb_shared(skb) || skb_head_is_locked(skb) ||
+ skb_is_nonlinear(skb) || headroom < XDP_PACKET_HEADROOM) {
+ struct sk_buff *nskb;
+ int size, head_off;
+ void *head, *start;
+ struct page *page;
+
+ size = SKB_DATA_ALIGN(VETH_XDP_HEADROOM + pktlen) +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+ if (size > PAGE_SIZE)
+ goto drop;
+
+ page = alloc_page(GFP_ATOMIC | __GFP_NOWARN);
+ if (!page)
+ goto drop;
+
+ head = page_address(page);
+ start = head + VETH_XDP_HEADROOM;
+ if (skb_copy_bits(skb, -mac_len, start, pktlen)) {
+ page_frag_free(head);
+ goto drop;
+ }
+
+ nskb = veth_build_skb(head,
+ VETH_XDP_HEADROOM + mac_len, skb->len,
+ PAGE_SIZE);
+ if (!nskb) {
+ page_frag_free(head);
+ goto drop;
+ }
+
+ skb_copy_header(nskb, skb);
+ head_off = skb_headroom(nskb) - skb_headroom(skb);
+ skb_headers_offset_update(nskb, head_off);
+ if (skb->sk)
+ skb_set_owner_w(nskb, skb->sk);
+ consume_skb(skb);
+ skb = nskb;
+ }
+
+ xdp.data_hard_start = skb->head;
+ xdp.data = skb_mac_header(skb);
+ xdp.data_end = xdp.data + pktlen;
+ xdp.data_meta = xdp.data;
+ xdp.rxq = &priv->xdp_rxq;
+ orig_data = xdp.data;
+ orig_data_end = xdp.data_end;
+
+ act = bpf_prog_run_xdp(xdp_prog, &xdp);
+
+ switch (act) {
+ case XDP_PASS:
+ break;
+ default:
+ bpf_warn_invalid_xdp_action(act);
+ case XDP_ABORTED:
+ trace_xdp_exception(priv->dev, xdp_prog, act);
+ case XDP_DROP:
+ goto drop;
+ }
+ rcu_read_unlock();
+
+ delta = orig_data - xdp.data;
+ off = mac_len + delta;
+ if (off > 0)
+ __skb_push(skb, off);
+ else if (off < 0)
+ __skb_pull(skb, -off);
+ skb->mac_header -= delta;
+ off = xdp.data_end - orig_data_end;
+ if (off != 0)
+ __skb_put(skb, off);
+ skb->protocol = eth_type_trans(skb, priv->dev);
+
+ metalen = xdp.data - xdp.data_meta;
+ if (metalen)
+ skb_metadata_set(skb, metalen);
+out:
+ return skb;
+drop:
+ rcu_read_unlock();
+ kfree_skb(skb);
+ return NULL;
+}
+
+static int veth_xdp_rcv(struct veth_priv *priv, int budget)
+{
+ int i, done = 0;
+
+ for (i = 0; i < budget; i++) {
+ struct sk_buff *skb = __ptr_ring_consume(&priv->xdp_ring);
+
+ if (!skb)
+ break;
+
+ skb = veth_xdp_rcv_skb(priv, skb);
+
+ if (skb)
+ napi_gro_receive(&priv->xdp_napi, skb);
+
+ done++;
+ }
+
+ return done;
+}
+
+static int veth_poll(struct napi_struct *napi, int budget)
+{
+ struct veth_priv *priv =
+ container_of(napi, struct veth_priv, xdp_napi);
+ int done;
+
+ done = veth_xdp_rcv(priv, budget);
+
+ if (done < budget && napi_complete_done(napi, done)) {
+ /* Write rx_notify_masked before reading ptr_ring */
+ smp_store_mb(priv->rx_notify_masked, false);
+ if (unlikely(!__ptr_ring_empty(&priv->xdp_ring))) {
+ priv->rx_notify_masked = true;
+ napi_schedule(&priv->xdp_napi);
+ }
+ }
+
+ return done;
+}
+
+static int veth_napi_add(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ int err;
+
+ err = ptr_ring_init(&priv->xdp_ring, VETH_RING_SIZE, GFP_KERNEL);
+ if (err)
+ return err;
+
+ netif_napi_add(dev, &priv->xdp_napi, veth_poll, NAPI_POLL_WEIGHT);
+ napi_enable(&priv->xdp_napi);
+
+ return 0;
+}
+
+static void veth_napi_del(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+
+ napi_disable(&priv->xdp_napi);
+ netif_napi_del(&priv->xdp_napi);
+ priv->rx_notify_masked = false;
+ ptr_ring_cleanup(&priv->xdp_ring, __skb_array_destroy_skb);
+}
+
+static int veth_enable_xdp(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ int err;
+
+ if (!xdp_rxq_info_is_reg(&priv->xdp_rxq)) {
+ err = xdp_rxq_info_reg(&priv->xdp_rxq, dev, 0);
+ if (err < 0)
+ return err;
+
+ err = xdp_rxq_info_reg_mem_model(&priv->xdp_rxq,
+ MEM_TYPE_PAGE_SHARED, NULL);
+ if (err < 0)
+ goto err;
+
+ err = veth_napi_add(dev);
+ if (err)
+ goto err;
+ }
+
+ rcu_assign_pointer(priv->xdp_prog, priv->_xdp_prog);
+
+ return 0;
+err:
+ xdp_rxq_info_unreg(&priv->xdp_rxq);
+
+ return err;
+}
+
+static void veth_disable_xdp(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+
+ rcu_assign_pointer(priv->xdp_prog, NULL);
+ veth_napi_del(dev);
+ xdp_rxq_info_unreg(&priv->xdp_rxq);
+}
+
static int veth_open(struct net_device *dev)
{
struct veth_priv *priv = netdev_priv(dev);
struct net_device *peer = rtnl_dereference(priv->peer);
+ int err;
if (!peer)
return -ENOTCONN;
+ if (priv->_xdp_prog) {
+ err = veth_enable_xdp(dev);
+ if (err)
+ return err;
+ }
+
if (peer->flags & IFF_UP) {
netif_carrier_on(dev);
netif_carrier_on(peer);
}
+
return 0;
}
@@ -203,6 +490,9 @@ static int veth_close(struct net_device *dev)
if (peer)
netif_carrier_off(peer);
+ if (priv->_xdp_prog)
+ veth_disable_xdp(dev);
+
return 0;
}
@@ -228,7 +518,7 @@ static void veth_dev_free(struct net_device *dev)
static void veth_poll_controller(struct net_device *dev)
{
/* veth only receives frames when its peer sends one
- * Since it's a synchronous operation, we are guaranteed
+ * Since it has nothing to do with disabling irqs, we are guaranteed
* never to have pending data when we poll for it so
* there is nothing to do here.
*
@@ -276,6 +566,72 @@ static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
rcu_read_unlock();
}
+static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
+ struct netlink_ext_ack *extack)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ struct bpf_prog *old_prog;
+ struct net_device *peer;
+ int err;
+
+ old_prog = priv->_xdp_prog;
+ priv->_xdp_prog = prog;
+ peer = rtnl_dereference(priv->peer);
+
+ if (prog) {
+ if (!peer) {
+ NL_SET_ERR_MSG_MOD(extack, "Cannot set XDP when peer is detached");
+ err = -ENOTCONN;
+ goto err;
+ }
+
+ if (dev->flags & IFF_UP) {
+ err = veth_enable_xdp(dev);
+ if (err) {
+ NL_SET_ERR_MSG_MOD(extack, "Setup for XDP failed");
+ goto err;
+ }
+ }
+ }
+
+ if (old_prog) {
+ if (!prog && dev->flags & IFF_UP)
+ veth_disable_xdp(dev);
+ bpf_prog_put(old_prog);
+ }
+
+ return 0;
+err:
+ priv->_xdp_prog = old_prog;
+
+ return err;
+}
+
+static u32 veth_xdp_query(struct net_device *dev)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ const struct bpf_prog *xdp_prog;
+
+ xdp_prog = priv->_xdp_prog;
+ if (xdp_prog)
+ return xdp_prog->aux->id;
+
+ return 0;
+}
+
+static int veth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
+{
+ switch (xdp->command) {
+ case XDP_SETUP_PROG:
+ return veth_xdp_set(dev, xdp->prog, xdp->extack);
+ case XDP_QUERY_PROG:
+ xdp->prog_id = veth_xdp_query(dev);
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
static const struct net_device_ops veth_netdev_ops = {
.ndo_init = veth_dev_init,
.ndo_open = veth_open,
@@ -290,6 +646,7 @@ static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
.ndo_get_iflink = veth_get_iflink,
.ndo_features_check = passthru_features_check,
.ndo_set_rx_headroom = veth_set_rx_headroom,
+ .ndo_bpf = veth_xdp,
};
#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
@@ -451,10 +808,13 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
*/
priv = netdev_priv(dev);
+ priv->dev = dev;
rcu_assign_pointer(priv->peer, peer);
priv = netdev_priv(peer);
+ priv->dev = peer;
rcu_assign_pointer(priv->peer, dev);
+
return 0;
err_register_dev:
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 bpf-next 1/9] net: Export skb_headers_offset_update
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
In-Reply-To: <1532947431-2737-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
This is needed for veth XDP which does skb_copy_expand()-like operation.
v2:
- Drop skb_copy_header part because it has already been exported now.
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
include/linux/skbuff.h | 1 +
net/core/skbuff.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index fd3cb1b..f692968 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1035,6 +1035,7 @@ static inline struct sk_buff *alloc_skb_fclone(unsigned int size,
}
struct sk_buff *skb_morph(struct sk_buff *dst, struct sk_buff *src);
+void skb_headers_offset_update(struct sk_buff *skb, int off);
int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask);
struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t priority);
void skb_copy_header(struct sk_buff *new, const struct sk_buff *old);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 266b954..f5670e6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1291,7 +1291,7 @@ struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
}
EXPORT_SYMBOL(skb_clone);
-static void skb_headers_offset_update(struct sk_buff *skb, int off)
+void skb_headers_offset_update(struct sk_buff *skb, int off)
{
/* Only adjust this if it actually is csum_start rather than csum */
if (skb->ip_summed == CHECKSUM_PARTIAL)
@@ -1305,6 +1305,7 @@ static void skb_headers_offset_update(struct sk_buff *skb, int off)
skb->inner_network_header += off;
skb->inner_mac_header += off;
}
+EXPORT_SYMBOL(skb_headers_offset_update);
void skb_copy_header(struct sk_buff *new, const struct sk_buff *old)
{
--
1.8.3.1
^ permalink raw reply related
* [PATCH v6 bpf-next 0/9] veth: Driver XDP
From: Toshiaki Makita @ 2018-07-30 10:43 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
John Fastabend
This patch set introduces driver XDP for veth.
Basically this is used in conjunction with redirect action of another XDP
program.
NIC -----------> veth===veth
(XDP) (redirect) (XDP)
In this case xdp_frame can be forwarded to the peer veth without
modification, so we can expect far better performance than generic XDP.
Envisioned use-cases
--------------------
* Container managed XDP program
Container host redirects frames to containers by XDP redirect action, and
privileged containers can deploy their own XDP programs.
* XDP program cascading
Two or more XDP programs can be called for each packet by redirecting
xdp frames to veth.
* Internal interface for an XDP bridge
When using XDP redirection to create a virtual bridge, veth can be used
to create an internal interface for the bridge.
Implementation
--------------
This changeset is making use of NAPI to implement ndo_xdp_xmit and
XDP_TX/REDIRECT. This is mainly because XDP heavily relies on NAPI
context.
- patch 1: Export a function needed for veth XDP.
- patch 2-3: Basic implementation of veth XDP.
- patch 4-5: Add ndo_xdp_xmit.
- patch 6-8: Add XDP_TX and XDP_REDIRECT.
- patch 9: Performance optimization for multi-queue env.
Tests and performance numbers
-----------------------------
Tested with a simple XDP program which only redirects packets between
NIC and veth. I used i40e 25G NIC (XXV710) for the physical NIC. The
server has 20 of Xeon Silver 2.20 GHz cores.
pktgen --(wire)--> XXV710 (i40e) <--(XDP redirect)--> veth===veth (XDP)
The rightmost veth loads XDP progs and just does DROP or TX. The number
of packets is measured in the XDP progs. The leftmost pktgen sends
packets at 37.1 Mpps (almost 25G wire speed).
veth XDP action Flows Mpps
================================
DROP 1 10.6
DROP 2 21.2
DROP 100 36.0
TX 1 5.0
TX 2 10.0
TX 100 31.0
I also measured netperf TCP_STREAM but was not so great performance due
to lack of tx/rx checksum offload and TSO, etc.
netperf <--(wire)--> XXV710 (i40e) <--(XDP redirect)--> veth===veth (XDP PASS)
Direction Flows Gbps
==============================
external->veth 1 20.8
external->veth 2 23.5
external->veth 100 23.6
veth->external 1 9.0
veth->external 2 17.8
veth->external 100 22.9
Also tested doing ifup/down or load/unload a XDP program repeatedly
during processing XDP packets in order to check if enabling/disabling
NAPI is working as expected, and found no problems.
v6:
- Check skb->len only if reallocation is needed.
- Add __GFP_NOWARN to alloc_page() since it can be triggered by external
events.
- Fix sparse warning around EXPORT_SYMBOL.
v5:
- Fix broken SOBs.
v4:
- Don't adjust MTU automatically.
- Skip peer IFF_UP check on .ndo_xdp_xmit() because it is unnecessary.
Add comments to explain that.
- Use redirect_info instead of xdp_mem_info for storing no_direct flag
to avoid per packet copy cost.
v3:
- Drop skb bulk xmit patch since it makes little performance
difference. The hotspot in TCP skb xmit at this point is checksum
computation in skb_segment and packet copy on XDP_REDIRECT due to
cloned/nonlinear skb.
- Fix race on closing device.
- Add extack messages in ndo_bpf.
v2:
- Squash NAPI patch with "Add driver XDP" patch.
- Remove conversion from xdp_frame to skb when NAPI is not enabled.
- Introduce per-queue XDP ring (patch 8).
- Introduce bulk skb xmit when XDP is enabled on the peer (patch 9).
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Toshiaki Makita (9):
net: Export skb_headers_offset_update
veth: Add driver XDP
veth: Avoid drops by oversized packets when XDP is enabled
veth: Handle xdp_frames in xdp napi ring
veth: Add ndo_xdp_xmit
bpf: Make redirect_info accessible from modules
xdp: Helpers for disabling napi_direct of xdp_return_frame
veth: Add XDP TX and REDIRECT
veth: Support per queue XDP ring
drivers/net/veth.c | 748 ++++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/filter.h | 35 +++
include/linux/skbuff.h | 1 +
net/core/filter.c | 29 +-
net/core/skbuff.c | 3 +-
net/core/xdp.c | 6 +-
6 files changed, 792 insertions(+), 30 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH] 9p: fix Use-After-Free in p9_write_work()
From: Tomas Bortoli @ 2018-07-30 9:45 UTC (permalink / raw)
To: Dominique Martinet; +Cc: davem, v9fs-developer, netdev, linux-kernel, syzkaller
In-Reply-To: <20180729233336.GB28684@nautica>
On 07/30/2018 01:33 AM, Dominique Martinet wrote:
> Tomas Bortoli wrote on Sun, Jul 29, 2018:
>> There is a race condition between p9_free_req() and p9_write_work().
>> A request might still need to be processed while p9_free_req() is called.
>>
>> To fix it, flush the read/write work before freeing any request.
>>
>> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
>> Reported-by: syzbot+467050c1ce275af2a5b8@syzkaller.appspotmail.com
>
> It looks like I have not received this report, I found it through google
> in the lkml archives but Dmitry do you have a convenient-ish way of
> finding the report on the syzkaller website with that reported-by tag?
>
>> ---
>>
>> To be able to flush the r/w work from client.c we need the p9_conn and
>> p9_trans_fd definitions. Therefore this commit moves most of the declarations in
>> trans_fd.c to trans_fd.h and import such file in client.c
>
> This cannot work as it is, because you're not just intorudcing the
> trans_fd types but you're really depending on the transport used being
> fd.
> 'conn.wq' won't even be valid memory in other transports so I don't want
> to know what trying to flush_work on this will do... :)
>
Yep, Oops
>
> Other transports also have the same issue see discussion in
> https://lkml.org/lkml/2018/7/19/727
> (that is another syzbot report, slightly different but I believe it
> points to the same issue)
>
> Basically, a more global view of the problem is a race between
> p9_tag_lookup returning a p9_req_t and another thread freeing it.
>
> Matthew wrote the problem himself in a comment in p9_tag_lookup in his new
> version that used to be in linux-next at the time (I took the commit out
> temporarily until I've had time to benchmark it, but it will come back in,
> just you're working on thin air right now because the bug was only found
> thanks to this commit):
> + /* There's no refcount on the req; a malicious server could
> cause
> + * us to dereference a NULL pointer
> + */
>
> So a more proper solution would be to had a refcount to req, have
> p9_tag_lookup increment the refcount within rcu_read_lock, and have a
> deref function free the req when the count hits 0.
>
>
Which commit ? that's a comment.
That sound like the proper solution. Let's do it that way then.
> Now we're here though I'm not sure what to suggest, I had promised to
> get some performance benchmark out by this past weekend and I obviously
> failed, so this patch might be delayed to 4.20 and the refcount approach
> would not work with the current req cache/reuse system we have right
> now.
> If you want to finish this anyway you can work on my 9p-test branch
> (I've kept the commit there), and I'll take that patch in at the same
> time as the other.
>
>
>> Moreover, a couple of identifiers were altered to avoid name conflicts with the
>> new import.
>
> If we were to stick to this approach, two suggestions:
> - headers aren't all-or-nothing, I think it's better to only expose
> what you need (and e.g. keep the Opt_* enum in the .c)
No we don't have to stick with this patch.
> - instead of exposing trans_fd specific stuff, it's cleaner to add a
> new op vector '.flush' to the p9_trans_module struct and call that if it
> exists.
>
I didn't thought at this way, it's smarter.
>
> Thanks,
>
^ permalink raw reply
* [PATCH net-next] liquidio: remove redundant function cn23xx_dump_iq_regs
From: YueHaibing @ 2018-07-30 11:03 UTC (permalink / raw)
To: davem, derek.chickles, satananda.burla, felix.manlunas,
raghu.vatsavayi
Cc: linux-kernel, netdev, YueHaibing
There are no in-tree callers of cn23xx_dump_iq_regs.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
.../ethernet/cavium/liquidio/cn23xx_pf_device.c | 44 ----------------------
1 file changed, 44 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
index e088ded..9f4f3c1d 100644
--- a/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/cn23xx_pf_device.c
@@ -1417,50 +1417,6 @@ int validate_cn23xx_pf_config_info(struct octeon_device *oct,
return 0;
}
-void cn23xx_dump_iq_regs(struct octeon_device *oct)
-{
- u32 regval, q_no;
-
- dev_dbg(&oct->pci_dev->dev, "SLI_IQ_DOORBELL_0 [0x%x]: 0x%016llx\n",
- CN23XX_SLI_IQ_DOORBELL(0),
- CVM_CAST64(octeon_read_csr64
- (oct, CN23XX_SLI_IQ_DOORBELL(0))));
-
- dev_dbg(&oct->pci_dev->dev, "SLI_IQ_BASEADDR_0 [0x%x]: 0x%016llx\n",
- CN23XX_SLI_IQ_BASE_ADDR64(0),
- CVM_CAST64(octeon_read_csr64
- (oct, CN23XX_SLI_IQ_BASE_ADDR64(0))));
-
- dev_dbg(&oct->pci_dev->dev, "SLI_IQ_FIFO_RSIZE_0 [0x%x]: 0x%016llx\n",
- CN23XX_SLI_IQ_SIZE(0),
- CVM_CAST64(octeon_read_csr64(oct, CN23XX_SLI_IQ_SIZE(0))));
-
- dev_dbg(&oct->pci_dev->dev, "SLI_CTL_STATUS [0x%x]: 0x%016llx\n",
- CN23XX_SLI_CTL_STATUS,
- CVM_CAST64(octeon_read_csr64(oct, CN23XX_SLI_CTL_STATUS)));
-
- for (q_no = 0; q_no < CN23XX_MAX_INPUT_QUEUES; q_no++) {
- dev_dbg(&oct->pci_dev->dev, "SLI_PKT[%d]_INPUT_CTL [0x%x]: 0x%016llx\n",
- q_no, CN23XX_SLI_IQ_PKT_CONTROL64(q_no),
- CVM_CAST64(octeon_read_csr64
- (oct, CN23XX_SLI_IQ_PKT_CONTROL64(q_no))));
- }
-
- pci_read_config_dword(oct->pci_dev, CN23XX_CONFIG_PCIE_DEVCTL, ®val);
- dev_dbg(&oct->pci_dev->dev, "Config DevCtl [0x%x]: 0x%08x\n",
- CN23XX_CONFIG_PCIE_DEVCTL, regval);
-
- dev_dbg(&oct->pci_dev->dev, "SLI_PRT[%d]_CFG [0x%llx]: 0x%016llx\n",
- oct->pcie_port, CN23XX_DPI_SLI_PRTX_CFG(oct->pcie_port),
- CVM_CAST64(lio_pci_readq(
- oct, CN23XX_DPI_SLI_PRTX_CFG(oct->pcie_port))));
-
- dev_dbg(&oct->pci_dev->dev, "SLI_S2M_PORT[%d]_CTL [0x%x]: 0x%016llx\n",
- oct->pcie_port, CN23XX_SLI_S2M_PORTX_CTL(oct->pcie_port),
- CVM_CAST64(octeon_read_csr64(
- oct, CN23XX_SLI_S2M_PORTX_CTL(oct->pcie_port))));
-}
-
int cn23xx_fw_loaded(struct octeon_device *oct)
{
u64 val;
--
2.7.0
^ permalink raw reply related
* Re: [PATCH bpf v2] tools/bpftool: fix a percpu_array map dump problem
From: Daniel Borkmann @ 2018-07-30 9:21 UTC (permalink / raw)
To: Yonghong Song, ast, netdev; +Cc: kernel-team
In-Reply-To: <20180729172039.1620647-1-yhs@fb.com>
Hi Yonghong,
On 07/29/2018 07:20 PM, Yonghong Song wrote:
> I hit the following problem when I tried to use bpftool
> to dump a percpu array.
>
> $ sudo ./bpftool map show
> 61: percpu_array name stub flags 0x0
> key 4B value 4B max_entries 1 memlock 4096B
> ...
> $ sudo ./bpftool map dump id 61
> bpftool: malloc.c:2406: sysmalloc: Assertion
> `(old_top == initial_top (av) && old_size == 0) || \
> ((unsigned long) (old_size) >= MINSIZE && \
> prev_inuse (old_top) && \
> ((unsigned long) old_end & (pagesize - 1)) == 0)'
> failed.
> Aborted
>
> Further debugging revealed that this is due to
> miscommunication between bpftool and kernel.
> For example, for the above percpu_array with value size of 4B.
> The map info returned to user space has value size of 4B.
>
> In bpftool, the values array for lookup is allocated like:
> info->value_size * get_possible_cpus() = 4 * get_possible_cpus()
> In kernel (kernel/bpf/syscall.c), the values array size is
> rounded up to multiple of 8.
> round_up(map->value_size, 8) * num_possible_cpus()
> = 8 * num_possible_cpus()
> So when kernel copies the values to user buffer, the kernel will
> overwrite beyond user buffer boundary.
>
> This patch fixed the issue by allocating and stepping through
> percpu map value array properly in bpftool.
>
> Fixes: 71bb428fe2c19 ("tools: bpf: add bpftool")
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> tools/bpf/bpftool/map.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> Changelogs:
> v1 -> v2:
> . Added missing fix in function print_entry_plain().
The patch does not apply against bpf tree. I think you've rebased that against
bpf-next instead, but the fix really should go into bpf. Please respin against
correct tree.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH] bpf: verifier: BPF_MOV don't mark dst reg if src == dst
From: Daniel Borkmann @ 2018-07-30 9:10 UTC (permalink / raw)
To: Arthur Fabre, Alexei Starovoitov; +Cc: Alexei Starovoitov, Network Development
In-Reply-To: <CAOn4ftuW+8DcdHTEP3CYwRjSEOfeWfoxZ=eUp8aTuLYmrNADXQ@mail.gmail.com>
On 07/30/2018 09:44 AM, Arthur Fabre wrote:
> On Sun, Jul 29, 2018 at 4:59 PM, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>> On Thu, Jul 26, 2018 at 1:08 AM, Arthur Fabre <afabre@cloudflare.com> wrote:
>>> When check_alu_op() handles a BPF_MOV between two registers,
>>> it calls check_reg_arg() on the dst register, marking it as unbounded.
>>> If the src and dst register are the same, this marks the src as
>>> unbounded, which can lead to unexpected errors for further checks that
>>> rely on bounds info.
>>>
>>> check_alu_op() now only marks the dst register as unbounded if it
>>> different from the src register.
>>>
>>> Signed-off-by: Arthur Fabre <afabre@cloudflare.com>
>>> ---
>>> kernel/bpf/verifier.c | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index 63aaac52a265..ddfe3c544a80 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -3238,8 +3238,9 @@ static int check_alu_op(struct bpf_verifier_env
>>> *env, struct bpf_insn *insn)
>>> }
>>> }
>>>
>>> - /* check dest operand */
>>> - err = check_reg_arg(env, insn->dst_reg, DST_OP);
>>> + /* check dest operand, only mark if dest != src */
>>> + err = check_reg_arg(env, insn->dst_reg,
>>> + insn->dst_reg == insn->src_reg ?
>>> DST_OP_NO_MARK : DST_OP);
>>
>> that doesn't look correct for 32-bit mov.
>> Is that the case you're trying to improve?
>
> The patch was originally for 64-bit mov only
Hmm, I'm not sure that is infact the case. The check_alu_op() is handled for
32 and 64 bit alu op case. So in the opcode == BPF_MOV case the check_reg_arg()
on the dst register is done for both at that point, whereas retaining any
current state should only be valid in 64 bit mov case, e.g. think of pointer
types, these really need to be scratched here. I think it would make sense that
after checking src operand we hold a temporary copy of its state and use that
for setting regs[insn->dst_reg] later on under BPF_ALU64.
^ permalink raw reply
* Re: [PATCH] can: ems_usb: Fix memory leak on ems_usb_disconnect
From: Marc Kleine-Budde @ 2018-07-30 9:09 UTC (permalink / raw)
To: Anton Vasilyev, Wolfgang Grandegger
Cc: David S. Miller, Martin Kelly, Martin Lederhilger, linux-can,
netdev, linux-kernel, ldv-project
In-Reply-To: <20180727155042.24582-1-vasilyev@ispras.ru>
[-- Attachment #1.1: Type: text/plain, Size: 610 bytes --]
On 07/27/2018 05:50 PM, Anton Vasilyev wrote:
> ems_usb_probe() allocates memory for dev->tx_msg_buffer, but there
> is no its deallocation in ems_usb_disconnect.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
Applied to linux-can.
Tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH] can: ems_usb: Fix memory leak on ems_usb_disconnect()
From: Marc Kleine-Budde @ 2018-07-30 9:08 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Anton Vasilyev, stable,
Marc Kleine-Budde
In-Reply-To: <20180730090822.16919-1-mkl@pengutronix.de>
From: Anton Vasilyev <vasilyev@ispras.ru>
ems_usb_probe() allocates memory for dev->tx_msg_buffer, but there
is no its deallocation in ems_usb_disconnect().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Anton Vasilyev <vasilyev@ispras.ru>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/ems_usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index 12ff0020ecd6..b7dfd4109d24 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -1072,6 +1072,7 @@ static void ems_usb_disconnect(struct usb_interface *intf)
usb_free_urb(dev->intr_urb);
kfree(dev->intr_in_buffer);
+ kfree(dev->tx_msg_buffer);
}
}
--
2.18.0
^ permalink raw reply related
* pull-request: can 2018-07-30
From: Marc Kleine-Budde @ 2018-07-30 9:08 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
this is a pull request of one patch for net/master.
The patch by Anton Vasilyev and the Linux Driver Verification project
fixes a memory leak in the ems_usb driver's disconnect function.
regards,
Marc
^ permalink raw reply
* [PATCH v3] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"
From: Thomas Richter @ 2018-07-30 8:53 UTC (permalink / raw)
To: daniel, ast, netdev, linux-kernel, jakub.kicinski
Cc: heiko.carstens, brueckner, schwidefsky, Thomas Richter
commit 531b014e7a2f ("tools: bpf: make use of reallocarray")
causes a compiler error when building the perf tool in the linux-next
tree. Compile file tools/lib/bpf/libbpf.c on a FEDORA 28
installation with gcc compiler
version: gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20)
shows this error message:
[root@p23lp27] # make V=1 EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"
[...]
make -f /home6/tmricht/linux-next/tools/build/Makefile.build
dir=./util/scripting-engines obj=libperf
libbpf.c: In function ‘bpf_object__elf_collect’:
libbpf.c:811:15: error: ignoring return value of ‘strerror_r’,
declared with attribute warn_unused_result [-Werror=unused-result]
strerror_r(-err, errmsg, sizeof(errmsg));
^
cc1: all warnings being treated as errors
mv: cannot stat './.libbpf.o.tmp': No such file or directory
/home6/tmricht/linux-next/tools/build/Makefile.build:96: recipe for target 'libbpf.o' failed
Replace all occurrences of strerror() by calls to strerror_r().
To keep the compiler quiet also use the return value from strerror_r()
otherwise a 'variable set but not use' warning which is treated as error
terminates the compile.
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
---
tools/lib/bpf/libbpf.c | 42 +++++++++++++++++++++++++++++-------------
1 file changed, 29 insertions(+), 13 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 73465caa33ba..e040c6b5fde9 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -468,8 +468,10 @@ static int bpf_object__elf_init(struct bpf_object *obj)
} else {
obj->efile.fd = open(obj->path, O_RDONLY);
if (obj->efile.fd < 0) {
- pr_warning("failed to open %s: %s\n", obj->path,
- strerror(errno));
+ char errmsg[STRERR_BUFSIZE];
+ char *cp = strerror_r(errno, errmsg, sizeof(errmsg));
+
+ pr_warning("failed to open %s: %s\n", obj->path, cp);
return -errno;
}
@@ -808,10 +810,11 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
data->d_size, name, idx);
if (err) {
char errmsg[STRERR_BUFSIZE];
+ char *cp = strerror_r(-err, errmsg,
+ sizeof(errmsg));
- strerror_r(-err, errmsg, sizeof(errmsg));
pr_warning("failed to alloc program %s (%s): %s",
- name, obj->path, errmsg);
+ name, obj->path, cp);
}
} else if (sh.sh_type == SHT_REL) {
void *reloc = obj->efile.reloc;
@@ -1097,6 +1100,7 @@ bpf_object__create_maps(struct bpf_object *obj)
for (i = 0; i < obj->nr_maps; i++) {
struct bpf_map *map = &obj->maps[i];
struct bpf_map_def *def = &map->def;
+ char *cp, errmsg[STRERR_BUFSIZE];
int *pfd = &map->fd;
if (map->fd >= 0) {
@@ -1124,8 +1128,9 @@ bpf_object__create_maps(struct bpf_object *obj)
*pfd = bpf_create_map_xattr(&create_attr);
if (*pfd < 0 && create_attr.btf_key_type_id) {
+ cp = strerror_r(errno, errmsg, sizeof(errmsg));
pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
- map->name, strerror(errno), errno);
+ map->name, cp, errno);
create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
@@ -1138,9 +1143,9 @@ bpf_object__create_maps(struct bpf_object *obj)
size_t j;
err = *pfd;
+ cp = strerror_r(errno, errmsg, sizeof(errmsg));
pr_warning("failed to create map (name: '%s'): %s\n",
- map->name,
- strerror(errno));
+ map->name, cp);
for (j = 0; j < i; j++)
zclose(obj->maps[j].fd);
return err;
@@ -1292,6 +1297,7 @@ load_program(enum bpf_prog_type type, enum bpf_attach_type expected_attach_type,
char *license, u32 kern_version, int *pfd, int prog_ifindex)
{
struct bpf_load_program_attr load_attr;
+ char *cp, errmsg[STRERR_BUFSIZE];
char *log_buf;
int ret;
@@ -1321,7 +1327,8 @@ load_program(enum bpf_prog_type type, enum bpf_attach_type expected_attach_type,
}
ret = -LIBBPF_ERRNO__LOAD;
- pr_warning("load bpf program failed: %s\n", strerror(errno));
+ cp = strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("load bpf program failed: %s\n", cp);
if (log_buf && log_buf[0] != '\0') {
ret = -LIBBPF_ERRNO__VERIFY;
@@ -1620,6 +1627,7 @@ int bpf_object__load(struct bpf_object *obj)
static int check_path(const char *path)
{
+ char *cp, errmsg[STRERR_BUFSIZE];
struct statfs st_fs;
char *dname, *dir;
int err = 0;
@@ -1633,7 +1641,8 @@ static int check_path(const char *path)
dir = dirname(dname);
if (statfs(dir, &st_fs)) {
- pr_warning("failed to statfs %s: %s\n", dir, strerror(errno));
+ cp = strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to statfs %s: %s\n", dir, cp);
err = -errno;
}
free(dname);
@@ -1649,6 +1658,7 @@ static int check_path(const char *path)
int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
int instance)
{
+ char *cp, errmsg[STRERR_BUFSIZE];
int err;
err = check_path(path);
@@ -1667,7 +1677,8 @@ int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
}
if (bpf_obj_pin(prog->instances.fds[instance], path)) {
- pr_warning("failed to pin program: %s\n", strerror(errno));
+ cp = strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to pin program: %s\n", cp);
return -errno;
}
pr_debug("pinned program '%s'\n", path);
@@ -1677,13 +1688,16 @@ int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
static int make_dir(const char *path)
{
+ char *cp, errmsg[STRERR_BUFSIZE];
int err = 0;
if (mkdir(path, 0700) && errno != EEXIST)
err = -errno;
- if (err)
- pr_warning("failed to mkdir %s: %s\n", path, strerror(-err));
+ if (err) {
+ cp = strerror_r(-err, errmsg, sizeof(errmsg));
+ pr_warning("failed to mkdir %s: %s\n", path, cp);
+ }
return err;
}
@@ -1730,6 +1744,7 @@ int bpf_program__pin(struct bpf_program *prog, const char *path)
int bpf_map__pin(struct bpf_map *map, const char *path)
{
+ char *cp, errmsg[STRERR_BUFSIZE];
int err;
err = check_path(path);
@@ -1742,7 +1757,8 @@ int bpf_map__pin(struct bpf_map *map, const char *path)
}
if (bpf_obj_pin(map->fd, path)) {
- pr_warning("failed to pin map: %s\n", strerror(errno));
+ cp = strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to pin map: %s\n", cp);
return -errno;
}
--
2.16.4
^ permalink raw reply related
* Re: [PATCH v3] Add BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES bpf(2) command
From: Daniel Colascione @ 2018-07-30 10:25 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Joel Fernandes, linux-kernel, Tim Murray, netdev,
Alexei Starovoitov, Lorenzo Colitti, Chenbo Feng,
Mathieu Desnoyers, Alexei Starovoitov
In-Reply-To: <e8f6e709-e694-ddad-b61e-d2ce1598dff3@iogearbox.net>
On Mon, Jul 30, 2018 at 3:04 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Hmm, I don't think such UAPI as above is future-proof. In case we would want
> a similar mechanism in future for other maps, we would need a whole new bpf
> command or reuse BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES as a workaround though
> the underlying map may not even be a map-to-map. Additionally, we don't have
> any map object at hand in the above, so we couldn't make any finer grained
> decisions either. Something like below would be more suitable and leaves room
> for extending this further in future.
YAGNI. Your proposed mechanism doesn't add anything under the current
implementation. It's also not clear how a map-specific synchronization
command is supposed to work in cases where we swap multiple map
references. Do we synchronize_rcu multiple times? Why would we impose
that inefficiency just for the sake of some non-specific future
extensibility? Add some kind of batching layer? The current approach
works for the anticipated use cases.
While my preference is not to talk about map-to-maps at all in the
user API and instead spec the thing as talking about map references in
general, I'd rather have something that talks about
references-to-maps-acquired-from-maps than this interface.
^ permalink raw reply
* Re: [PATCH] 9p: fix Use-After-Free in p9_write_work()
From: Dominique Martinet @ 2018-07-30 10:23 UTC (permalink / raw)
To: Tomas Bortoli; +Cc: davem, v9fs-developer, netdev, linux-kernel, syzkaller
In-Reply-To: <4ac26f97-778b-6527-9a5b-08b7bfc8a5e8@gmail.com>
Tomas Bortoli wrote on Mon, Jul 30, 2018:
> > Other transports also have the same issue see discussion in
> > https://lkml.org/lkml/2018/7/19/727
> > (that is another syzbot report, slightly different but I believe it
> > points to the same issue)
> >
> > Basically, a more global view of the problem is a race between
> > p9_tag_lookup returning a p9_req_t and another thread freeing it.
> >
> > Matthew wrote the problem himself in a comment in p9_tag_lookup in his new
> > version that used to be in linux-next at the time (I took the commit out
> > temporarily until I've had time to benchmark it, but it will come back in,
> > just you're working on thin air right now because the bug was only found
> > thanks to this commit):
> > + /* There's no refcount on the req; a malicious server could
> > cause
> > + * us to dereference a NULL pointer
> > + */
> >
> > So a more proper solution would be to had a refcount to req, have
> > p9_tag_lookup increment the refcount within rcu_read_lock, and have a
> > deref function free the req when the count hits 0.
>
> Which commit ? that's a comment.
Sorry, the commit is this one:
http://lkml.kernel.org/r/20180711210225.19730-6-willy@infradead.org
It's now out of my 9p-next branch due to performance reasons but I'll
definitely take it back in once my performance mitigation patches have
had a few reviews.
> That sound like the proper solution. Let's do it that way then.
Cool :)
--
Dominique
^ permalink raw reply
* Re: [PATCH v3] Add BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES bpf(2) command
From: Daniel Borkmann @ 2018-07-30 10:04 UTC (permalink / raw)
To: Daniel Colascione, joelaf
Cc: linux-kernel, timmurray, netdev, Alexei Starovoitov,
Lorenzo Colitti, Chenbo Feng, Mathieu Desnoyers,
Alexei Starovoitov
In-Reply-To: <20180729205835.34850-1-dancol@google.com>
On 07/29/2018 10:58 PM, Daniel Colascione wrote:
> BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES waits for the release of all
> references to maps active at the instant the
> BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES is
> issued. BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES waits only for the
> expiration of map references obtained by BPF programs from other maps.
>
> The purpose of this command is to provide a means for userspace to
> replace a BPF map with another, newer version, then ensure that no
> component is still using the "old" map before manipulating the "old"
> map in some way.
>
> Signed-off-by: Daniel Colascione <dancol@google.com>
> ---
> include/uapi/linux/bpf.h | 14 ++++++++++++++
> kernel/bpf/syscall.c | 13 +++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index b7db3261c62d..ca3cfca76edc 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -75,6 +75,19 @@ struct bpf_lpm_trie_key {
> __u8 data[0]; /* Arbitrary size */
> };
>
> +/* BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES waits for the release of all
> + * references to maps active at the instant the
> + * BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES is
> + * issued. BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES waits only for the
> + * expiration of map references obtained by BPF programs from
> + * other maps.
> + *
> + * The purpose of this command is to provide a means for userspace to
> + * replace a BPF map with another, newer version, then ensure that no
> + * component is still using the "old" map before manipulating the
> + * "old" map in some way.
> + */
> +
> /* BPF syscall commands, see bpf(2) man-page for details. */
> enum bpf_cmd {
> BPF_MAP_CREATE,
> @@ -98,6 +111,7 @@ enum bpf_cmd {
> BPF_BTF_LOAD,
> BPF_BTF_GET_FD_BY_ID,
> BPF_TASK_FD_QUERY,
> + BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES,
> };
>
> enum bpf_map_type {
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index a31a1ba0f8ea..bc9a0713f47d 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2274,6 +2274,19 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
> if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
> return -EPERM;
>
> + if (cmd == BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES) {
> + if (uattr != NULL || size != 0)
> + return -EINVAL;
> + err = security_bpf(cmd, NULL, 0);
> + if (err < 0)
> + return err;
> + /* BPF programs always enter a critical section while
> + * they have a map reference outstanding.
> + */
> + synchronize_rcu();
> + return 0;
> + }
> +
> err = bpf_check_uarg_tail_zero(uattr, sizeof(attr), size);
> if (err)
> return err;
>
Hmm, I don't think such UAPI as above is future-proof. In case we would want
a similar mechanism in future for other maps, we would need a whole new bpf
command or reuse BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES as a workaround though
the underlying map may not even be a map-to-map. Additionally, we don't have
any map object at hand in the above, so we couldn't make any finer grained
decisions either. Something like below would be more suitable and leaves room
for extending this further in future.
Thanks,
Daniel
>From 8dfea71b73fa0d402633b76f78c106e82a7a5007 Mon Sep 17 00:00:00 2001
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Mon, 30 Jul 2018 11:47:37 +0200
Subject: [PATCH] sync map refs
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/bpf.h | 1 +
include/uapi/linux/bpf.h | 1 +
kernel/bpf/arraymap.c | 1 +
kernel/bpf/hashtab.c | 1 +
kernel/bpf/map_in_map.c | 6 ++++++
kernel/bpf/map_in_map.h | 1 +
kernel/bpf/syscall.c | 24 ++++++++++++++++++++++++
7 files changed, 35 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5b5ad95..7b51f86 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -34,6 +34,7 @@ struct bpf_map_ops {
void (*map_free)(struct bpf_map *map);
int (*map_get_next_key)(struct bpf_map *map, void *key, void *next_key);
void (*map_release_uref)(struct bpf_map *map);
+ int (*map_sync_refs)(struct bpf_map *map);
/* funcs callable from userspace and from eBPF programs */
void *(*map_lookup_elem)(struct bpf_map *map, void *key);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 8701139..e6ec1de 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -98,6 +98,7 @@ enum bpf_cmd {
BPF_BTF_LOAD,
BPF_BTF_GET_FD_BY_ID,
BPF_TASK_FD_QUERY,
+ BPF_MAP_SYNC_REFS,
};
enum bpf_map_type {
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 544e58f..ddaf42a 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -748,5 +748,6 @@ const struct bpf_map_ops array_of_maps_map_ops = {
.map_fd_get_ptr = bpf_map_fd_get_ptr,
.map_fd_put_ptr = bpf_map_fd_put_ptr,
.map_fd_sys_lookup_elem = bpf_map_fd_sys_lookup_elem,
+ .map_sync_refs = bpf_map_sync_refs,
.map_gen_lookup = array_of_map_gen_lookup,
};
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 513d9df..05380ea 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -1407,5 +1407,6 @@ const struct bpf_map_ops htab_of_maps_map_ops = {
.map_fd_get_ptr = bpf_map_fd_get_ptr,
.map_fd_put_ptr = bpf_map_fd_put_ptr,
.map_fd_sys_lookup_elem = bpf_map_fd_sys_lookup_elem,
+ .map_sync_refs = bpf_map_sync_refs,
.map_gen_lookup = htab_of_map_gen_lookup,
};
diff --git a/kernel/bpf/map_in_map.c b/kernel/bpf/map_in_map.c
index 1da5746..698a50f 100644
--- a/kernel/bpf/map_in_map.c
+++ b/kernel/bpf/map_in_map.c
@@ -96,6 +96,12 @@ void bpf_map_fd_put_ptr(void *ptr)
bpf_map_put(ptr);
}
+int bpf_map_sync_refs(struct bpf_map *map)
+{
+ synchronize_rcu();
+ return 0;
+}
+
u32 bpf_map_fd_sys_lookup_elem(void *ptr)
{
return ((struct bpf_map *)ptr)->id;
diff --git a/kernel/bpf/map_in_map.h b/kernel/bpf/map_in_map.h
index 6183db9..ac02456 100644
--- a/kernel/bpf/map_in_map.h
+++ b/kernel/bpf/map_in_map.h
@@ -19,6 +19,7 @@ bool bpf_map_meta_equal(const struct bpf_map *meta0,
void *bpf_map_fd_get_ptr(struct bpf_map *map, struct file *map_file,
int ufd);
void bpf_map_fd_put_ptr(void *ptr);
+int bpf_map_sync_refs(struct bpf_map *map);
u32 bpf_map_fd_sys_lookup_elem(void *ptr);
#endif
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index a31a1ba..b1286cc 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -896,6 +896,27 @@ static int map_get_next_key(union bpf_attr *attr)
return err;
}
+#define BPF_MAP_SYNC_REFS_LAST_FIELD map_fd
+
+static int map_sync_refs(union bpf_attr *attr)
+{
+ int err = -ENOTSUPP, ufd = attr->map_fd;
+ struct bpf_map *map;
+ struct fd f;
+
+ if (CHECK_ATTR(BPF_MAP_SYNC_REFS))
+ return -EINVAL;
+
+ f = fdget(ufd);
+ map = __bpf_map_get(f);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+ if (map->ops->map_sync_refs)
+ err = map->ops->map_sync_refs(map);
+ fdput(f);
+ return err;
+}
+
static const struct bpf_prog_ops * const bpf_prog_types[] = {
#define BPF_PROG_TYPE(_id, _name) \
[_id] = & _name ## _prog_ops,
@@ -2303,6 +2324,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
case BPF_MAP_GET_NEXT_KEY:
err = map_get_next_key(&attr);
break;
+ case BPF_MAP_SYNC_REFS:
+ err = map_sync_refs(&attr);
+ break;
case BPF_PROG_LOAD:
err = bpf_prog_load(&attr);
break;
--
2.9.5
^ permalink raw reply related
* [PATCH 3/3] ptp_qoriq: convert to use module parameters for initialization
From: Yangbo Lu @ 2018-07-30 10:01 UTC (permalink / raw)
To: netdev, madalin.bucur, Richard Cochran, Rob Herring, Shawn Guo,
David S . Miller
Cc: devicetree, linuxppc-dev, linux-arm-kernel, linux-kernel,
Yangbo Lu
In-Reply-To: <20180730100154.27906-1-yangbo.lu@nxp.com>
The ptp_qoriq driver initialized the 1588 timer with the
configurations provided by the properties of device tree
node. For example,
fsl,tclk-period = <5>;
fsl,tmr-prsc = <2>;
fsl,tmr-add = <0xaaaaaaab>;
fsl,tmr-fiper1 = <999999995>;
fsl,tmr-fiper2 = <99990>;
fsl,max-adj = <499999999>;
These things actually were runtime configurations which
were not proper to be put into dts. This patch is to convert
to use module parameters for 1588 timer initialization, and
to support initial register values calculation.
If the parameters are not provided, the driver will calculate
register values with a set of default parameters. With this
patch, those dts properties are no longer needed for new
platform to support 1588 timer, and many QorIQ DPAA platforms
(some P series and T series platforms of PowerPC, and some
LS series platforms of ARM64) could use this driver for their
fman ptp timer with default module parameters. However, this
patch didn't remove the dts method. Because there were still
many old platforms using the dts method. We need to clean up
their dts files, verify module parameters on them, and convert
them to the new method gradually in case of breaking any
function.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
drivers/ptp/ptp_qoriq.c | 117 +++++++++++++++++++++++++++++++++++++++-
include/linux/fsl/ptp_qoriq.h | 1 +
2 files changed, 115 insertions(+), 3 deletions(-)
diff --git a/drivers/ptp/ptp_qoriq.c b/drivers/ptp/ptp_qoriq.c
index a14c317..22baf83 100644
--- a/drivers/ptp/ptp_qoriq.c
+++ b/drivers/ptp/ptp_qoriq.c
@@ -29,9 +29,30 @@
#include <linux/of_platform.h>
#include <linux/timex.h>
#include <linux/slab.h>
+#include <linux/clk.h>
#include <linux/fsl/ptp_qoriq.h>
+static unsigned int cksel = DEFAULT_CKSEL;
+module_param(cksel, uint, 0644);
+MODULE_PARM_DESC(cksel, "Select reference clock");
+
+static unsigned int clk_src;
+module_param(clk_src, uint, 0644);
+MODULE_PARM_DESC(clk_src, "Reference clock frequency (if clocks property not provided in dts)");
+
+static unsigned int tmr_prsc = 2;
+module_param(tmr_prsc, uint, 0644);
+MODULE_PARM_DESC(tmr_prsc, "Output clock division/prescale factor");
+
+static unsigned int tmr_fiper1 = 1000000000;
+module_param(tmr_fiper1, uint, 0644);
+MODULE_PARM_DESC(tmr_fiper1, "Desired fixed interval pulse period (ns)");
+
+static unsigned int tmr_fiper2 = 100000;
+module_param(tmr_fiper2, uint, 0644);
+MODULE_PARM_DESC(tmr_fiper2, "Desired fixed interval pulse period (ns)");
+
/*
* Register access functions
*/
@@ -317,6 +338,91 @@ static int ptp_qoriq_enable(struct ptp_clock_info *ptp,
.enable = ptp_qoriq_enable,
};
+/**
+ * qoriq_ptp_nominal_freq - calculate nominal frequency by reference clock
+ * frequency
+ *
+ * @clk_src: reference clock frequency
+ *
+ * The nominal frequency is the desired clock frequency.
+ * It should be less than the reference clock frequency.
+ * It should be a factor of 1000MHz.
+ *
+ * Return the nominal frequency
+ */
+static u32 qoriq_ptp_nominal_freq(u32 clk_src)
+{
+ u32 remainder = 0;
+
+ clk_src /= 1000000;
+ remainder = clk_src % 100;
+ if (remainder) {
+ clk_src -= remainder;
+ clk_src += 100;
+ }
+
+ do {
+ clk_src -= 100;
+
+ } while (1000 % clk_src);
+
+ return clk_src * 1000000;
+}
+
+static int qoriq_ptp_config(struct qoriq_ptp *qoriq_ptp,
+ struct device_node *node)
+{
+ struct clk *clk;
+ u64 freq_comp;
+ u64 max_adj;
+ u32 nominal_freq;
+
+ qoriq_ptp->cksel = cksel;
+
+ if (clk_src) {
+ qoriq_ptp->clk_src = clk_src;
+ } else {
+ clk = of_clk_get(node, 0);
+ if (!IS_ERR(clk)) {
+ qoriq_ptp->clk_src = clk_get_rate(clk);
+ clk_put(clk);
+ }
+ }
+
+ if (qoriq_ptp->clk_src <= 100000000UL) {
+ pr_err("error reference clock value, or lower than 100MHz\n");
+ return -EINVAL;
+ }
+
+ nominal_freq = qoriq_ptp_nominal_freq(qoriq_ptp->clk_src);
+ if (!nominal_freq)
+ return -EINVAL;
+
+ qoriq_ptp->tclk_period = 1000000000UL / nominal_freq;
+ qoriq_ptp->tmr_prsc = tmr_prsc;
+
+ /* Calculate initial frequency compensation value for TMR_ADD register.
+ * freq_comp = ceil(2^32 / freq_ratio)
+ * freq_ratio = reference_clock_freq / nominal_freq
+ */
+ freq_comp = ((u64)1 << 32) * nominal_freq;
+ if (do_div(freq_comp, qoriq_ptp->clk_src))
+ freq_comp++;
+
+ qoriq_ptp->tmr_add = freq_comp;
+ qoriq_ptp->tmr_fiper1 = tmr_fiper1 - qoriq_ptp->tclk_period;
+ qoriq_ptp->tmr_fiper2 = tmr_fiper2 - qoriq_ptp->tclk_period;
+
+ /* max_adj = 1000000000 * (freq_ratio - 1.0) - 1
+ * freq_ratio = reference_clock_freq / nominal_freq
+ */
+ max_adj = 1000000000ULL * (qoriq_ptp->clk_src - nominal_freq);
+ max_adj = max_adj / nominal_freq - 1;
+ qoriq_ptp->caps.max_adj = max_adj;
+
+ return 0;
+}
+
static int qoriq_ptp_probe(struct platform_device *dev)
{
struct device_node *node = dev->dev.of_node;
@@ -332,7 +438,7 @@ static int qoriq_ptp_probe(struct platform_device *dev)
if (!qoriq_ptp)
goto no_memory;
- err = -ENODEV;
+ err = -EINVAL;
qoriq_ptp->caps = ptp_qoriq_caps;
@@ -351,10 +457,14 @@ static int qoriq_ptp_probe(struct platform_device *dev)
"fsl,tmr-fiper2", &qoriq_ptp->tmr_fiper2) ||
of_property_read_u32(node,
"fsl,max-adj", &qoriq_ptp->caps.max_adj)) {
- pr_err("device tree node missing required elements\n");
- goto no_node;
+ pr_warn("device tree node missing required elements, try module param\n");
+
+ if (qoriq_ptp_config(qoriq_ptp, node))
+ goto no_param;
}
+ err = -ENODEV;
+
qoriq_ptp->irq = platform_get_irq(dev, 0);
if (qoriq_ptp->irq < 0) {
@@ -436,6 +546,7 @@ static int qoriq_ptp_probe(struct platform_device *dev)
release_resource(qoriq_ptp->rsrc);
no_resource:
free_irq(qoriq_ptp->irq, qoriq_ptp);
+no_param:
no_node:
kfree(qoriq_ptp);
no_memory:
diff --git a/include/linux/fsl/ptp_qoriq.h b/include/linux/fsl/ptp_qoriq.h
index dc3dac4..586d430 100644
--- a/include/linux/fsl/ptp_qoriq.h
+++ b/include/linux/fsl/ptp_qoriq.h
@@ -147,6 +147,7 @@ struct qoriq_ptp {
u32 cksel;
u32 tmr_fiper1;
u32 tmr_fiper2;
+ u32 clk_src;
};
static inline u32 qoriq_read(unsigned __iomem *addr)
--
1.7.1
^ permalink raw reply related
* [PATCH 2/3] powerpc/mpc85xx: add clocks property for fman ptp timer node
From: Yangbo Lu @ 2018-07-30 10:01 UTC (permalink / raw)
To: netdev, madalin.bucur, Richard Cochran, Rob Herring, Shawn Guo,
David S . Miller
Cc: devicetree, linuxppc-dev, linux-arm-kernel, linux-kernel,
Yangbo Lu
In-Reply-To: <20180730100154.27906-1-yangbo.lu@nxp.com>
This patch is to add clocks property for fman ptp timer node.
Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi | 1 +
arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi | 1 +
arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi | 1 +
arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi | 1 +
arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi | 1 +
5 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi
index 6b124f7..9b6cf91 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi
@@ -100,4 +100,5 @@ ptp_timer0: ptp-timer@4fe000 {
compatible = "fsl,fman-ptp-timer";
reg = <0x4fe000 0x1000>;
interrupts = <96 2 0 0>;
+ clocks = <&clockgen 3 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi
index b80aaf5..e95c11f 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi
@@ -100,4 +100,5 @@ ptp_timer1: ptp-timer@5fe000 {
compatible = "fsl,fman-ptp-timer";
reg = <0x5fe000 0x1000>;
interrupts = <97 2 0 0>;
+ clocks = <&clockgen 3 1>;
};
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi
index d3720fd..d62b36c 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi
@@ -105,4 +105,5 @@ ptp_timer0: ptp-timer@4fe000 {
compatible = "fsl,fman-ptp-timer";
reg = <0x4fe000 0x1000>;
interrupts = <96 2 0 0>;
+ clocks = <&clockgen 3 0>;
};
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi
index ae34c20..3102324 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi
@@ -105,4 +105,5 @@ ptp_timer1: ptp-timer@5fe000 {
compatible = "fsl,fman-ptp-timer";
reg = <0x5fe000 0x1000>;
interrupts = <97 2 0 0>;
+ clocks = <&clockgen 3 1>;
};
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi
index 02f2755..c90702b 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi
@@ -93,4 +93,5 @@ ptp_timer0: ptp-timer@4fe000 {
compatible = "fsl,fman-ptp-timer";
reg = <0x4fe000 0x1000>;
interrupts = <96 2 0 0>;
+ clocks = <&clockgen 3 0>;
};
--
1.7.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox