* [PATCH] brcmfmac: Use standard SKB list accessors in brcmf_sdiod_sglist_rw.
From: David Miller @ 2018-11-11 0:34 UTC (permalink / raw)
To: netdev; +Cc: linux-wireless
[ As I am trying to remove direct SKB list pointer accesses I am
committing this to net-next. If this causes a lot of grief I
can and will revert, just let me know. ]
Instead of direct SKB list pointer accesses.
The loops in this function had to be rewritten to accommodate this
more easily.
The first loop iterates now over the target list in the outer loop,
and triggers an mmc data operation when the per-operation limits are
hit.
Then after the loops, if we have any residue, we trigger the last
and final operation.
For the page aligned workaround, where we have to copy the read data
back into the original list of SKBs, we use a two-tiered loop. The
outer loop stays the same and iterates over pktlist, and then we have
an inner loop which uses skb_peek_next(). The break logic has been
simplified because we know that the aggregate length of the SKBs in
the source and destination lists are the same.
This change also ends up fixing a bug, having to do with the
maintainance of the seg_sz variable and how it drove the outermost
loop. It begins as:
seg_sz = target_list->qlen;
ie. the number of packets in the target_list queue. The loop
structure was then:
while (seq_sz) {
...
while (not at end of target_list) {
...
sg_cnt++
...
}
...
seg_sz -= sg_cnt;
The assumption built into that last statement is that sg_cnt counts
how many packets from target_list have been fully processed by the
inner loop. But this not true.
If we hit one of the limits, such as the max segment size or the max
request size, we will break and copy a partial packet then contine
back up to the top of the outermost loop.
With the new loops we don't have this problem as we don't guard the
loop exit with a packet count, but instead use the progression of the
pkt_next SKB through the list to the end. The general structure is:
sg_cnt = 0;
skb_queue_walk(target_list, pkt_next) {
pkt_offset = 0;
...
sg_cnt++;
...
while (pkt_offset < pkt_next->len) {
pkt_offset += sg_data_size;
if (queued up max per request)
mmc_submit_one();
}
}
if (sg_cnt)
mmc_submit_one();
The variables that maintain where we are in the MMC command state such
as req_sz, sg_cnt, and sgl are reset when we emit one of these full
sized requests.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
.../broadcom/brcm80211/brcmfmac/bcmsdh.c | 137 ++++++++++--------
1 file changed, 74 insertions(+), 63 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 3e37c8cf82c6..b2ad2122c8c4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -342,6 +342,37 @@ static int brcmf_sdiod_skbuff_write(struct brcmf_sdio_dev *sdiodev,
return err;
}
+static int mmc_submit_one(struct mmc_data *md, struct mmc_request *mr,
+ struct mmc_command *mc, int sg_cnt, int req_sz,
+ int func_blk_sz, u32 *addr,
+ struct brcmf_sdio_dev *sdiodev,
+ struct sdio_func *func, int write)
+{
+ int ret;
+
+ md->sg_len = sg_cnt;
+ md->blocks = req_sz / func_blk_sz;
+ mc->arg |= (*addr & 0x1FFFF) << 9; /* address */
+ mc->arg |= md->blocks & 0x1FF; /* block count */
+ /* incrementing addr for function 1 */
+ if (func->num == 1)
+ *addr += req_sz;
+
+ mmc_set_data_timeout(md, func->card);
+ mmc_wait_for_req(func->card->host, mr);
+
+ ret = mc->error ? mc->error : md->error;
+ if (ret == -ENOMEDIUM) {
+ brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
+ } else if (ret != 0) {
+ brcmf_err("CMD53 sg block %s failed %d\n",
+ write ? "write" : "read", ret);
+ ret = -EIO;
+ }
+
+ return ret;
+}
+
/**
* brcmf_sdiod_sglist_rw - SDIO interface function for block data access
* @sdiodev: brcmfmac sdio device
@@ -360,11 +391,11 @@ static int brcmf_sdiod_sglist_rw(struct brcmf_sdio_dev *sdiodev,
struct sk_buff_head *pktlist)
{
unsigned int req_sz, func_blk_sz, sg_cnt, sg_data_sz, pkt_offset;
- unsigned int max_req_sz, orig_offset, dst_offset;
- unsigned short max_seg_cnt, seg_sz;
+ unsigned int max_req_sz, src_offset, dst_offset;
unsigned char *pkt_data, *orig_data, *dst_data;
- struct sk_buff *pkt_next = NULL, *local_pkt_next;
struct sk_buff_head local_list, *target_list;
+ struct sk_buff *pkt_next = NULL, *src;
+ unsigned short max_seg_cnt;
struct mmc_request mmc_req;
struct mmc_command mmc_cmd;
struct mmc_data mmc_dat;
@@ -404,9 +435,6 @@ static int brcmf_sdiod_sglist_rw(struct brcmf_sdio_dev *sdiodev,
max_req_sz = sdiodev->max_request_size;
max_seg_cnt = min_t(unsigned short, sdiodev->max_segment_count,
target_list->qlen);
- seg_sz = target_list->qlen;
- pkt_offset = 0;
- pkt_next = target_list->next;
memset(&mmc_req, 0, sizeof(struct mmc_request));
memset(&mmc_cmd, 0, sizeof(struct mmc_command));
@@ -425,12 +453,12 @@ static int brcmf_sdiod_sglist_rw(struct brcmf_sdio_dev *sdiodev,
mmc_req.cmd = &mmc_cmd;
mmc_req.data = &mmc_dat;
- while (seg_sz) {
- req_sz = 0;
- sg_cnt = 0;
- sgl = sdiodev->sgtable.sgl;
- /* prep sg table */
- while (pkt_next != (struct sk_buff *)target_list) {
+ req_sz = 0;
+ sg_cnt = 0;
+ sgl = sdiodev->sgtable.sgl;
+ skb_queue_walk(target_list, pkt_next) {
+ pkt_offset = 0;
+ while (pkt_offset < pkt_next->len) {
pkt_data = pkt_next->data + pkt_offset;
sg_data_sz = pkt_next->len - pkt_offset;
if (sg_data_sz > sdiodev->max_segment_size)
@@ -439,72 +467,55 @@ static int brcmf_sdiod_sglist_rw(struct brcmf_sdio_dev *sdiodev,
sg_data_sz = max_req_sz - req_sz;
sg_set_buf(sgl, pkt_data, sg_data_sz);
-
sg_cnt++;
+
sgl = sg_next(sgl);
req_sz += sg_data_sz;
pkt_offset += sg_data_sz;
- if (pkt_offset == pkt_next->len) {
- pkt_offset = 0;
- pkt_next = pkt_next->next;
+ if (req_sz >= max_req_sz || sg_cnt >= max_seg_cnt) {
+ ret = mmc_submit_one(&mmc_dat, &mmc_req, &mmc_cmd,
+ sg_cnt, req_sz, func_blk_sz,
+ &addr, sdiodev, func, write);
+ if (ret)
+ goto exit_queue_walk;
+ req_sz = 0;
+ sg_cnt = 0;
+ sgl = sdiodev->sgtable.sgl;
}
-
- if (req_sz >= max_req_sz || sg_cnt >= max_seg_cnt)
- break;
- }
- seg_sz -= sg_cnt;
-
- if (req_sz % func_blk_sz != 0) {
- brcmf_err("sg request length %u is not %u aligned\n",
- req_sz, func_blk_sz);
- ret = -ENOTBLK;
- goto exit;
- }
-
- mmc_dat.sg_len = sg_cnt;
- mmc_dat.blocks = req_sz / func_blk_sz;
- mmc_cmd.arg |= (addr & 0x1FFFF) << 9; /* address */
- mmc_cmd.arg |= mmc_dat.blocks & 0x1FF; /* block count */
- /* incrementing addr for function 1 */
- if (func->num == 1)
- addr += req_sz;
-
- mmc_set_data_timeout(&mmc_dat, func->card);
- mmc_wait_for_req(func->card->host, &mmc_req);
-
- ret = mmc_cmd.error ? mmc_cmd.error : mmc_dat.error;
- if (ret == -ENOMEDIUM) {
- brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_NOMEDIUM);
- break;
- } else if (ret != 0) {
- brcmf_err("CMD53 sg block %s failed %d\n",
- write ? "write" : "read", ret);
- ret = -EIO;
- break;
}
}
-
+ if (sg_cnt)
+ ret = mmc_submit_one(&mmc_dat, &mmc_req, &mmc_cmd,
+ sg_cnt, req_sz, func_blk_sz,
+ &addr, sdiodev, func, write);
+exit_queue_walk:
if (!write && sdiodev->settings->bus.sdio.broken_sg_support) {
- local_pkt_next = local_list.next;
- orig_offset = 0;
+ src = __skb_peek(&local_list);
+ src_offset = 0;
skb_queue_walk(pktlist, pkt_next) {
dst_offset = 0;
- do {
- req_sz = local_pkt_next->len - orig_offset;
- req_sz = min_t(uint, pkt_next->len - dst_offset,
- req_sz);
- orig_data = local_pkt_next->data + orig_offset;
+
+ /* This is safe because we must have enough SKB data
+ * in the local list to cover everything in pktlist.
+ */
+ while (1) {
+ req_sz = pkt_next->len - dst_offset;
+ if (req_sz > src->len - src_offset)
+ req_sz = src->len - src_offset;
+
+ orig_data = src->data + src_offset;
dst_data = pkt_next->data + dst_offset;
memcpy(dst_data, orig_data, req_sz);
- orig_offset += req_sz;
- dst_offset += req_sz;
- if (orig_offset == local_pkt_next->len) {
- orig_offset = 0;
- local_pkt_next = local_pkt_next->next;
+
+ src_offset += req_sz;
+ if (src_offset == src->len) {
+ src_offset = 0;
+ src = skb_peek_next(src, &local_list);
}
+ dst_offset += req_sz;
if (dst_offset == pkt_next->len)
break;
- } while (!skb_queue_empty(&local_list));
+ }
}
}
--
2.19.1
^ permalink raw reply related
* [PATCH] iucv: Remove SKB list assumptions.
From: David Miller @ 2018-11-11 0:55 UTC (permalink / raw)
To: netdev
Eliminate the assumption that SKBs and SKB list heads can
be cast to eachother in SKB list handling code.
This change also appears to fix a bug since the list->next pointer is
sampled outside of holding the SKB queue lock.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/iucv/af_iucv.c | 41 +++++++++++++++--------------------------
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index 0bed4cc20603..78ea5a739d10 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -1873,30 +1873,26 @@ static void iucv_callback_txdone(struct iucv_path *path,
struct sock *sk = path->private;
struct sk_buff *this = NULL;
struct sk_buff_head *list = &iucv_sk(sk)->send_skb_q;
- struct sk_buff *list_skb = list->next;
+ struct sk_buff *list_skb;
unsigned long flags;
bh_lock_sock(sk);
- if (!skb_queue_empty(list)) {
- spin_lock_irqsave(&list->lock, flags);
- while (list_skb != (struct sk_buff *)list) {
- if (msg->tag == IUCV_SKB_CB(list_skb)->tag) {
- this = list_skb;
- break;
- }
- list_skb = list_skb->next;
+ spin_lock_irqsave(&list->lock, flags);
+ skb_queue_walk(list, list_skb) {
+ if (msg->tag == IUCV_SKB_CB(list_skb)->tag) {
+ this = list_skb;
+ break;
}
- if (this)
- __skb_unlink(this, list);
-
- spin_unlock_irqrestore(&list->lock, flags);
+ }
+ if (this)
+ __skb_unlink(this, list);
+ spin_unlock_irqrestore(&list->lock, flags);
- if (this) {
- kfree_skb(this);
- /* wake up any process waiting for sending */
- iucv_sock_wake_msglim(sk);
- }
+ if (this) {
+ kfree_skb(this);
+ /* wake up any process waiting for sending */
+ iucv_sock_wake_msglim(sk);
}
if (sk->sk_state == IUCV_CLOSING) {
@@ -2284,11 +2280,7 @@ static void afiucv_hs_callback_txnotify(struct sk_buff *skb,
list = &iucv->send_skb_q;
spin_lock_irqsave(&list->lock, flags);
- if (skb_queue_empty(list))
- goto out_unlock;
- list_skb = list->next;
- nskb = list_skb->next;
- while (list_skb != (struct sk_buff *)list) {
+ skb_queue_walk_safe(list, list_skb, nskb) {
if (skb_shinfo(list_skb) == skb_shinfo(skb)) {
switch (n) {
case TX_NOTIFY_OK:
@@ -2321,10 +2313,7 @@ static void afiucv_hs_callback_txnotify(struct sk_buff *skb,
}
break;
}
- list_skb = nskb;
- nskb = nskb->next;
}
-out_unlock:
spin_unlock_irqrestore(&list->lock, flags);
if (sk->sk_state == IUCV_CLOSING) {
--
2.19.1
^ permalink raw reply related
* [PATCH net-next v2] tcp: minor optimization in tcp ack fast path processing
From: Yafang Shao @ 2018-11-11 12:10 UTC (permalink / raw)
To: davem, edumazet; +Cc: netdev, linux-kernel, Yafang Shao, Joe Perches
Bitwise operation is a little faster.
So I replace after() with using the flag FLAG_SND_UNA_ADVANCED as it is
already set before.
In addtion, there's another similar improvement in tcp_cwnd_reduction().
Cc: Joe Perches <joe@perches.com>
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
net/ipv4/tcp_input.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2868ef2..edaaebf 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2457,8 +2457,8 @@ void tcp_cwnd_reduction(struct sock *sk, int newly_acked_sacked, int flag)
u64 dividend = (u64)tp->snd_ssthresh * tp->prr_delivered +
tp->prior_cwnd - 1;
sndcnt = div_u64(dividend, tp->prior_cwnd) - tp->prr_out;
- } else if ((flag & FLAG_RETRANS_DATA_ACKED) &&
- !(flag & FLAG_LOST_RETRANS)) {
+ } else if ((flag & (FLAG_RETRANS_DATA_ACKED | FLAG_LOST_RETRANS)) ==
+ FLAG_RETRANS_DATA_ACKED) {
sndcnt = min_t(int, delta,
max_t(int, tp->prr_delivered - tp->prr_out,
newly_acked_sacked) + 1);
@@ -3610,7 +3610,8 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
if (flag & FLAG_UPDATE_TS_RECENT)
tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
- if (!(flag & FLAG_SLOWPATH) && after(ack, prior_snd_una)) {
+ if ((flag & (FLAG_SLOWPATH | FLAG_SND_UNA_ADVANCED)) ==
+ FLAG_SND_UNA_ADVANCED) {
/* Window is constant, pure forward advance.
* No more checks are required.
* Note, we use the fact that SND.UNA>=SND.WL2.
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next] sctp: Fix SKB list traversal in sctp_intl_store_reasm().
From: David Miller @ 2018-11-11 3:29 UTC (permalink / raw)
To: netdev
To be fully correct, an iterator has an undefined value when something
like skb_queue_walk() naturally terminates.
This will actually matter when SKB queues are converted over to
list_head.
Formalize what this code ends up doing with the current
implementation.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/sctp/stream_interleave.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index 0a78cdf86463..368d9c33fde1 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -140,7 +140,7 @@ static void sctp_intl_store_reasm(struct sctp_ulpq *ulpq,
struct sctp_ulpevent *event)
{
struct sctp_ulpevent *cevent;
- struct sk_buff *pos;
+ struct sk_buff *pos, *loc;
pos = skb_peek_tail(&ulpq->reasm);
if (!pos) {
@@ -166,23 +166,30 @@ static void sctp_intl_store_reasm(struct sctp_ulpq *ulpq,
return;
}
+ loc = NULL;
skb_queue_walk(&ulpq->reasm, pos) {
cevent = sctp_skb2event(pos);
if (event->stream < cevent->stream ||
(event->stream == cevent->stream &&
- MID_lt(event->mid, cevent->mid)))
+ MID_lt(event->mid, cevent->mid))) {
+ loc = pos;
break;
-
+ }
if (event->stream == cevent->stream &&
event->mid == cevent->mid &&
!(cevent->msg_flags & SCTP_DATA_FIRST_FRAG) &&
(event->msg_flags & SCTP_DATA_FIRST_FRAG ||
- event->fsn < cevent->fsn))
+ event->fsn < cevent->fsn)) {
+ loc = pos;
break;
+ }
}
- __skb_queue_before(&ulpq->reasm, pos, sctp_event2skb(event));
+ if (!loc)
+ __skb_queue_tail(&ulpq->reasm, sctp_event2skb(event));
+ else
+ __skb_queue_before(&ulpq->reasm, loc, sctp_event2skb(event));
}
static struct sctp_ulpevent *sctp_intl_retrieve_partial(
--
2.19.1
^ permalink raw reply related
* [PATCH net-next] sctp: Fix SKB list traversal in sctp_intl_store_ordered().
From: David Miller @ 2018-11-11 3:32 UTC (permalink / raw)
To: netdev
Same change as made to sctp_intl_store_reasm().
To be fully correct, an iterator has an undefined value when something
like skb_queue_walk() naturally terminates.
This will actually matter when SKB queues are converted over to
list_head.
Formalize what this code ends up doing with the current
implementation.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/sctp/stream_interleave.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/net/sctp/stream_interleave.c b/net/sctp/stream_interleave.c
index 368d9c33fde1..2b499a85db0e 100644
--- a/net/sctp/stream_interleave.c
+++ b/net/sctp/stream_interleave.c
@@ -390,7 +390,7 @@ static void sctp_intl_store_ordered(struct sctp_ulpq *ulpq,
struct sctp_ulpevent *event)
{
struct sctp_ulpevent *cevent;
- struct sk_buff *pos;
+ struct sk_buff *pos, *loc;
pos = skb_peek_tail(&ulpq->lobby);
if (!pos) {
@@ -410,18 +410,25 @@ static void sctp_intl_store_ordered(struct sctp_ulpq *ulpq,
return;
}
+ loc = NULL;
skb_queue_walk(&ulpq->lobby, pos) {
cevent = (struct sctp_ulpevent *)pos->cb;
- if (cevent->stream > event->stream)
+ if (cevent->stream > event->stream) {
+ loc = pos;
break;
-
+ }
if (cevent->stream == event->stream &&
- MID_lt(event->mid, cevent->mid))
+ MID_lt(event->mid, cevent->mid)) {
+ loc = pos;
break;
+ }
}
- __skb_queue_before(&ulpq->lobby, pos, sctp_event2skb(event));
+ if (!loc)
+ __skb_queue_tail(&ulpq->lobby, sctp_event2skb(event));
+ else
+ __skb_queue_before(&ulpq->lobby, loc, sctp_event2skb(event));
}
static void sctp_intl_retrieve_ordered(struct sctp_ulpq *ulpq,
--
2.19.1
^ permalink raw reply related
* RE,
From: Miss Juliet Muhammad @ 2018-11-11 4:21 UTC (permalink / raw)
To: Recipients
I have a deal for you, in your region.
^ permalink raw reply
* Re: [PATCH net-next v2] tcp: minor optimization in tcp ack fast path processing
From: Eric Dumazet @ 2018-11-11 14:34 UTC (permalink / raw)
To: Yafang Shao, davem, edumazet; +Cc: netdev, linux-kernel, Joe Perches
In-Reply-To: <1541938210-11797-1-git-send-email-laoar.shao@gmail.com>
On 11/11/2018 04:10 AM, Yafang Shao wrote:
> Bitwise operation is a little faster.
> So I replace after() with using the flag FLAG_SND_UNA_ADVANCED as it is
> already set before.
>
> In addtion, there's another similar improvement in tcp_cwnd_reduction().
>
> Cc: Joe Perches <joe@perches.com>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Thanks.
^ permalink raw reply
* RE: [RFC PATCH 00/10] net: hns3: Adds support of debugfs to HNS3 driver
From: Salil Mehta @ 2018-11-11 15:12 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem@davemloft.net, yuvalm@mellanox.com, leon@kernel.org,
Zhuangyuzeng (Yisen), lipeng (Y), mehta.salil@opnsrc.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rdma@vger.kernel.org, Linuxarm
In-Reply-To: <20181109224346.GD5259@lunn.ch>
Hi Andrew,
Thanks for replying. Sorry, for not being prompt as I was
traveling.
Please find some further follow-up questions below
Salil.
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Andrew Lunn
> Sent: Friday, November 9, 2018 10:44 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: davem@davemloft.net; yuvalm@mellanox.com; leon@kernel.org;
> Zhuangyuzeng (Yisen) <yisen.zhuang@huawei.com>; lipeng (Y)
> <lipeng321@huawei.com>; mehta.salil@opnsrc.net; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> <linuxarm@huawei.com>
> Subject: Re: [RFC PATCH 00/10] net: hns3: Adds support of debugfs to
> HNS3 driver
>
> > 3. Debugfs looks more unstructured unlike sysfs. Is there any
> > de-facto standard of the user-api or drivers are allowed to
> > use it in any way to expose the information from kernel.
>
> Hi Salil
>
> You don't really have a user api using debugfs, because debugfs is
> unstable. Anything can change at any time. Any user tools which use
> debugfs can be expected to break at any time as the information in
> debugfs changes. debugfs is for debug, not to export an API. And in
> production systems, it is often not mounted.
Sure, I understand.
>
> As much as possible, you are recommended to use existing APIs,
> ethtool, devlink, etc.
Agreed. But what about if we want to expose anything related to
firmware to user-space using the debugfs, assuming we are presenting
information in structured way and not as a black-box to some user-space
application. Is it something which might be discouraged?
Many Thanks
^ permalink raw reply
* [PATCH bpf-next v2 0/3] bpf: Allow narrow loads with offset > 0
From: Andrey Ignatov @ 2018-11-11 6:15 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, yhs, kernel-team
This patch set adds support for narrow loads with offset > 0 to BPF
verifier.
Patch 1 provides more details and is the main patch in the set.
Patches 2 and 3 add new test cases to test_verifier and test_sock_addr
selftests.
v1->v2:
- fix -Wdeclaration-after-statement warning.
Andrey Ignatov (3):
bpf: Allow narrow loads with offset > 0
selftests/bpf: Test narrow loads with off > 0 in test_verifier
selftests/bpf: Test narrow loads with off > 0 for bpf_sock_addr
include/linux/filter.h | 16 +------
kernel/bpf/verifier.c | 21 +++++++--
tools/testing/selftests/bpf/test_sock_addr.c | 28 ++++++++++--
tools/testing/selftests/bpf/test_verifier.c | 48 ++++++++++++++++----
4 files changed, 79 insertions(+), 34 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH bpf-next v2 2/3] selftests/bpf: Test narrow loads with off > 0 in test_verifier
From: Andrey Ignatov @ 2018-11-11 6:15 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, yhs, kernel-team
In-Reply-To: <cover.1541916635.git.rdna@fb.com>
Test the following narrow loads in test_verifier for context __sk_buff:
* off=1, size=1 - ok;
* off=2, size=1 - ok;
* off=3, size=1 - ok;
* off=0, size=2 - ok;
* off=1, size=2 - fail;
* off=0, size=2 - ok;
* off=3, size=2 - fail.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/testing/selftests/bpf/test_verifier.c | 48 ++++++++++++++++-----
1 file changed, 38 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 6f61df62f690..54d16fbdef8b 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -2026,29 +2026,27 @@ static struct bpf_test tests[] = {
.result = ACCEPT,
},
{
- "check skb->hash byte load not permitted 1",
+ "check skb->hash byte load permitted 1",
.insns = {
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1,
offsetof(struct __sk_buff, hash) + 1),
BPF_EXIT_INSN(),
},
- .errstr = "invalid bpf_context access",
- .result = REJECT,
+ .result = ACCEPT,
},
{
- "check skb->hash byte load not permitted 2",
+ "check skb->hash byte load permitted 2",
.insns = {
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1,
offsetof(struct __sk_buff, hash) + 2),
BPF_EXIT_INSN(),
},
- .errstr = "invalid bpf_context access",
- .result = REJECT,
+ .result = ACCEPT,
},
{
- "check skb->hash byte load not permitted 3",
+ "check skb->hash byte load permitted 3",
.insns = {
BPF_MOV64_IMM(BPF_REG_0, 0),
#if __BYTE_ORDER == __LITTLE_ENDIAN
@@ -2060,8 +2058,7 @@ static struct bpf_test tests[] = {
#endif
BPF_EXIT_INSN(),
},
- .errstr = "invalid bpf_context access",
- .result = REJECT,
+ .result = ACCEPT,
},
{
"check cb access: byte, wrong type",
@@ -2173,7 +2170,7 @@ static struct bpf_test tests[] = {
.result = ACCEPT,
},
{
- "check skb->hash half load not permitted",
+ "check skb->hash half load permitted 2",
.insns = {
BPF_MOV64_IMM(BPF_REG_0, 0),
#if __BYTE_ORDER == __LITTLE_ENDIAN
@@ -2182,6 +2179,37 @@ static struct bpf_test tests[] = {
#else
BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
offsetof(struct __sk_buff, hash)),
+#endif
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ },
+ {
+ "check skb->hash half load not permitted, unaligned 1",
+ .insns = {
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+ BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, hash) + 1),
+#else
+ BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, hash) + 3),
+#endif
+ BPF_EXIT_INSN(),
+ },
+ .errstr = "invalid bpf_context access",
+ .result = REJECT,
+ },
+ {
+ "check skb->hash half load not permitted, unaligned 3",
+ .insns = {
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+ BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, hash) + 3),
+#else
+ BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
+ offsetof(struct __sk_buff, hash) + 1),
#endif
BPF_EXIT_INSN(),
},
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next v2 3/3] selftests/bpf: Test narrow loads with off > 0 for bpf_sock_addr
From: Andrey Ignatov @ 2018-11-11 6:15 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, yhs, kernel-team
In-Reply-To: <cover.1541916635.git.rdna@fb.com>
Add more test cases for context bpf_sock_addr to test narrow loads with
offset > 0 for ctx->user_ip4 field (__u32):
* off=1, size=1;
* off=2, size=1;
* off=3, size=1;
* off=2, size=2.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/testing/selftests/bpf/test_sock_addr.c | 28 +++++++++++++++++---
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_sock_addr.c b/tools/testing/selftests/bpf/test_sock_addr.c
index aeeb76a54d63..73b7493d4120 100644
--- a/tools/testing/selftests/bpf/test_sock_addr.c
+++ b/tools/testing/selftests/bpf/test_sock_addr.c
@@ -574,24 +574,44 @@ static int bind4_prog_load(const struct sock_addr_test *test)
/* if (sk.family == AF_INET && */
BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_6,
offsetof(struct bpf_sock_addr, family)),
- BPF_JMP_IMM(BPF_JNE, BPF_REG_7, AF_INET, 16),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_7, AF_INET, 24),
/* (sk.type == SOCK_DGRAM || sk.type == SOCK_STREAM) && */
BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_6,
offsetof(struct bpf_sock_addr, type)),
BPF_JMP_IMM(BPF_JNE, BPF_REG_7, SOCK_DGRAM, 1),
BPF_JMP_A(1),
- BPF_JMP_IMM(BPF_JNE, BPF_REG_7, SOCK_STREAM, 12),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_7, SOCK_STREAM, 20),
/* 1st_byte_of_user_ip4 == expected && */
BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_6,
offsetof(struct bpf_sock_addr, user_ip4)),
- BPF_JMP_IMM(BPF_JNE, BPF_REG_7, ip4.u4_addr8[0], 10),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_7, ip4.u4_addr8[0], 18),
+
+ /* 2nd_byte_of_user_ip4 == expected && */
+ BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_6,
+ offsetof(struct bpf_sock_addr, user_ip4) + 1),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_7, ip4.u4_addr8[1], 16),
+
+ /* 3rd_byte_of_user_ip4 == expected && */
+ BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_6,
+ offsetof(struct bpf_sock_addr, user_ip4) + 2),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_7, ip4.u4_addr8[2], 14),
+
+ /* 4th_byte_of_user_ip4 == expected && */
+ BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_6,
+ offsetof(struct bpf_sock_addr, user_ip4) + 3),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_7, ip4.u4_addr8[3], 12),
/* 1st_half_of_user_ip4 == expected && */
BPF_LDX_MEM(BPF_H, BPF_REG_7, BPF_REG_6,
offsetof(struct bpf_sock_addr, user_ip4)),
- BPF_JMP_IMM(BPF_JNE, BPF_REG_7, ip4.u4_addr16[0], 8),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_7, ip4.u4_addr16[0], 10),
+
+ /* 2nd_half_of_user_ip4 == expected && */
+ BPF_LDX_MEM(BPF_H, BPF_REG_7, BPF_REG_6,
+ offsetof(struct bpf_sock_addr, user_ip4) + 2),
+ BPF_JMP_IMM(BPF_JNE, BPF_REG_7, ip4.u4_addr16[1], 8),
/* whole_user_ip4 == expected) { */
BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_6,
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next v2 1/3] bpf: Allow narrow loads with offset > 0
From: Andrey Ignatov @ 2018-11-11 6:15 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, yhs, kernel-team
In-Reply-To: <cover.1541916635.git.rdna@fb.com>
Currently BPF verifier allows narrow loads for a context field only with
offset zero. E.g. if there is a __u32 field then only the following
loads are permitted:
* off=0, size=1 (narrow);
* off=0, size=2 (narrow);
* off=0, size=4 (full).
On the other hand LLVM can generate a load with offset different than
zero that make sense from program logic point of view, but verifier
doesn't accept it.
E.g. tools/testing/selftests/bpf/sendmsg4_prog.c has code:
#define DST_IP4 0xC0A801FEU /* 192.168.1.254 */
...
if ((ctx->user_ip4 >> 24) == (bpf_htonl(DST_IP4) >> 24) &&
where ctx is struct bpf_sock_addr.
Some versions of LLVM can produce the following byte code for it:
8: 71 12 07 00 00 00 00 00 r2 = *(u8 *)(r1 + 7)
9: 67 02 00 00 18 00 00 00 r2 <<= 24
10: 18 03 00 00 00 00 00 fe 00 00 00 00 00 00 00 00 r3 = 4261412864 ll
12: 5d 32 07 00 00 00 00 00 if r2 != r3 goto +7 <LBB0_6>
where `*(u8 *)(r1 + 7)` means narrow load for ctx->user_ip4 with size=1
and offset=3 (7 - sizeof(ctx->user_family) = 3). This load is currently
rejected by verifier.
Verifier code that rejects such loads is in bpf_ctx_narrow_access_ok()
what means any is_valid_access implementation, that uses the function,
works this way, e.g. bpf_skb_is_valid_access() for __sk_buff or
sock_addr_is_valid_access() for bpf_sock_addr.
The patch makes such loads supported. Offset can be in [0; size_default)
but has to be multiple of load size. E.g. for __u32 field the following
loads are supported now:
* off=0, size=1 (narrow);
* off=1, size=1 (narrow);
* off=2, size=1 (narrow);
* off=3, size=1 (narrow);
* off=0, size=2 (narrow);
* off=2, size=2 (narrow);
* off=0, size=4 (full).
Reported-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
include/linux/filter.h | 16 +---------------
kernel/bpf/verifier.c | 21 ++++++++++++++++-----
2 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index de629b706d1d..cc17f5f32fbb 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -668,24 +668,10 @@ static inline u32 bpf_ctx_off_adjust_machine(u32 size)
return size;
}
-static inline bool bpf_ctx_narrow_align_ok(u32 off, u32 size_access,
- u32 size_default)
-{
- size_default = bpf_ctx_off_adjust_machine(size_default);
- size_access = bpf_ctx_off_adjust_machine(size_access);
-
-#ifdef __LITTLE_ENDIAN
- return (off & (size_default - 1)) == 0;
-#else
- return (off & (size_default - 1)) + size_access == size_default;
-#endif
-}
-
static inline bool
bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
{
- return bpf_ctx_narrow_align_ok(off, size, size_default) &&
- size <= size_default && (size & (size - 1)) == 0;
+ return size <= size_default && (size & (size - 1)) == 0;
}
#define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8d0977980cfa..b5222aa61d54 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5718,10 +5718,10 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
int i, cnt, size, ctx_field_size, delta = 0;
const int insn_cnt = env->prog->len;
struct bpf_insn insn_buf[16], *insn;
+ u32 target_size, size_default, off;
struct bpf_prog *new_prog;
enum bpf_access_type type;
bool is_narrower_load;
- u32 target_size;
if (ops->gen_prologue || env->seen_direct_write) {
if (!ops->gen_prologue) {
@@ -5814,9 +5814,9 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
* we will apply proper mask to the result.
*/
is_narrower_load = size < ctx_field_size;
+ size_default = bpf_ctx_off_adjust_machine(ctx_field_size);
+ off = insn->off;
if (is_narrower_load) {
- u32 size_default = bpf_ctx_off_adjust_machine(ctx_field_size);
- u32 off = insn->off;
u8 size_code;
if (type == BPF_WRITE) {
@@ -5844,12 +5844,23 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
}
if (is_narrower_load && size < target_size) {
- if (ctx_field_size <= 4)
+ u8 shift = (off & (size_default - 1)) * 8;
+
+ if (ctx_field_size <= 4) {
+ if (shift)
+ insn_buf[cnt++] = BPF_ALU32_IMM(BPF_RSH,
+ insn->dst_reg,
+ shift);
insn_buf[cnt++] = BPF_ALU32_IMM(BPF_AND, insn->dst_reg,
(1 << size * 8) - 1);
- else
+ } else {
+ if (shift)
+ insn_buf[cnt++] = BPF_ALU64_IMM(BPF_RSH,
+ insn->dst_reg,
+ shift);
insn_buf[cnt++] = BPF_ALU64_IMM(BPF_AND, insn->dst_reg,
(1 << size * 8) - 1);
+ }
}
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 0/9] Use __vlan_hwaccel_*() helpers
From: mirq-linux @ 2018-11-11 16:12 UTC (permalink / raw)
To: Tariq Toukan
Cc: ajit.khaparde@broadcom.com, somnath.kotur@broadcom.com,
linux-rdma@vger.kernel.org, bridge@lists.linux-foundation.org,
coreteam@netfilter.org, kadlec@blackhole.kfki.hu,
kuznet@ms2.inr.ac.ru, pablo@netfilter.org, jiri@resnulli.us,
swise@chelsio.com, nikolay@cumulusnetworks.com,
roopa@cumulusnetworks.com, jhs@mojatatu.com,
sathya.perla@broadcom.com, xiyou.wangcong@gmail.com,
mlindner@marvell.com,
"sriharsha.basavapatna@broadcom.com"
In-Reply-To: <28a275b4-4fd5-2c00-217a-c6ab69fb2ad9@mellanox.com>
On Sun, Nov 11, 2018 at 03:26:29PM +0000, Tariq Toukan wrote:
>
>
> On 09/11/2018 6:45 AM, David Miller wrote:
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Date: Fri, 09 Nov 2018 00:17:58 +0100
> >
> >> This series removes from networking core and driver code an assumption
> >> about how VLAN tag presence is stored in an skb. This will allow to free
> >> up overloading of VLAN.CFI bit to incidate tag's presence.
> >
> > This looks good, series applied.
> >
> > Thanks.
> >
>
> Hi Michal,
>
> We started seeing the compilation errors below, they look related to
> this series.
Do you have the previous series applied that introduced __vlan_hwaccel_clear_tag()?
It was applied to net-next before I sent this series.
I did a allmodconfig build of net-next and saw no failures (apart from
OVS that is fixed already).
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH net-next 0/9] Use __vlan_hwaccel_*() helpers
From: mirq-linux @ 2018-11-11 16:16 UTC (permalink / raw)
To: Tariq Toukan
Cc: ajit.khaparde@broadcom.com, somnath.kotur@broadcom.com,
linux-rdma@vger.kernel.org, bridge@lists.linux-foundation.org,
coreteam@netfilter.org, kadlec@blackhole.kfki.hu,
kuznet@ms2.inr.ac.ru, pablo@netfilter.org, jiri@resnulli.us,
swise@chelsio.com, nikolay@cumulusnetworks.com,
roopa@cumulusnetworks.com, jhs@mojatatu.com,
sathya.perla@broadcom.com, xiyou.wangcong@gmail.com,
mlindner@marvell.com,
"sriharsha.basavapatna@broadcom.com"
In-Reply-To: <20181111161243.GA12849@qmqm.qmqm.pl>
On Sun, Nov 11, 2018 at 05:12:43PM +0100, mirq-linux@rere.qmqm.pl wrote:
> On Sun, Nov 11, 2018 at 03:26:29PM +0000, Tariq Toukan wrote:
> > On 09/11/2018 6:45 AM, David Miller wrote:
> > > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > Date: Fri, 09 Nov 2018 00:17:58 +0100
> > >> This series removes from networking core and driver code an assumption
> > >> about how VLAN tag presence is stored in an skb. This will allow to free
> > >> up overloading of VLAN.CFI bit to incidate tag's presence.
> > >
> > > This looks good, series applied.
> > >
> > > Thanks.
> > Hi Michal,
> >
> > We started seeing the compilation errors below, they look related to
> > this series.
>
> Do you have the previous series applied that introduced __vlan_hwaccel_clear_tag()?
> It was applied to net-next before I sent this series.
>
> I did a allmodconfig build of net-next and saw no failures (apart from
> OVS that is fixed already).
The prerequisite commit in net-next is:
7025abb2e447b ("Merge branch 'vlan-prepare-for-removal-of-VLAN_TAG_PRESENT'")
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [RFC PATCH 00/10] net: hns3: Adds support of debugfs to HNS3 driver
From: Andrew Lunn @ 2018-11-11 16:18 UTC (permalink / raw)
To: Salil Mehta
Cc: davem@davemloft.net, yuvalm@mellanox.com, leon@kernel.org,
Zhuangyuzeng (Yisen), lipeng (Y), mehta.salil@opnsrc.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rdma@vger.kernel.org, Linuxarm
In-Reply-To: <F4CC6FACFEB3C54C9141D49AD221F7F93BF9FC72@FRAEML521-MBS.china.huawei.com>
> Agreed. But what about if we want to expose anything related to
> firmware to user-space using the debugfs, assuming we are presenting
> information in structured way and not as a black-box to some user-space
> application. Is it something which might be discouraged?
Hi Salil
Please take a look at devlink. Mellonex have been adding commands to
it for dumping firmware states.
Andrew
^ permalink raw reply
* RE: [RFC PATCH 07/10] net: hns3: Add checksum info query function
From: Salil Mehta @ 2018-11-11 16:23 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem@davemloft.net, yuvalm@mellanox.com, leon@kernel.org,
Zhuangyuzeng (Yisen), lipeng (Y), mehta.salil@opnsrc.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rdma@vger.kernel.org, Linuxarm, Liuzhongzhu
In-Reply-To: <20181109225114.GF5259@lunn.ch>
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Andrew Lunn
> Sent: Friday, November 9, 2018 10:51 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: davem@davemloft.net; yuvalm@mellanox.com; leon@kernel.org;
> Zhuangyuzeng (Yisen) <yisen.zhuang@huawei.com>; lipeng (Y)
> <lipeng321@huawei.com>; mehta.salil@opnsrc.net; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> <linuxarm@huawei.com>; Liuzhongzhu <liuzhongzhu@huawei.com>
> Subject: Re: [RFC PATCH 07/10] net: hns3: Add checksum info query
> function
>
> On Fri, Nov 09, 2018 at 10:07:40PM +0000, Salil Mehta wrote:
> > From: liuzhongzhu <liuzhongzhu@huawei.com>
> >
> > This patch prints checksum config information
> > related to various layers of headers.
> >
> > debugfs command:
> > echo dump checksum > cmd
>
> How does the information here differ from ethtool -k
Sure, this should be enough. Will drop this patch.
Thanks
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] bpf: Allow narrow loads with offset > 0
From: Alexei Starovoitov @ 2018-11-11 6:39 UTC (permalink / raw)
To: Andrey Ignatov; +Cc: netdev, ast, daniel, yhs, kernel-team
In-Reply-To: <cover.1541916635.git.rdna@fb.com>
On Sat, Nov 10, 2018 at 10:15:12PM -0800, Andrey Ignatov wrote:
> This patch set adds support for narrow loads with offset > 0 to BPF
> verifier.
>
> Patch 1 provides more details and is the main patch in the set.
> Patches 2 and 3 add new test cases to test_verifier and test_sock_addr
> selftests.
>
> v1->v2:
> - fix -Wdeclaration-after-statement warning.
Applied, Thanks
^ permalink raw reply
* Re: net: ipv4: tcp_vegas
From: David Miller @ 2018-11-11 17:25 UTC (permalink / raw)
To: suraj1998; +Cc: netdev, linux-kernel
In-Reply-To: <1541956134-14029-1-git-send-email-suraj1998@gmail.com>
From: Suraj Singh <suraj1998@gmail.com>
Date: Sun, 11 Nov 2018 22:38:54 +0530
> I don't think you've been getting my emails because I've been sending them
> in response to the message-id of my patch.
I got your email, I'm just too busy to reply.
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt
From: David Miller @ 2018-11-11 17:38 UTC (permalink / raw)
To: hkallweit1
Cc: andrew, f.fainelli, netdev, bcm-kernel-feedback-list,
richardcochran, carlo, khilman, linux-kernel, linux-arm-kernel,
linux-amlogic
In-Reply-To: <203c4d9e-f39a-7a08-46c3-4ee6e61f181e@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 9 Nov 2018 18:15:31 +0100
> Flag PHY_HAS_INTERRUPT is used only here for this small check. I think
> using interrupts isn't possible if a driver defines neither
> config_intr nor ack_interrupts callback. So we can replace checking
> flag PHY_HAS_INTERRUPT with checking for these callbacks.
> This allows to remove this flag from all driver configs.
>
> v2:
> - add helper for check in patch 1
> - remove PHY_HAS_INTERRUPT from all drivers, not only Realtek
> - remove flag PHY_HAS_INTERRUPT completely
>
> v3:
> - rebase patch 2
Series applied, but please sort out that one driver which is preventing
us requiring both config_intr && ack_interrupt.
Thanks.
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Jesper Dangaard Brouer @ 2018-11-11 8:03 UTC (permalink / raw)
To: Paweł Staszewski; +Cc: Saeed Mahameed, netdev@vger.kernel.org, brouer
In-Reply-To: <428dfb15-0123-ce6d-c403-73e83caf145d@itcare.pl>
On Sat, 10 Nov 2018 23:19:50 +0100
Paweł Staszewski <pstaszewski@itcare.pl> wrote:
> W dniu 10.11.2018 o 23:06, Jesper Dangaard Brouer pisze:
> > On Sat, 10 Nov 2018 20:56:02 +0100
> > Paweł Staszewski <pstaszewski@itcare.pl> wrote:
> >
> >> W dniu 10.11.2018 o 20:49, Paweł Staszewski pisze:
> >>>
> >>> W dniu 10.11.2018 o 20:34, Jesper Dangaard Brouer pisze:
> >>>> On Fri, 9 Nov 2018 23:20:38 +0100 Paweł Staszewski
> >>>> <pstaszewski@itcare.pl> wrote:
> >>>>
> >>>>> W dniu 08.11.2018 o 20:12, Paweł Staszewski pisze:
[...]
> > Do notice, the per CPU squeeze is not too large.
>
> Yes - but im searching invisible thing now :) something invisible is
> slowing down packet processing :)
> So trying to find any counter that have something to do with packet
> processing.
NOTICE, I have given you the counters you need (below)
> >
> > [...]
> >>>>> Remember those tests are now on two separate connectx5 connected to
> >>>>> two separate pcie x16 gen 3.0
> >>>> That is strange... I still suspect some HW NIC issue, can you provide
> >>>> ethtool stats info via tool:
> >>>>
> >>>> https://github.com/netoptimizer/network-testing/blob/master/bin/ethtool_stats.pl
> >>>>
> >>>> $ ethtool_stats.pl --dev enp175s0 --dev enp216s0
> >>>>
> >>>> The tool remove zero-stats counters and report per sec stats. It makes
> >>>> it easier to spot that is relevant for the given workload.
> >>> yes mlnx have just too many counters that are always 0 for my case :)
> >>> Will try this also
> >>>
> >> But still alot of non 0 counters
> >> Show adapter(s) (enp175s0 enp216s0) statistics (ONLY that changed!)
> >> Ethtool(enp175s0) stat: 8891 ( 8,891) <= ch0_arm /sec
> > [...]
> >
> > I have copied the stats over in another document so I can better looks
> > at it... and I've found some interesting stats.
> >
> > E.g. we can see that the NIC hardware is dropping packets.
> >
> > RX-drops on enp175s0:
> >
> > (enp175s0) stat: 4850734036 ( 4,850,734,036) <= rx_bytes /sec
> > (enp175s0) stat: 5069043007 ( 5,069,043,007) <= rx_bytes_phy /sec
> > -218308971 ( -218,308,971) Dropped bytes /sec
> >
> > (enp175s0) stat: 139602 ( 139,602) <= rx_discards_phy /sec
> >
> > (enp175s0) stat: 3717148 ( 3,717,148) <= rx_packets /sec
> > (enp175s0) stat: 3862420 ( 3,862,420) <= rx_packets_phy /sec
> > -145272 ( -145,272) Dropped packets /sec
> >
> >
> > RX-drops on enp216s0 is less:
> >
> > (enp216s0) stat: 2592286809 ( 2,592,286,809) <= rx_bytes /sec
> > (enp216s0) stat: 2633575771 ( 2,633,575,771) <= rx_bytes_phy /sec
> > -41288962 ( -41,288,962) Dropped bytes /sec
> >
> > (enp216s0) stat: 464 (464) <= rx_discards_phy /sec
> >
> > (enp216s0) stat: 4971677 ( 4,971,677) <= rx_packets /sec
> > (enp216s0) stat: 4975563 ( 4,975,563) <= rx_packets_phy /sec
> > -3886 ( -3,886) Dropped packets /sec
> >
I would recommend, that you use ethtool stats and monitor rx_discards_phy.
The PHY are the counters from the hardware, and it shows that packets
are getting dropped at HW level. This can be because software is not
fast enough to empty RX-queue, but in this case where CPUs are mostly
idle I don't think that is the case.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* (unknown),
From: Oliver Carter @ 2018-11-11 8:05 UTC (permalink / raw)
To: netdev
Netdev https://goo.gl/pW8d8y Oliver Carter
^ permalink raw reply
* Re: KASAN: use-after-free Read in update_blocked_averages
From: Dmitry Vyukov @ 2018-11-11 18:24 UTC (permalink / raw)
To: syzbot; +Cc: LKML, syzkaller-bugs, netdev
In-Reply-To: <000000000000fd464c057a679adb@google.com>
On Sun, Nov 11, 2018 at 10:18 AM, syzbot
<syzbot+0dbf864d3b52555e8265@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 12ceaf8864c2 infiniband: nes: Fix more direct skb list acc..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=15a82783400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=dcbea7daf3ea3e3e
> dashboard link: https://syzkaller.appspot.com/bug?extid=0dbf864d3b52555e8265
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=137ae6d5400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15dbd77b400000
Looking at the reproducer this looks network related, +netdev
r0 = socket$inet6(0xa, 0x2, 0x0)
connect$inet6(r0, &(0x7f0000000100)={0xa, 0x0, 0x0, @dev, 0x6}, 0x1c)
connect$inet6(r0, &(0x7f0000000580)={0xa, 0x4e22, 0x0, @ipv4={[], [],
@local}}, 0x1c)
sendmmsg(r0, &(0x7f00000092c0), 0x4ff, 0x0)
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+0dbf864d3b52555e8265@syzkaller.appspotmail.com
>
> IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
> IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
> 8021q: adding VLAN 0 to HW filter on device team0
> ==================================================================
> BUG: KASAN: use-after-free in skip_blocked_update kernel/sched/fair.c:3324
> [inline]
> BUG: KASAN: use-after-free in update_blocked_averages+0x1533/0x1e00
> kernel/sched/fair.c:7400
> kasan: CONFIG_KASAN_INLINE enabled
> Read of size 8 at addr ffff8801bf0d6ea0 by task syz-executor841/6015
>
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> CPU: 1 PID: 6015 Comm: syz-executor841 Not tainted 4.20.0-rc1+ #289
> general protection fault: 0000 [#1] PREEMPT SMP KASAN
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> CPU: 0 PID: 6272 Comm: syz-executor841 Not tainted 4.20.0-rc1+ #289
> Call Trace:
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x244/0x39d lib/dump_stack.c:113
> RIP: 0010:vmalloc_fault+0x426/0x770 arch/x86/mm/fault.c:405
> Code: e0 e8 fe 25 47 00 48 b8 00 00 00 00 00 88 ff ff 48 ba 00 00 00 00 00
> fc ff df 48 01 c3 4d 21 e5 4c 01 eb 48 89 d9 48 c1 e9 03 <80> 3c 11 00 0f 85
> b2 02 00 00 48 8b 1b 31 ff 49 89 dc 49 83 e4 9f
> RSP: 0018:ffff8801d9d624c8 EFLAGS: 00010002
> RAX: ffff880000000000 RBX: 000f100180000040 RCX: 0001e20030000008
> print_address_description.cold.7+0x9/0x1ff mm/kasan/report.c:256
> RDX: dffffc0000000000 RSI: ffffffff813864c2 RDI: 0000000000000007
> kasan_report_error mm/kasan/report.c:354 [inline]
> kasan_report.cold.8+0x242/0x309 mm/kasan/report.c:412
> RBP: ffff8801d9d624f8 R08: ffff8801bbb56700 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 000fffffc0000000
> __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
> R13: 000f880180000000 R14: ffffc90001159838 R15: 1ffffffff12a3f98
> skip_blocked_update kernel/sched/fair.c:3324 [inline]
> update_blocked_averages+0x1533/0x1e00 kernel/sched/fair.c:7400
> FS: 0000000001f7d880(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffc90001159838 CR3: 00000001ba705000 CR4: 00000000001406f0
> Call Trace:
> Modules linked in:
> ---[ end trace f6450057874cc9c7 ]---
> RIP: 0010:vmalloc_fault+0x426/0x770 arch/x86/mm/fault.c:405
> Code: e0 e8 fe 25 47 00 48 b8 00 00 00 00 00 88 ff ff 48 ba 00 00 00 00 00
> fc ff df 48 01 c3 4d 21 e5 4c 01 eb 48 89 d9 48 c1 e9 03 <80> 3c 11 00 0f 85
> b2 02 00 00 48 8b 1b 31 ff 49 89 dc 49 83 e4 9f
> RSP: 0018:ffff8801d9d624c8 EFLAGS: 00010002
> RAX: ffff880000000000 RBX: 000f100180000040 RCX: 0001e20030000008
> RDX: dffffc0000000000 RSI: ffffffff813864c2 RDI: 0000000000000007
> RBP: ffff8801d9d624f8 R08: ffff8801bbb56700 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 000fffffc0000000
> R13: 000f880180000000 R14: ffffc90001159838 R15: 1ffffffff12a3f98
> FS: 0000000001f7d880(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffc90001159838 CR3: 00000001ba705000 CR4: 00000000001406f0
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/syzkaller-bugs/000000000000fd464c057a679adb%40google.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH net-next v2] tcp: minor optimization in tcp ack fast path processing
From: David Miller @ 2018-11-11 18:26 UTC (permalink / raw)
To: laoar.shao; +Cc: edumazet, netdev, linux-kernel, joe
In-Reply-To: <1541938210-11797-1-git-send-email-laoar.shao@gmail.com>
From: Yafang Shao <laoar.shao@gmail.com>
Date: Sun, 11 Nov 2018 20:10:10 +0800
> Bitwise operation is a little faster.
> So I replace after() with using the flag FLAG_SND_UNA_ADVANCED as it is
> already set before.
>
> In addtion, there's another similar improvement in tcp_cwnd_reduction().
>
> Cc: Joe Perches <joe@perches.com>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: mv88e6xxx: Work around mv886e6161 SERDES missing MII_PHYSID2
From: Sergei Shtylyov @ 2018-11-11 8:44 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev
In-Reply-To: <1541893839-4828-1-git-send-email-andrew@lunn.ch>
Hello!
On 11.11.2018 2:50, Andrew Lunn wrote:
> We already have a workaround for a couple of switches whose internal
> PHYs only have the Marvel OUI, but no model number. We detect such
> PHYs and give them the 6390 ID as the model number. However the
> mv88e6161 has two SERDES interfaces in the same address range as its
> internal PHYs. These suffer from the same problem, the Marvell OUI,
> but no model number. As a result, these SERDES interfaces were getting
> the same PHY ID as the mv88e6390, even though they are not PHYs, and
> the Marvell PHY driver was trying to drive them.
>
> Add a special case to stop this happen.
Dtop this from happening, maybe?
>
> Reported-by: Chris Healy <Chris.Healy@zii.aero>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> drivers/net/dsa/mv88e6xxx/chip.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index fc0f508879d4..4906a4f39d62 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2524,11 +2524,22 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
> mutex_unlock(&chip->reg_lock);
>
> if (reg == MII_PHYSID2) {
> - /* Some internal PHYS don't have a model number. Use
> - * the mv88e6390 family model number instead.
> - */
> - if (!(val & 0x3f0))
> - val |= MV88E6XXX_PORT_SWITCH_ID_PROD_6390 >> 4;
> + /* Some internal PHYS don't have a model number. */
PHYs (else you're inconsistent with the next comment).
> + if (chip->info->family != MV88E6XXX_FAMILY_6165)
> + /* Then there is the 6165 family. It gets is
> + * PHYs correct. But it can also have two
> + * SERDES interfaces in the PHY address
> + * space. And these don't have a model
> + * number. But they are not PHYs, so we don't
> + * want to give them something a PHY driver
> + * will recognise.
> + *
> + * Use the mv88e6390 family model number
> + * instead, for anything which really could be
> + * a PHY,
> + */
> + if (!(val & 0x3f0))
> + val |= MV88E6XXX_PORT_SWITCH_ID_PROD_6390 >> 4;
> }
>
> return err ? err : val;
MBR, Sergei
^ permalink raw reply
* Re: [PATCH] iucv: Remove SKB list assumptions.
From: Sergei Shtylyov @ 2018-11-11 8:48 UTC (permalink / raw)
To: David Miller, netdev
In-Reply-To: <20181110.165545.1492330402009389392.davem@davemloft.net>
Hello!
On 11.11.2018 3:55, David Miller wrote:
> Eliminate the assumption that SKBs and SKB list heads can
> be cast to eachother in SKB list handling code.
Each other? My spellchecker trips here.
> This change also appears to fix a bug since the list->next pointer is
> sampled outside of holding the SKB queue lock.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
[...]
MBR, Sergei
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox