* Re: [PATCH bpf-next 0/3] bpf: Allow narrow loads with offset > 0
From: Alexei Starovoitov @ 2018-11-10 23:38 UTC (permalink / raw)
To: Andrey Ignatov
Cc: Network Development, Alexei Starovoitov, Daniel Borkmann,
Yonghong Song, Kernel Team
In-Reply-To: <20181110191345.vbqst4ephtfxk2jx@ast-mbp.dhcp.thefacebook.com>
On Sat, Nov 10, 2018 at 11:13 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Nov 06, 2018 at 05:23:25PM -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.
>
> Applied, Thanks
reverted.
please fix compiler warning and respin.
^ permalink raw reply
* [patch net] net: dsa: mv88e6xxx: Fix clearing of stats counters
From: Andrew Lunn @ 2018-11-10 23:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Andrew Lunn
The mv88e6161 would sometime fail to probe with a timeout waiting for
the switch to complete an operation. This operation is supposed to
clear the statistics counters. However, due to a read/modify/write,
without the needed mask, the operation actually carried out was more
random, with invalid parameters, resulting in the switch not
responding. We need to preserve the histogram mode bits, so apply a
mask to keep them.
Reported-by: Chris Healy <Chris.Healy@zii.aero>
Fixes: 40cff8fca9e3 ("net: dsa: mv88e6xxx: Fix stats histogram mode")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/dsa/mv88e6xxx/global1.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/global1.c b/drivers/net/dsa/mv88e6xxx/global1.c
index d721ccf7d8be..38e399e0f30e 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.c
+++ b/drivers/net/dsa/mv88e6xxx/global1.c
@@ -567,6 +567,8 @@ int mv88e6xxx_g1_stats_clear(struct mv88e6xxx_chip *chip)
if (err)
return err;
+ /* Keep the histogram mode bits */
+ val &= MV88E6XXX_G1_STATS_OP_HIST_RX_TX;
val |= MV88E6XXX_G1_STATS_OP_BUSY | MV88E6XXX_G1_STATS_OP_FLUSH_ALL;
err = mv88e6xxx_g1_write(chip, MV88E6XXX_G1_STATS_OP, val);
--
2.19.1
^ permalink raw reply related
* Re: [PATCH bpf-next] filter: add BPF_ADJ_ROOM_DATA mode to bpf_skb_adjust_room()
From: Nicolas Dichtel @ 2018-11-10 23:43 UTC (permalink / raw)
To: Martin Lau
Cc: ast@kernel.org, daniel@iogearbox.net, davem@davemloft.net,
netdev@vger.kernel.org
In-Reply-To: <20181109185131.7f7osxg2tfp2xapx@kafai-mbp.dhcp.thefacebook.com>
Le 09/11/2018 à 19:51, Martin Lau a écrit :
> On Thu, Nov 08, 2018 at 04:11:37PM +0100, Nicolas Dichtel wrote:
[snip]
>> +static int bpf_skb_data_shrink(struct sk_buff *skb, u32 len)
>> +{
>> + unsigned short hhlen = skb->dev->header_ops ?
>> + skb->dev->hard_header_len : 0;
>> + int ret;
>> +
>> + ret = skb_unclone(skb, GFP_ATOMIC);
>> + if (unlikely(ret < 0))
>> + return ret;
>> +
>> + __skb_pull(skb, len);
>> + skb_reset_mac_header(skb);
>> + skb_reset_network_header(skb);
>> + skb->network_header += hhlen;
>> + skb_reset_transport_header(skb);
> hmm...why transport_header does not need += hhlen here
> while network_header does?
network_header is mandatory because bpf_redirect(BPF_F_INGRESS) can be called
and network_header is expected to be correctly set in this case.
For transport_header, I choose to not set it, because the stack will set it
later (for example ip_rcv_core()).
Regards,
Nicolas
^ permalink raw reply
* [PATCH net-next 0/2] r8169: add USR PCI vendor id
From: Heiner Kallweit @ 2018-11-10 23:43 UTC (permalink / raw)
To: Bjorn Helgaas, David Miller
Cc: netdev@vger.kernel.org, linux-pci@vger.kernel.org
Add constant for U.S. Robotics (USR) PCI id and use it in the r8169
driver. Alternatively this change could go thorugh the PCI tree,
up to Bjorn/David.
Heiner Kallweit (2):
PCI: add USR vendor id
r8169: use proper constant for PCI vendor USR instead of id
drivers/net/ethernet/realtek/r8169.c | 2 +-
include/linux/pci_ids.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
--
2.19.1
^ permalink raw reply
* Re: Should the bridge learn from frames with link local destination MAC address?
From: Andrew Lunn @ 2018-11-10 23:46 UTC (permalink / raw)
To: nikolay; +Cc: Roopa Prabhu, Stephen Hemminger, netdev, Florian Fainelli
In-Reply-To: <30C8207C-1DA3-44B0-8BD2-66B10D0C406E@cumulusnetworks.com>
> >Andrew, I agree with your analysis also. We have hit this problem too
> >(and we have an internal bug tracking it).
> >We have not acted on this so far because of the fear of breaking
> >existing deployments. I am all for fixing this if there is a
> >clean way.
>
> +1 and since this would be a new bridge boolean option I'd like to add one new
> 64 bit option with mask for new boolean bridge options so we can avoid
> increasing the max rtnl attr id for such options. Please let me know
> if you plan to work on the new option or I can cook something.
Hi Nik
For the moment i made a hack, which is enough for my own personal use.
I'm not too familiar with the bridge code and its netlink interface. I
suspect you can implement this properly much quicker than i could. So
i would prefer leaving it to you. But we can talk about this during
LPC.
Andrew
^ permalink raw reply
* [PATCH net-next 1/2] PCI: add USR vendor id
From: Heiner Kallweit @ 2018-11-10 23:45 UTC (permalink / raw)
To: Bjorn Helgaas, David Miller
Cc: netdev@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <9af6abf5-ba01-155f-17d1-ca4b71b74fa6@gmail.com>
Add U.S. Robotics (USR) PCI vendor id.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
include/linux/pci_ids.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 69f0abe1b..144de2e89 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2359,6 +2359,8 @@
#define PCI_VENDOR_ID_SYNOPSYS 0x16c3
+#define PCI_VENDOR_ID_USR 0x16ec
+
#define PCI_VENDOR_ID_VITESSE 0x1725
#define PCI_DEVICE_ID_VITESSE_VSC7174 0x7174
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 2/2] r8169: use proper constant for PCI vendor USR instead of numerical id
From: Heiner Kallweit @ 2018-11-10 23:47 UTC (permalink / raw)
To: Bjorn Helgaas, David Miller
Cc: netdev@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <9af6abf5-ba01-155f-17d1-ca4b71b74fa6@gmail.com>
Use proper constant for PCI vendor USR instead of numerical id.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 1fd01688d..366a690eb 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -224,7 +224,7 @@ static const struct pci_device_id rtl8169_pci_tbl[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_DLINK, 0x4300), 0, 0, RTL_CFG_0 },
{ PCI_DEVICE(PCI_VENDOR_ID_DLINK, 0x4302), 0, 0, RTL_CFG_0 },
{ PCI_DEVICE(PCI_VENDOR_ID_AT, 0xc107), 0, 0, RTL_CFG_0 },
- { PCI_DEVICE(0x16ec, 0x0116), 0, 0, RTL_CFG_0 },
+ { PCI_DEVICE(PCI_VENDOR_ID_USR, 0x0116), 0, 0, RTL_CFG_0 },
{ PCI_VENDOR_ID_LINKSYS, 0x1032,
PCI_ANY_ID, 0x0024, 0, 0, RTL_CFG_0 },
{ 0x0001, 0x8168,
--
2.19.1
^ permalink raw reply related
* [PATCH net-next] net: dsa: mv88e6xxx: Work around mv886e6161 SERDES missing MII_PHYSID2
From: Andrew Lunn @ 2018-11-10 23:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Andrew Lunn
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.
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. */
+ 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;
--
2.19.1
^ permalink raw reply related
* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Paweł Staszewski @ 2018-11-10 23:54 UTC (permalink / raw)
To: Alexander Duyck, aaron.lu
Cc: linux-mm, LKML, Netdev, Andrew Morton, Jesper Dangaard Brouer,
Eric Dumazet, Tariq Toukan, ilias.apalodimas, yoel, Mel Gorman,
Saeed Mahameed, Michal Hocko, Vlastimil Babka, dave.hansen
In-Reply-To: <CAKgT0UdvYVTA8OjgLhXo9tRUOGikrCi3zJXSrqM0ZmeHb5P2mA@mail.gmail.com>
W dniu 05.11.2018 o 16:44, Alexander Duyck pisze:
> On Mon, Nov 5, 2018 at 12:58 AM Aaron Lu <aaron.lu@intel.com> wrote:
>> page_frag_free() calls __free_pages_ok() to free the page back to
>> Buddy. This is OK for high order page, but for order-0 pages, it
>> misses the optimization opportunity of using Per-Cpu-Pages and can
>> cause zone lock contention when called frequently.
>>
>> Paweł Staszewski recently shared his result of 'how Linux kernel
>> handles normal traffic'[1] and from perf data, Jesper Dangaard Brouer
>> found the lock contention comes from page allocator:
>>
>> mlx5e_poll_tx_cq
>> |
>> --16.34%--napi_consume_skb
>> |
>> |--12.65%--__free_pages_ok
>> | |
>> | --11.86%--free_one_page
>> | |
>> | |--10.10%--queued_spin_lock_slowpath
>> | |
>> | --0.65%--_raw_spin_lock
>> |
>> |--1.55%--page_frag_free
>> |
>> --1.44%--skb_release_data
>>
>> Jesper explained how it happened: mlx5 driver RX-page recycle
>> mechanism is not effective in this workload and pages have to go
>> through the page allocator. The lock contention happens during
>> mlx5 DMA TX completion cycle. And the page allocator cannot keep
>> up at these speeds.[2]
>>
>> I thought that __free_pages_ok() are mostly freeing high order
>> pages and thought this is an lock contention for high order pages
>> but Jesper explained in detail that __free_pages_ok() here are
>> actually freeing order-0 pages because mlx5 is using order-0 pages
>> to satisfy its page pool allocation request.[3]
>>
>> The free path as pointed out by Jesper is:
>> skb_free_head()
>> -> skb_free_frag()
>> -> skb_free_frag()
>> -> page_frag_free()
>> And the pages being freed on this path are order-0 pages.
>>
>> Fix this by doing similar things as in __page_frag_cache_drain() -
>> send the being freed page to PCP if it's an order-0 page, or
>> directly to Buddy if it is a high order page.
>>
>> With this change, Paweł hasn't noticed lock contention yet in
>> his workload and Jesper has noticed a 7% performance improvement
>> using a micro benchmark and lock contention is gone.
>>
>> [1]: https://www.spinics.net/lists/netdev/msg531362.html
>> [2]: https://www.spinics.net/lists/netdev/msg531421.html
>> [3]: https://www.spinics.net/lists/netdev/msg531556.html
>> Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
>> Analysed-by: Jesper Dangaard Brouer <brouer@redhat.com>
>> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
>> ---
>> mm/page_alloc.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index ae31839874b8..91a9a6af41a2 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -4555,8 +4555,14 @@ void page_frag_free(void *addr)
>> {
>> struct page *page = virt_to_head_page(addr);
>>
>> - if (unlikely(put_page_testzero(page)))
>> - __free_pages_ok(page, compound_order(page));
>> + if (unlikely(put_page_testzero(page))) {
>> + unsigned int order = compound_order(page);
>> +
>> + if (order == 0)
>> + free_unref_page(page);
>> + else
>> + __free_pages_ok(page, order);
>> + }
>> }
>> EXPORT_SYMBOL(page_frag_free);
>>
> One thing I would suggest for Pawel to try would be to reduce the Tx
> qdisc size on his transmitting interfaces, Reduce the Tx ring size,
> and possibly increase the Tx interrupt rate. Ideally we shouldn't have
> too many packets in-flight and I suspect that is the issue that Pawel
> is seeing that is leading to the page pool allocator freeing up the
> memory. I know we like to try to batch things but the issue is
> processing too many Tx buffers in one batch leads to us eating up too
> much memory and causing evictions from the cache. Ideally the Rx and
> Tx rings and queues should be sized as small as possible while still
> allowing us to process up to our NAPI budget. Usually I run things
> with a 128 Rx / 128 Tx setup and then reduce the Tx queue length so we
> don't have more buffers stored there than we can place in the Tx ring.
> Then we can avoid the extra thrash of having to pull/push memory into
> and out of the freelists. Essentially the issue here ends up being
> another form of buffer bloat.
Thanks Aleksandar - yes it can be - but in my scenario setting RX buffer
<4096 producing more interface rx drops - and no_rx_buffer on network
controller that is receiving more packets
So i need to stick with 3000-4000 on RX - and yes i was trying to lower
the TX buff on connectx4 - but that changed nothing before Aaron patch
After Aaron patch - decreasing TX buffer influencing total bandwidth
that can be handled by the router/server
Dono why before this patch there was no difference there no matter what
i set there there was always page_alloc/slowpath on top in perf
Currently testing RX4096/TX256 - this helps with bandwidth like +10%
more bandwidth with less interrupts...
>
> With that said this change should be mostly harmless and does address
> the fact that we can have both regular order 0 pages and page frags
> used for skb->head.
>
> Acked-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>
^ permalink raw reply
* Re: [PATCH v5 bpf-next 0/7] bpftool: support loading flow dissector
From: Alexei Starovoitov @ 2018-11-11 0:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Stanislav Fomichev, netdev, linux-kselftest, ast, daniel, shuah,
quentin.monnet, guro, jiong.wang, bhole_prashant_q7,
john.fastabend, jbenc, treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181109121511.254dfd59@cakuba.netronome.com>
On Fri, Nov 09, 2018 at 12:15:11PM -0800, Jakub Kicinski wrote:
> On Fri, 9 Nov 2018 08:21:39 -0800, Stanislav Fomichev wrote:
> > v5 changes:
> > * FILE -> PATH for load/loadall (can be either file or directory now)
> > * simpler implementation for __bpf_program__pin_name
> > * removed p_err for REQ_ARGS checks
> > * parse_atach_detach_args -> parse_attach_detach_args
> > * for -> while in bpf_object__pin_{programs,maps} recovery
>
> Thanks! Patch 3 needs attention from maintainers but the rest LGTM!
Whole thing looks good to me.
Applied, Thanks everyone
^ permalink raw reply
* [PATCH net] act_mirred: clear skb->tstamp on redirect
From: Eric Dumazet @ 2018-11-11 0:22 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
If sch_fq is used at ingress, skbs that might have been
timestamped by net_timestamp_set() if a packet capture
is requesting timestamps could be delayed by arbitrary
amount of time, since sch_fq time base is MONOTONIC.
Fix this problem by moving code from sch_netem.c to act_mirred.c.
Fixes: fb420d5d91c1 ("tcp/fq: move back to CLOCK_MONOTONIC")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/sched/act_mirred.c | 3 ++-
net/sched/sch_netem.c | 9 ---------
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 1dae5f2b358fcf3dac2bbc0ef80f53b0dba16c91..c8cf4d10c4355f934c02d407ec725670e36433b8 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -258,7 +258,8 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
if (is_redirect) {
skb2->tc_redirected = 1;
skb2->tc_from_ingress = skb2->tc_at_ingress;
-
+ if (skb2->tc_from_ingress)
+ skb2->tstamp = 0;
/* let's the caller reinsert the packet, if possible */
if (use_reinsert) {
res->ingress = want_ingress;
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 57b3ad9394ad7a9f42c48d7645a61a6a03b2efe0..2c38e3d0792468162ee0dc4137f1400160ab9276 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -648,15 +648,6 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)
*/
skb->dev = qdisc_dev(sch);
-#ifdef CONFIG_NET_CLS_ACT
- /*
- * If it's at ingress let's pretend the delay is
- * from the network (tstamp will be updated).
- */
- if (skb->tc_redirected && skb->tc_from_ingress)
- skb->tstamp = 0;
-#endif
-
if (q->slot.slot_next) {
q->slot.packets_left--;
q->slot.bytes_left -= qdisc_pkt_len(skb);
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* [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
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