Netdev List
 help / color / mirror / Atom feed
* Re: [Patch net-next 2/2] net: dump whole skb data in netdev_rx_csum_fault()
From: Eric Dumazet @ 2018-11-21 18:26 UTC (permalink / raw)
  To: Cong Wang; +Cc: Eric Dumazet, netdev, Herbert Xu, David Miller
In-Reply-To: <CAM_iQpXO1Etf0489ssLoGGg9bgtZWuYSYb25Si913aOi5pgsBQ@mail.gmail.com>

On Wed, Nov 21, 2018 at 10:17 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Wed, Nov 21, 2018 at 5:05 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> >
> >
> > On 11/20/2018 06:13 PM, Cong Wang wrote:
> > > Currently, we only dump a few selected skb fields in
> > > netdev_rx_csum_fault(). It is not suffient for debugging checksum
> > > fault. This patch introduces skb_dump() which dumps skb mac header,
> > > network header and its whole skb->data too.
> > >
> > > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Cc: David Miller <davem@davemloft.net>
> > > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> > > ---
> >
> >
> > > +     print_hex_dump(level, "skb data: ", DUMP_PREFIX_OFFSET, 16, 1,
> > > +                    skb->data, skb->len, false);
> >
> > As I mentioned to David, we want all the bytes that were maybe already pulled
> >
> > (skb->head starting point, not skb->data)
>
> Hmm, with mac header and network header, it is effectively from skb->head, no?
> Is there anything between skb->head and mac header?

Oh, I guess we wanted a single hex dump, or we need some user program
to be able to
rebuild from different memory zones the original CHECKSUM_COMPLETE value.

>
> >
> > Also we will miss the trimmed bytes if there were padding data.
> > And it seems the various bugs we have are all tied to the pulled or trimmed bytes.
> >
>
> Unless I miss something, the tailing padding data should be in range
> [iphdr->tot_len, skb->len]. No?


Not after we did the pskb_trim_rcsum() call, since it has effectively
reduced skb->len by the number of padding bytes.

^ permalink raw reply

* [PATCH v2 net] lan743x: fix return value for lan743x_tx_napi_poll
From: Bryan Whitehead @ 2018-11-21 18:31 UTC (permalink / raw)
  To: davem; +Cc: netdev, UNGLinuxDriver

The lan743x driver, when under heavy traffic load, has been noticed
to sometimes hang, or cause a kernel panic.

Debugging reveals that the TX napi poll routine was returning
the wrong value, 'weight'. Most other drivers return 0.
And call napi_complete, instead of napi_complete_done.

Additionally when creating the tx napi poll routine.
Changed netif_napi_add, to netif_tx_napi_add.

Updates for v2:
use napi_complete, instead of napi_complete_done in
    lan743x_tx_napi_poll
use netif_tx_napi_add, instead of netif_napi_add for
    registration of tx napi poll routine

fixes: rare kernel panic under heavy traffic load.
Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>
---
 drivers/net/ethernet/microchip/lan743x_main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 867cddb..d627129 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -1672,7 +1672,7 @@ static int lan743x_tx_napi_poll(struct napi_struct *napi, int weight)
 		netif_wake_queue(adapter->netdev);
 	}
 
-	if (!napi_complete_done(napi, weight))
+	if (!napi_complete(napi))
 		goto done;
 
 	/* enable isr */
@@ -1681,7 +1681,7 @@ static int lan743x_tx_napi_poll(struct napi_struct *napi, int weight)
 	lan743x_csr_read(adapter, INT_STS);
 
 done:
-	return weight;
+	return 0;
 }
 
 static void lan743x_tx_ring_cleanup(struct lan743x_tx *tx)
@@ -1870,9 +1870,9 @@ static int lan743x_tx_open(struct lan743x_tx *tx)
 	tx->vector_flags = lan743x_intr_get_vector_flags(adapter,
 							 INT_BIT_DMA_TX_
 							 (tx->channel_number));
-	netif_napi_add(adapter->netdev,
-		       &tx->napi, lan743x_tx_napi_poll,
-		       tx->ring_size - 1);
+	netif_tx_napi_add(adapter->netdev,
+			  &tx->napi, lan743x_tx_napi_poll,
+			  tx->ring_size - 1);
 	napi_enable(&tx->napi);
 
 	data = 0;
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH bpf-next] bpf: add read/write access to skb->tstamp from tc clsact progs
From: Vlad Dumitrescu @ 2018-11-21 18:48 UTC (permalink / raw)
  To: eric.dumazet, alexei.starovoitov
  Cc: Vlad Dumitrescu, ast, Daniel Borkmann, netdev, edumazet, willemb
In-Reply-To: <623fe7c7-47a2-6fb9-be82-b3637a18b7a9@gmail.com>

On Wed, Nov 21, 2018 at 5:08 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 11/20/2018 06:40 PM, Alexei Starovoitov wrote:
>
> >
> > looks good to me.
> >
> > Any particular reason you decided to disable it for cg_skb ?
> > It seems to me the same EDT approach will work from
> > cgroup-bpf skb hooks just as well and then we can have neat
> > way of controlling traffic per-container instead of tc-clsbpf global.
> > If you're already on cgroup v2 it will save you a lot of classifier
> > cycles, since you'd be able to group apps by cgroup
> > instead of relying on ip only.
>
> Vlad first wrote a complete version, but we felt explaining the _why_
> was probably harder.
>
> No particular reason, other than having to write more tests perhaps.

This sounds reasonable to me. I can prepare a v2.

Any concerns regarding capabilities? For example data and data_end are
only available to CAP_SYS_ADMIN. Note that enforcement of this would
be done by a global component later in the pipeline (e.g., FQ qdisc).

Any opinions on sk_filter, lwt, and sk_skb before I send v2?

^ permalink raw reply

* Re: [PATCH net] sctp: hold transport before accessing its asoc in sctp_hash_transport
From: Marcelo Ricardo Leitner @ 2018-11-21 18:52 UTC (permalink / raw)
  To: Neil Horman; +Cc: Xin Long, network dev, linux-sctp, davem
In-Reply-To: <20181121132721.GA11254@hmswarspite.think-freely.org>

On Wed, Nov 21, 2018 at 08:27:21AM -0500, Neil Horman wrote:
> On Tue, Nov 20, 2018 at 10:46:26PM -0200, Marcelo Ricardo Leitner wrote:
> > On Tue, Nov 20, 2018 at 07:52:48AM -0500, Neil Horman wrote:
> > > On Tue, Nov 20, 2018 at 07:09:16PM +0800, Xin Long wrote:
> > > > In sctp_hash_transport, it dereferences a transport's asoc only under
> > > > rcu_read_lock. Without holding the transport, its asoc could be freed
> > > > already, which leads to a use-after-free panic.
> > > > 
> > > > A similar fix as Commit bab1be79a516 ("sctp: hold transport before
> > > > accessing its asoc in sctp_transport_get_next") is needed to hold
> > > > the transport before accessing its asoc in sctp_hash_transport.
> > > > 
> > > > Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport")
> > > > Reported-by: syzbot+0b05d8aa7cb185107483@syzkaller.appspotmail.com
> > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > ---
> > > >  net/sctp/input.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/net/sctp/input.c b/net/sctp/input.c
> > > > index 5c36a99..69584e9 100644
> > > > --- a/net/sctp/input.c
> > > > +++ b/net/sctp/input.c
> > > > @@ -896,11 +896,16 @@ int sctp_hash_transport(struct sctp_transport *t)
> > > >  	list = rhltable_lookup(&sctp_transport_hashtable, &arg,
> > > >  			       sctp_hash_params);
> > > >  
> > > > -	rhl_for_each_entry_rcu(transport, tmp, list, node)
> > > > +	rhl_for_each_entry_rcu(transport, tmp, list, node) {
> > > > +		if (!sctp_transport_hold(transport))
> > > > +			continue;
> > > >  		if (transport->asoc->ep == t->asoc->ep) {
> > > > +			sctp_transport_put(transport);
> > > >  			rcu_read_unlock();
> > > >  			return -EEXIST;
> > > >  		}
> > > > +		sctp_transport_put(transport);
> > > > +	}
> > > >  	rcu_read_unlock();
> > > >  
> > > >  	err = rhltable_insert_key(&sctp_transport_hashtable, &arg,
> > > > -- 
> > > > 2.1.0
> > > > 
> > > > 
> > > 
> > > something doesn't feel at all right about this.  If we are inserting a transport
> > > to an association, it would seem to me that we should have at least one user of
> > > the association (i.e. non-zero refcount).  As such it seems something is wrong
> > > with the association refcount here.  At the very least, if there is a case where
> > > an association is being removed while a transport is being added, the better
> > > solution would be to ensure that sctp_association_destroy goes through a
> > > quiescent point prior to unhashing transports from the list, to ensure that
> > > there is no conflict with the add operation above.
> > 
> > Consider that the rhl_for_each_entry_rcu() is traversing the global
> > rhashtable, and that it may operate on unrelated transports/asocs.
> > E.g., transport->asoc in the for() is potentially different from the
> > asoc under socket lock.
> > 
> Ah, ok, we're comparing associations that are not related to the association
> being searched for, that makes sense.
> 
> > The core of the fix is at:
> > +		if (!sctp_transport_hold(transport))
> > +			continue;
> > If we can get a hold, the asoc will be available for dereferencing in
> > subsequent lines. Otherwise, move on.
> > 
> > With that, the patch makes sense to me.
> > 
> Yes, I agree, but as you note below, this still seems like a lousy way to fix
> the problem.
> 
> > Although I would prefer if we come up with a better way to do this
> > jump, or even avoid the jump. We are only comparing pointers here and
> > if we had asoc->ep cached on sctp_transport itself, we could avoid the
> > atomics here.
> > 
> > This change, in the next patch on sctp_epaddr_lookup_transport, will
> > hurt performance as that is called in datapath. Rhashtable will help
> > on keeping entry lists to a size, but still.
> > 
> I still think the rcu_read_lock would be sufficient here, if we just ensured
> that removals from the list occured after a quiescent point.  The lookup is in

I'm not sure I follow.

> the datapath, but adds/removes can have a little more latency added to them, and
> if it removes the atomic operation from the fast path, I think thats a net win.

Agree.

^ permalink raw reply

* [PATCH bpf-next] bpf: fix a libbpf loader issue
From: Yonghong Song @ 2018-11-21 19:22 UTC (permalink / raw)
  To: ast, daniel, netdev; +Cc: kernel-team

Commit 2993e0515bb4 ("tools/bpf: add support to read .BTF.ext sections")
added support to read .BTF.ext sections from an object file, create
and pass prog_btf_fd and func_info to the kernel.

The program btf_fd (prog->btf_fd) is initialized to be -1 to please
zclose so we do not need special handling dur prog close.
Passing -1 to the kernel, however, will cause loading error.
Passing btf_fd 0 to the kernel if prog->btf_fd is invalid
fixed the problem.

Fixes: 2993e0515bb4 ("tools/bpf: add support to read .BTF.ext sections")
Reported-by: Andrey Ignatov <rdna@fb.com>
Reported-by: Emre Cantimur <haydum@fb.com>
Tested-by: Andrey Ignatov <rdna@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/lib/bpf/libbpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index cb6565d79603..f022ac82e882 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1387,7 +1387,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
 	load_attr.license = license;
 	load_attr.kern_version = kern_version;
 	load_attr.prog_ifindex = prog->prog_ifindex;
-	load_attr.prog_btf_fd = prog->btf_fd;
+	load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
 	load_attr.func_info = prog->func_info;
 	load_attr.func_info_rec_size = prog->func_info_rec_size;
 	load_attr.func_info_cnt = func_info_cnt;
-- 
2.17.1

^ permalink raw reply related

* Re: [RFC v3 0/3] Add VRF support for VXLAN underlay
From: David Ahern @ 2018-11-21 19:26 UTC (permalink / raw)
  To: Alexis Bauvin, roopa; +Cc: netdev, akherbouche
In-Reply-To: <16C39F29-3805-4E23-9282-E3B9AF9020A7@scaleway.com>

On 11/21/18 6:30 AM, Alexis Bauvin wrote:
> Le 20 nov. 2018 à 22:45, David Ahern <dsa@cumulusnetworks.com> a écrit :
>>
>> On 11/20/18 7:23 AM, Alexis Bauvin wrote:
>>> We are trying to isolate the VXLAN traffic from different VMs with VRF as shown
>>> in the schemas below:
>>>
>>> +-------------------------+   +----------------------------+
>>> | +----------+            |   |     +------------+         |
>>> | |          |            |   |     |            |         |
>>> | | tap-red  |            |   |     |  tap-blue  |         |
>>> | |          |            |   |     |            |         |
>>> | +----+-----+            |   |     +-----+------+         |
>>> |      |                  |   |           |                |
>>> |      |                  |   |           |                |
>>> | +----+---+              |   |      +----+----+           |
>>> | |        |              |   |      |         |           |
>>> | | br-red |              |   |      | br-blue |           |
>>> | |        |              |   |      |         |           |
>>> | +----+---+              |   |      +----+----+           |
>>> |      |                  |   |           |                |
>>> |      |                  |   |           |                |
>>> |      |                  |   |           |                |
>>> | +----+--------+         |   |     +--------------+       |
>>> | |             |         |   |     |              |       |
>>> | |  vxlan-red  |         |   |     |  vxlan-blue  |       |
>>> | |             |         |   |     |              |       |
>>> | +------+------+         |   |     +-------+------+       |
>>> |        |                |   |             |              |
>>> |        |           VRF  |   |             |          VRF |
>>> |        |           red  |   |             |         blue |
>>> +-------------------------+   +----------------------------+
>>
>> Roopa and I were discussing this setup and are puzzled by the VRF
>> association here. Does br-red and br-blue have an address? The commands
>> below do not show it and from our perspective seems odd for this
>> scenario. If it does not have an address, then there is no reason for
>> the VRF labeling.
> 
> Yes, br-red and br-blue have an address. Addresses (both MAC and IP)
> are anycast addresses, to make an anycast gateway for the VMs behind
> the taps. This serves for a classical evpn symmetric distributed
> routing, both between vxlans and towards external networks. I am using
> FRR as a control plane to exchange bgp-evpn information between
> hypervisors.
> 
> The schematic could have been more complete, including more bridges
> and more taps in the vrf, with an added bridge+vxlan for the l3vni.
> I did not include them as I wanted to focus on the underlay itself
> and not on what vxlan was used for.
> 
> Here is a more complete example:
>                                    +---------+
>                                    |         |
>                                    | vrf1000 |
>                                    |         |
>                                    +--+-+-+--+
>                                       | | |
>                 +---------------------+ | +---------------+
>                 |                       |                 |
>          +------+------+         +------+------+     +----+---+
>          |             |         |             |     |        |
>          |     br0     |         |     br1     |     | br1000 |
>          | 10.0.0.1/24 |         | 10.0.1.1/24 |     |        |
>          |             |         |             |     +----+---+
>          +----+-+-+----+         +-----+-+-----+          |
>               | | |                    | |                |
>       +-------+ | +-------+         +--+ +---+            |
>       |         |         |         |        |            |
>    +--+---+ +---+--+ +----+---+ +---+--+ +---+----+ +-----+-----+
>    |      | |      | |        | |      | |        | |           |
>    | tap0 | | tap1 | | vxlan0 | | tap2 | | vxlan1 | | vxlan1000 |
>    |      | |      | |        | |      | |        | |           |
>    +------+ +------+ +--------+ +------+ +--------+ +-----------+
>                           .                   .           .
> . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
> .
> .                    +---------+                  +--------------+
> .                    |         |                  |              |
> .                    | vrf1001 |                  | vrf-underlay |
> .                    |         |                  |              |
> .                    +---+-+---+                  +-------+------+
> .                        | |                              |
> .              +---------+ +----------+                   |
> .              |                      |                   |
> .       +------+------+          +----+---+               |
> .       |             |          |        |               |
> .       |     br2     |          | br1001 |               |
> .       | 10.0.2.1/24 |          |        |               |
> .       |             |          +----+---+               |
> .       +----+-+-+----+               |                   |
> .            | | |                    |                   |
> .      +-----+ | +-------+            |                   |
> .      |       |         |            |           +-------+-------+
> .  +---+--+ +--+---+ +---+----+ +-----+-----+     |               |
> .  |      | |      | |        | |           |     |   eth0.2030   |
> .  | tap3 | | tap4 | | vxlan2 | | vxlan1001 |     | 172.16.0.2/24 |
> .  |      | |      | |        | |           |     |               |
> .  +------+ +------+ +--------+ +-----------+     +---------------+
> .                         .           .                 ^  .
> . . . . . . . . . . . . . . . . . . . . . . . . . . . . .  .
>            (vxlan lower device is eth0.2030)               v
>                                                  +----------------+
>                                                  |                |
>                                                  |      eth0      |
>                                                  | 192.168.1.2/24 |
>                                                  |                |
>                                                  +----------------+
> 
> In this example, the underlay is rather simple. A more complex case
> could be with several uplinks in the underlay vrf, and a dummy/loopback
> with a /32 ip, used as a vxlan lower device and to bind routing daemons
> (e.g. bgpd).
> 
>                   +--------------+
>                   |              |
>                   | vrf-underlay |
>                   |              |
>                   +-----+-+-+----+
>                         | | |
>         +---------------+ | +---------------+
>         |                 |                 |
> +-------+-------+ +-------+-------+ +-------+--------+
> |               | |               | |                |
> |     eth1      | |     eth2      | |     dummy0     |
> | 172.16.0.2/31 | | 172.16.0.4/31 | | 192.168.0.1/32 |
> |               | |               | |                |
> +---------------+ +---------------+ +----------------+
>                                             .
> . . . . . . . . . . . . . . . . . . . . . . .
>         (vxlan lower device is dummy0)

Thanks for sending a more complete example. I forwarded it to some FRR
folks as well.

> 
> As for default vrf, it would contain for example management traffic. We
> don’t want to mix the underlay routing tables to other traffics.

Management VRF? :-)


> 
>> Also, it would be good to have a unit test this case. Can you create a
>> shell script that creates the setup and runs a few tests verifying
>> connectivity? You can use network namespaces and veth pairs in place of
>> the VM with a tap device. From there the functionality is the same.
>> Tests can be initial VRF association for the vxlan lower device,
>> changing the VRF to another device, and then changing again back to
>> default VRF - checking proper connectivity for each.
> 
> Sure! I’ve just finished writing it, but I am not sure of the best way
> so send it. I guess I will have to add it to a v4 of the patches, in
> tools/testing/selftests/net?

yes, that would be good. Thank you for writing it.

^ permalink raw reply

* Re: [RFC v3 3/3] vxlan: handle underlay VRF changes
From: David Ahern @ 2018-11-21 19:28 UTC (permalink / raw)
  To: Alexis Bauvin; +Cc: Roopa Prabhu, netdev, akherbouche
In-Reply-To: <28ECBD08-90F5-4FB6-A6AB-E5BB32C0037E@scaleway.com>

On 11/21/18 7:05 AM, Alexis Bauvin wrote:
> Le 20 nov. 2018 à 18:09, David Ahern <dsa@cumulusnetworks.com> a écrit :
>> On 11/20/18 9:58 AM, Alexis Bauvin wrote:
>>> A socket bound to vrf-blue listens on *:4789, thus owning the port. If moving an
>>> underlay to the default vrf (ip link set dummy-b nomaster), a new socket will be
>>> created, unbound to any interface and listening on *:4789. However, because it
>>> will be in the default vrf, it will try to take ownership of port 4789 on ALL
>>> vrfs, and fail because this port is already owned in vrf-blue for vxlan-a.
>>
>> SO_REUSEPORT will fix that and incoming traffic through a vrf and
>> default (non-)vrf will work. The recent changes by Vyatta provide even
>> better isolation of default vrf and overlapping ports.
> 
> Did not think about that one, I will give it a shot.
> 
> There is one issue I can see with SO_REUSEPORT (if my understanding of it is
> correct). From what I understood, enabling this option will balance incoming
> connections (for TCP) / dgrams (for UDP) based on a 4-tuple hash (sip, dip,
> sport, dport) between sockets listening on the same port.

AFAIK there is no balancing done. There is an order to which socket is
selected - and it includes the VRF device if relevant.

> 
> If we have two separate vxlan fabrics, with one underlay in the default vrf, and
> another in some random vrf. Since the socket for the default vrf would own the
> port on all vrfs, the port would effectively be reused between the two vrfs.
> Wouldn't vxlan packets be directed to "random" (as in not related to the vxlan
> fabric itself) sockets, meaning a complete mix of the two fabrics? This would
> imply a complete drop of the packets not directed to the correct socket.
> 
> I guess the Vyatta changes you are talking about are "vrf: allow simultaneous
> service instances in default and other VRFs"? If so, it looks like it would
> solve the default vrf problem, not even requiring SO_REUSEPORT.
> 

yes.

^ permalink raw reply

* [PATCH v1 net] lan743x: Enable driver to work with LAN7431
From: Bryan Whitehead @ 2018-11-21 19:22 UTC (permalink / raw)
  To: davem; +Cc: netdev, UNGLinuxDriver

This driver was designed to work with both LAN7430 and LAN7431.
The only difference between the two is the LAN7431 has support
for external phy.

This change adds LAN7431 to the list of recognized devices
supported by this driver.

fixes: driver won't load for LAN7431
Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>
---
 drivers/net/ethernet/microchip/lan743x_main.c | 1 +
 drivers/net/ethernet/microchip/lan743x_main.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 867cddb..e2f1531 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -3017,6 +3017,7 @@ static const struct dev_pm_ops lan743x_pm_ops = {
 
 static const struct pci_device_id lan743x_pcidev_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_SMSC, PCI_DEVICE_ID_SMSC_LAN7430) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_SMSC, PCI_DEVICE_ID_SMSC_LAN7431) },
 	{ 0, }
 };
 
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index 0e82b63..2d6eea1 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -548,6 +548,7 @@ struct lan743x_adapter;
 /* SMSC acquired EFAR late 1990's, MCHP acquired SMSC 2012 */
 #define PCI_VENDOR_ID_SMSC		PCI_VENDOR_ID_EFAR
 #define PCI_DEVICE_ID_SMSC_LAN7430	(0x7430)
+#define PCI_DEVICE_ID_SMSC_LAN7431	(0x7431)
 
 #define PCI_CONFIG_LENGTH		(0x1000)
 
-- 
2.7.4

^ permalink raw reply related

* Re: [Patch net-next 2/2] net: dump whole skb data in netdev_rx_csum_fault()
From: Saeed Mahameed @ 2018-11-21 19:33 UTC (permalink / raw)
  To: xiyou.wangcong@gmail.com, edumazet@google.com
  Cc: eric.dumazet@gmail.com, netdev@vger.kernel.org,
	davem@davemloft.net, herbert@gondor.apana.org.au
In-Reply-To: <CANn89iJV1_Z4zxBbLM-sG+h=-m8s2Oaq0T90Ew6HRy+5yOAOEw@mail.gmail.com>

On Wed, 2018-11-21 at 10:26 -0800, Eric Dumazet wrote:
> On Wed, Nov 21, 2018 at 10:17 AM Cong Wang <xiyou.wangcong@gmail.com>
> wrote:
> > On Wed, Nov 21, 2018 at 5:05 AM Eric Dumazet <
> > eric.dumazet@gmail.com> wrote:
> > > 
> > > 
> > > On 11/20/2018 06:13 PM, Cong Wang wrote:
> > > > Currently, we only dump a few selected skb fields in
> > > > netdev_rx_csum_fault(). It is not suffient for debugging
> > > > checksum
> > > > fault. This patch introduces skb_dump() which dumps skb mac
> > > > header,
> > > > network header and its whole skb->data too.
> > > > 
> > > > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > > > Cc: Eric Dumazet <edumazet@google.com>
> > > > Cc: David Miller <davem@davemloft.net>
> > > > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> > > > ---
> > > > +     print_hex_dump(level, "skb data: ", DUMP_PREFIX_OFFSET,
> > > > 16, 1,
> > > > +                    skb->data, skb->len, false);
> > > 
> > > As I mentioned to David, we want all the bytes that were maybe
> > > already pulled
> > > 
> > > (skb->head starting point, not skb->data)
> > 
> > Hmm, with mac header and network header, it is effectively from
> > skb->head, no?
> > Is there anything between skb->head and mac header?
> 
> Oh, I guess we wanted a single hex dump, or we need some user program
> to be able to
> rebuild from different memory zones the original CHECKSUM_COMPLETE
> value.
> 

Normally the driver keeps some headroom @skb->head, so the actual mac
header starts @ skb->head + driver_specific_headroom

for example in mlx5 we do:
va = page_addr + offset
build_skb(va)
/* skb->data = va + headroom */
skb_reserve(mlx5_headroom)


> > > Also we will miss the trimmed bytes if there were padding data.
> > > And it seems the various bugs we have are all tied to the pulled
> > > or trimmed bytes.
> > > 
> > 
> > Unless I miss something, the tailing padding data should be in
> > range
> > [iphdr->tot_len, skb->len]. No?
> 
> Not after we did the pskb_trim_rcsum() call, since it has effectively
> reduced skb->len by the number of padding bytes.

^ permalink raw reply

* [PATCH net-next] net-gro: use ffs() to speedup napi_gro_flush()
From: Eric Dumazet @ 2018-11-21 19:39 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet

We very often have few flows/chains to look at, and we
might increase GRO_HASH_BUCKETS to 32 or 64 in the future.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/dev.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index f2bfd2eda7b2734d29d30f0e82c1a48c1b5b166a..d83582623cd74523033a4798218f8da4760b7a09 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5364,11 +5364,13 @@ static void __napi_gro_flush_chain(struct napi_struct *napi, u32 index,
  */
 void napi_gro_flush(struct napi_struct *napi, bool flush_old)
 {
-	u32 i;
+	unsigned long bitmask = napi->gro_bitmask;
+	unsigned int i, base = ~0U;
 
-	for (i = 0; i < GRO_HASH_BUCKETS; i++) {
-		if (test_bit(i, &napi->gro_bitmask))
-			__napi_gro_flush_chain(napi, i, flush_old);
+	while ((i = ffs(bitmask)) != 0) {
+		bitmask >>= i;
+		base += i;
+		__napi_gro_flush_chain(napi, base, flush_old);
 	}
 }
 EXPORT_SYMBOL(napi_gro_flush);
-- 
2.19.1.1215.g8438c0b245-goog

^ permalink raw reply related

* [net-next 0/6][pull request] Intel Wired LAN Driver Updates 2018-11-21
From: Jeff Kirsher @ 2018-11-21 19:54 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jesse.brandeburg

This series contains updates to all of the Intel LAN drivers and
documentation.

Shannon Nelson updates the ixgbe kernel documentation to include IPsec
hardware offload.

Joe Perches cleans up whitespace issues in the igb driver.

Jesse update the netdev kernel documentation for NETIF_F_GSO_UDP_L4 to
align with the actual code.  Also aligned all the NAPI driver code for
all of the Intel drivers to implement the recommendations of Eric
Dumazet to check the return code of the napi_complete_done() to
determine whether or not to enable interrupts or exit poll.

Paul E. McKenney replaces synchronize_sched() with synchronize_rcu() for
ixgbe.

Sasha implements suggestions made by Joe Perches to remove obsolete code
and to use the dev_err() method.

The following are changes since commit 11c6c0c22857967815b65cc7692181866549a06f:
  Merge branch '100GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 1GbE

Jesse Brandeburg (2):
  docs-networking: fix typo in define
  ethernet/intel: consolidate NAPI and NAPI exit

Joe Perches (1):
  igb: Fix format with line continuation whitespace

Paul E. McKenney (1):
  ixgbe: Replace synchronize_sched() with synchronize_rcu()

Sasha Neftin (1):
  igc: Remove obsolete IGC_ERR define

Shannon Nelson (1):
  ixgbe: add ipsec hw offload note to ixgbe Documentation

 Documentation/networking/ixgbe.rst            | 13 +++++++++++
 Documentation/networking/netdev-features.txt  |  2 +-
 drivers/net/ethernet/intel/e100.c             | 10 ++++----
 drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +++++----
 drivers/net/ethernet/intel/e1000e/netdev.c    | 17 +++++++-------
 drivers/net/ethernet/intel/fm10k/fm10k_main.c | 10 ++++----
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |  9 ++++----
 drivers/net/ethernet/intel/iavf/iavf_txrx.c   |  9 ++++----
 drivers/net/ethernet/intel/ice/ice_txrx.c     | 10 ++++----
 drivers/net/ethernet/intel/igb/igb_main.c     | 23 ++++++++++---------
 drivers/net/ethernet/intel/igbvf/netdev.c     |  9 +++++---
 drivers/net/ethernet/intel/igc/igc.h          |  2 --
 drivers/net/ethernet/intel/igc/igc_main.c     | 12 ++++++----
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  6 ++---
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 ++++++++++--------
 15 files changed, 97 insertions(+), 68 deletions(-)

-- 
2.19.1

^ permalink raw reply

* [net-next 1/6] ixgbe: add ipsec hw offload note to ixgbe Documentation
From: Jeff Kirsher @ 2018-11-21 19:54 UTC (permalink / raw)
  To: davem; +Cc: Shannon Nelson, netdev, nhorman, sassmann, Jeff Kirsher
In-Reply-To: <20181121195423.20509-1-jeffrey.t.kirsher@intel.com>

From: Shannon Nelson <shannon.nelson@oracle.com>

Add a short note about using IPsec Hardware Offload with
the ixgbe driver.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 Documentation/networking/ixgbe.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/networking/ixgbe.rst b/Documentation/networking/ixgbe.rst
index 725fc697fd8f..86d887a63606 100644
--- a/Documentation/networking/ixgbe.rst
+++ b/Documentation/networking/ixgbe.rst
@@ -501,6 +501,19 @@ NOTE: This feature can be disabled for a specific Virtual Function (VF)::
 
   ip link set <pf dev> vf <vf id> spoofchk {off|on}
 
+IPsec Offload
+-------------
+The ixgbe driver supports IPsec Hardware Offload.  When creating Security
+Associations with "ip xfrm ..." the 'offload' tag option can be used to
+register the IPsec SA with the driver in order to get higher throughput in
+the secure communications.
+
+The offload is also supported for ixgbe's VFs, but the VF must be set as
+'trusted' and the support must be enabled with::
+
+  ethtool --set-priv-flags eth<x> vf-ipsec on
+  ip link set eth<x> vf <y> trust on
+
 
 Known Issues/Troubleshooting
 ============================
-- 
2.19.1

^ permalink raw reply related

* [net-next 2/6] igb: Fix format with line continuation whitespace
From: Jeff Kirsher @ 2018-11-21 19:54 UTC (permalink / raw)
  To: davem; +Cc: Joe Perches, netdev, nhorman, sassmann, Jeff Kirsher
In-Reply-To: <20181121195423.20509-1-jeffrey.t.kirsher@intel.com>

From: Joe Perches <joe@perches.com>

The line continuation unintentionally adds whitespace so
instead use a coalesced format to remove the whitespace.

Miscellanea:

o Use a more typical style for ternaries and arguments
  for this logging message

Signed-off-by: Joe Perches <joe@perches.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 4584ebc9e8fe..4331ea71b86e 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1850,13 +1850,12 @@ static void igb_config_tx_modes(struct igb_adapter *adapter, int queue)
 	 * configuration' in respect to these parameters.
 	 */
 
-	netdev_dbg(netdev, "Qav Tx mode: cbs %s, launchtime %s, queue %d \
-			    idleslope %d sendslope %d hiCredit %d \
-			    locredit %d\n",
-		   (ring->cbs_enable) ? "enabled" : "disabled",
-		   (ring->launchtime_enable) ? "enabled" : "disabled", queue,
-		   ring->idleslope, ring->sendslope, ring->hicredit,
-		   ring->locredit);
+	netdev_dbg(netdev, "Qav Tx mode: cbs %s, launchtime %s, queue %d idleslope %d sendslope %d hiCredit %d locredit %d\n",
+		   ring->cbs_enable ? "enabled" : "disabled",
+		   ring->launchtime_enable ? "enabled" : "disabled",
+		   queue,
+		   ring->idleslope, ring->sendslope,
+		   ring->hicredit, ring->locredit);
 }
 
 static int igb_save_txtime_params(struct igb_adapter *adapter, int queue,
-- 
2.19.1

^ permalink raw reply related

* [net-next 5/6] ixgbe: Replace synchronize_sched() with synchronize_rcu()
From: Jeff Kirsher @ 2018-11-21 19:54 UTC (permalink / raw)
  To: davem; +Cc: Paul E. McKenney, netdev, nhorman, sassmann, Jeff Kirsher
In-Reply-To: <20181121195423.20509-1-jeffrey.t.kirsher@intel.com>

From: "Paul E. McKenney" <paulmck@linux.ibm.com>

Now that synchronize_rcu() waits for preempt-disable regions of code
as well as RCU read-side critical sections, synchronize_sched() can be
replaced by synchronize_rcu().  This commit therefore makes this change.

Signed-off-by: "Paul E. McKenney" <paulmck@linux.ibm.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index cfb83687c3d8..49a4ea38eb07 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6077,9 +6077,9 @@ void ixgbe_down(struct ixgbe_adapter *adapter)
 	/* Disable Rx */
 	ixgbe_disable_rx(adapter);
 
-	/* synchronize_sched() needed for pending XDP buffers to drain */
+	/* synchronize_rcu() needed for pending XDP buffers to drain */
 	if (adapter->xdp_ring[0])
-		synchronize_sched();
+		synchronize_rcu();
 
 	ixgbe_irq_disable(adapter);
 
@@ -10476,7 +10476,7 @@ void ixgbe_txrx_ring_disable(struct ixgbe_adapter *adapter, int ring)
 	ixgbe_disable_rxr_hw(adapter, rx_ring);
 
 	if (xdp_ring)
-		synchronize_sched();
+		synchronize_rcu();
 
 	/* Rx/Tx/XDP Tx share the same napi context. */
 	napi_disable(&rx_ring->q_vector->napi);
-- 
2.19.1

^ permalink raw reply related

* [net-next 3/6] docs-networking: fix typo in define
From: Jeff Kirsher @ 2018-11-21 19:54 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, nhorman, sassmann, Jeff Kirsher
In-Reply-To: <20181121195423.20509-1-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

The #define for NETIF_F_GSO_UDP_L4 was incorrect in the
documentation, fix it by making it match the actual code.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 Documentation/networking/netdev-features.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/networking/netdev-features.txt b/Documentation/networking/netdev-features.txt
index c4a54c162547..58dd1c1e3c65 100644
--- a/Documentation/networking/netdev-features.txt
+++ b/Documentation/networking/netdev-features.txt
@@ -115,7 +115,7 @@ set, be it TCPv4 (when NETIF_F_TSO is enabled) or TCPv6 (NETIF_F_TSO6).
 
  * Transmit UDP segmentation offload
 
-NETIF_F_GSO_UDP_GSO_L4 accepts a single UDP header with a payload that exceeds
+NETIF_F_GSO_UDP_L4 accepts a single UDP header with a payload that exceeds
 gso_size. On segmentation, it segments the payload on gso_size boundaries and
 replicates the network and UDP headers (fixing up the last one if less than
 gso_size).
-- 
2.19.1

^ permalink raw reply related

* [net-next 4/6] ethernet/intel: consolidate NAPI and NAPI exit
From: Jeff Kirsher @ 2018-11-21 19:54 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, nhorman, sassmann, Jeff Kirsher
In-Reply-To: <20181121195423.20509-1-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

While reviewing code, I noticed that Eric Dumazet recommends that
drivers check the return code of napi_complete_done, and use that
to decide to enable interrupts or not when exiting poll.  One of
the Intel drivers was already fixed (ixgbe).

Upon looking at the Intel drivers as a whole, we are handling our
polling and NAPI exit in a few different ways based on whether we
have multiqueue and whether we have Tx cleanup included. Several
drivers had the bug of exiting NAPI with return 0, which appears
to mess up the accounting in the stack.

Consolidate all the NAPI routines to do best known way of exiting
and to just mostly look like each other.
1) check return code of napi_complete_done to control interrupt enable
2) return the actual amount of work done.
3) return budget immediately if need NAPI poll again

Tested the changes on e1000e with a high interrupt rate set, and
it shows about an 8% reduction in the CPU utilization when busy
polling because we aren't re-enabling interrupts when we're about
to be polled.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e100.c             | 10 +++++----
 drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +++++-----
 drivers/net/ethernet/intel/e1000e/netdev.c    | 17 +++++++-------
 drivers/net/ethernet/intel/fm10k/fm10k_main.c | 10 ++++-----
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |  9 ++++----
 drivers/net/ethernet/intel/iavf/iavf_txrx.c   |  9 ++++----
 drivers/net/ethernet/intel/ice/ice_txrx.c     | 10 +++++----
 drivers/net/ethernet/intel/igb/igb_main.c     | 10 +++++----
 drivers/net/ethernet/intel/igbvf/netdev.c     |  9 +++++---
 drivers/net/ethernet/intel/igc/igc_main.c     | 10 +++++----
 .../net/ethernet/intel/ixgbevf/ixgbevf_main.c | 22 +++++++++++--------
 11 files changed, 73 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 7c4b55482f72..5e5c57db0d3f 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -2225,11 +2225,13 @@ static int e100_poll(struct napi_struct *napi, int budget)
 	e100_rx_clean(nic, &work_done, budget);
 	e100_tx_clean(nic);
 
-	/* If budget not fully consumed, exit the polling mode */
-	if (work_done < budget) {
-		napi_complete_done(napi, work_done);
+	/* If budget fully consumed, continue polling */
+	if (work_done == budget)
+		return budget;
+
+	/* only re-enable interrupt if stack agrees polling is really done */
+	if (likely(napi_complete_done(napi, work_done)))
 		e100_enable_irq(nic);
-	}
 
 	return work_done;
 }
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 43b6d3cec3b3..8fe9af0e2ab7 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -3803,14 +3803,15 @@ static int e1000_clean(struct napi_struct *napi, int budget)
 
 	adapter->clean_rx(adapter, &adapter->rx_ring[0], &work_done, budget);
 
-	if (!tx_clean_complete)
-		work_done = budget;
+	if (!tx_clean_complete || work_done == budget)
+		return budget;
 
-	/* If budget not fully consumed, exit the polling mode */
-	if (work_done < budget) {
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done))) {
 		if (likely(adapter->itr_setting & 3))
 			e1000_set_itr(adapter);
-		napi_complete_done(napi, work_done);
 		if (!test_bit(__E1000_DOWN, &adapter->flags))
 			e1000_irq_enable(adapter);
 	}
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 59bd587d809d..308c006cb41d 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -2651,9 +2651,9 @@ static int e1000_alloc_queues(struct e1000_adapter *adapter)
 /**
  * e1000e_poll - NAPI Rx polling callback
  * @napi: struct associated with this polling callback
- * @weight: number of packets driver is allowed to process this poll
+ * @budget: number of packets driver is allowed to process this poll
  **/
-static int e1000e_poll(struct napi_struct *napi, int weight)
+static int e1000e_poll(struct napi_struct *napi, int budget)
 {
 	struct e1000_adapter *adapter = container_of(napi, struct e1000_adapter,
 						     napi);
@@ -2667,16 +2667,17 @@ static int e1000e_poll(struct napi_struct *napi, int weight)
 	    (adapter->rx_ring->ims_val & adapter->tx_ring->ims_val))
 		tx_cleaned = e1000_clean_tx_irq(adapter->tx_ring);
 
-	adapter->clean_rx(adapter->rx_ring, &work_done, weight);
+	adapter->clean_rx(adapter->rx_ring, &work_done, budget);
 
-	if (!tx_cleaned)
-		work_done = weight;
+	if (!tx_cleaned || work_done == budget)
+		return budget;
 
-	/* If weight not fully consumed, exit the polling mode */
-	if (work_done < weight) {
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done))) {
 		if (adapter->itr_setting & 3)
 			e1000_set_itr(adapter);
-		napi_complete_done(napi, work_done);
 		if (!test_bit(__E1000_DOWN, &adapter->state)) {
 			if (adapter->msix_entries)
 				ew32(IMS, adapter->rx_ring->ims_val);
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 5b2a50e5798f..6fd15a734324 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -1465,11 +1465,11 @@ static int fm10k_poll(struct napi_struct *napi, int budget)
 	if (!clean_complete)
 		return budget;
 
-	/* all work done, exit the polling mode */
-	napi_complete_done(napi, work_done);
-
-	/* re-enable the q_vector */
-	fm10k_qv_enable(q_vector);
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done)))
+		fm10k_qv_enable(q_vector);
 
 	return min(work_done, budget - 1);
 }
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index c4d44096cdaf..a0b1575468fc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2667,10 +2667,11 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
 	if (vsi->back->flags & I40E_TXR_FLAGS_WB_ON_ITR)
 		q_vector->arm_wb_state = false;
 
-	/* Work is done so exit the polling mode and re-enable the interrupt */
-	napi_complete_done(napi, work_done);
-
-	i40e_update_enable_itr(vsi, q_vector);
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done)))
+		i40e_update_enable_itr(vsi, q_vector);
 
 	return min(work_done, budget - 1);
 }
diff --git a/drivers/net/ethernet/intel/iavf/iavf_txrx.c b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
index 3b1dc77ae368..9b4d7cec2e18 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_txrx.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_txrx.c
@@ -1761,10 +1761,11 @@ int iavf_napi_poll(struct napi_struct *napi, int budget)
 	if (vsi->back->flags & IAVF_TXR_FLAGS_WB_ON_ITR)
 		q_vector->arm_wb_state = false;
 
-	/* Work is done so exit the polling mode and re-enable the interrupt */
-	napi_complete_done(napi, work_done);
-
-	iavf_update_enable_itr(vsi, q_vector);
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done)))
+		iavf_update_enable_itr(vsi, q_vector);
 
 	return min(work_done, budget - 1);
 }
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index 4b92863b3500..49fc38094185 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -1103,10 +1103,12 @@ int ice_napi_poll(struct napi_struct *napi, int budget)
 	if (!clean_complete)
 		return budget;
 
-	/* Work is done so exit the polling mode and re-enable the interrupt */
-	napi_complete_done(napi, work_done);
-	if (test_bit(ICE_FLAG_MSIX_ENA, pf->flags))
-		ice_irq_dynamic_ena(&vsi->back->hw, vsi, q_vector);
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done)))
+		if (test_bit(ICE_FLAG_MSIX_ENA, pf->flags))
+			ice_irq_dynamic_ena(&vsi->back->hw, vsi, q_vector);
 
 	return min(work_done, budget - 1);
 }
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 4331ea71b86e..453ae1d9e5f3 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7752,11 +7752,13 @@ static int igb_poll(struct napi_struct *napi, int budget)
 	if (!clean_complete)
 		return budget;
 
-	/* If not enough Rx work done, exit the polling mode */
-	napi_complete_done(napi, work_done);
-	igb_ring_irq_enable(q_vector);
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done)))
+		igb_ring_irq_enable(q_vector);
 
-	return 0;
+	return min(work_done, budget - 1);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index 820d49eb41ab..4eab83faec62 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -1186,10 +1186,13 @@ static int igbvf_poll(struct napi_struct *napi, int budget)
 
 	igbvf_clean_rx_irq(adapter, &work_done, budget);
 
-	/* If not enough Rx work done, exit the polling mode */
-	if (work_done < budget) {
-		napi_complete_done(napi, work_done);
+	if (work_done == budget)
+		return budget;
 
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done))) {
 		if (adapter->requested_itr & 3)
 			igbvf_set_itr(adapter);
 
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index d002055c0623..28ffe98f8921 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -2852,11 +2852,13 @@ static int igc_poll(struct napi_struct *napi, int budget)
 	if (!clean_complete)
 		return budget;
 
-	/* If not enough Rx work done, exit the polling mode */
-	napi_complete_done(napi, work_done);
-	igc_ring_irq_enable(q_vector);
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done)))
+		igc_ring_irq_enable(q_vector);
 
-	return 0;
+	return min(work_done, budget - 1);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 196b890467b2..2de81f046fb5 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1293,16 +1293,20 @@ static int ixgbevf_poll(struct napi_struct *napi, int budget)
 	/* If all work not completed, return budget and keep polling */
 	if (!clean_complete)
 		return budget;
-	/* all work done, exit the polling mode */
-	napi_complete_done(napi, work_done);
-	if (adapter->rx_itr_setting == 1)
-		ixgbevf_set_itr(q_vector);
-	if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
-	    !test_bit(__IXGBEVF_REMOVING, &adapter->state))
-		ixgbevf_irq_enable_queues(adapter,
-					  BIT(q_vector->v_idx));
 
-	return 0;
+	/* Exit the polling mode, but don't re-enable interrupts if stack might
+	 * poll us due to busy-polling
+	 */
+	if (likely(napi_complete_done(napi, work_done))) {
+		if (adapter->rx_itr_setting == 1)
+			ixgbevf_set_itr(q_vector);
+		if (!test_bit(__IXGBEVF_DOWN, &adapter->state) &&
+		    !test_bit(__IXGBEVF_REMOVING, &adapter->state))
+			ixgbevf_irq_enable_queues(adapter,
+						  BIT(q_vector->v_idx));
+	}
+
+	return min(work_done, budget - 1);
 }
 
 /**
-- 
2.19.1

^ permalink raw reply related

* [net-next 6/6] igc: Remove obsolete IGC_ERR define
From: Jeff Kirsher @ 2018-11-21 19:54 UTC (permalink / raw)
  To: davem; +Cc: Sasha Neftin, netdev, nhorman, sassmann, Jeff Kirsher
In-Reply-To: <20181121195423.20509-1-jeffrey.t.kirsher@intel.com>

From: Sasha Neftin <sasha.neftin@intel.com>

Address community comment.
Remove obsolete IGC_ERR define and use dev_err method.
Suggested by Joe Perches.

Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h      | 2 --
 drivers/net/ethernet/intel/igc/igc_main.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 3b00b109b34a..b1039dd3dd13 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -11,8 +11,6 @@
 #include <linux/ethtool.h>
 #include <linux/sctp.h>
 
-#define IGC_ERR(args...) pr_err("igc: " args)
-
 #include "igc_hw.h"
 
 /* main */
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 28ffe98f8921..f20183037fb2 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -3535,7 +3535,7 @@ static int igc_probe(struct pci_dev *pdev,
 			err = dma_set_coherent_mask(&pdev->dev,
 						    DMA_BIT_MASK(32));
 			if (err) {
-				IGC_ERR("Wrong DMA configuration, aborting\n");
+				dev_err(&pdev->dev, "igc: Wrong DMA config\n");
 				goto err_dma;
 			}
 		}
-- 
2.19.1

^ permalink raw reply related

* [PATCH perf,bpf 2/5] perf: sync tools/include/uapi/linux/perf_event.h
From: Song Liu @ 2018-11-21 19:54 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: Song Liu, ast, daniel, acme, peterz, kernel-team
In-Reply-To: <20181121195502.3259930-1-songliubraving@fb.com>

Sync changes for PERF_RECORD_BPF_EVENT.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/include/uapi/linux/perf_event.h | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index f35eb72739c0..72a7da2b713f 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -372,7 +372,8 @@ struct perf_event_attr {
 				context_switch :  1, /* context switch data */
 				write_backward :  1, /* Write ring buffer from end to beginning */
 				namespaces     :  1, /* include namespaces data */
-				__reserved_1   : 35;
+				bpf_event      :  1, /* include bpf events */
+				__reserved_1   : 34;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
@@ -963,9 +964,33 @@ enum perf_event_type {
 	 */
 	PERF_RECORD_NAMESPACES			= 16,
 
+	/*
+	 * Record different types of bpf events:
+	 *  enum perf_bpf_event_type {
+	 *     PERF_BPF_EVENT_UNKNOWN		= 0,
+	 *     PERF_BPF_EVENT_PROG_LOAD		= 1,
+	 *     PERF_BPF_EVENT_PROG_UNLOAD	= 2,
+	 *  };
+	 *
+	 * struct {
+	 *	struct perf_event_header header;
+	 *	u16 type;
+	 *	u16 flags;
+	 *	u32 id;  // prog_id or map_id
+	 * };
+	 */
+	PERF_RECORD_BPF_EVENT			= 17,
+
 	PERF_RECORD_MAX,			/* non-ABI */
 };
 
+enum perf_bpf_event_type {
+	PERF_BPF_EVENT_UNKNOWN		= 0,
+	PERF_BPF_EVENT_PROG_LOAD	= 1,
+	PERF_BPF_EVENT_PROG_UNLOAD	= 2,
+	PERF_BPF_EVENT_MAX,		/* non-ABI */
+};
+
 #define PERF_MAX_STACK_DEPTH		127
 #define PERF_MAX_CONTEXTS_PER_STACK	  8
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH perf,bpf 3/5] perf util: basic handling of PERF_RECORD_BPF_EVENT
From: Song Liu @ 2018-11-21 19:55 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: Song Liu, ast, daniel, acme, peterz, kernel-team
In-Reply-To: <20181121195502.3259930-1-songliubraving@fb.com>

This patch adds basic handling of PERF_RECORD_BPF_EVENT in perf util.
Following patches add more logic that leverages these events.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/perf/util/event.c   | 20 ++++++++++++++++++++
 tools/perf/util/event.h   | 15 +++++++++++++++
 tools/perf/util/evsel.c   |  1 +
 tools/perf/util/machine.c | 10 ++++++++++
 tools/perf/util/machine.h |  2 ++
 tools/perf/util/session.c |  4 ++++
 tools/perf/util/tool.h    |  4 +++-
 7 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 0cd42150f712..04104a4ffe93 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -24,6 +24,7 @@
 #include "symbol/kallsyms.h"
 #include "asm/bug.h"
 #include "stat.h"
+#include "session.h"
 
 static const char *perf_event__names[] = {
 	[0]					= "TOTAL",
@@ -43,6 +44,7 @@ static const char *perf_event__names[] = {
 	[PERF_RECORD_SWITCH]			= "SWITCH",
 	[PERF_RECORD_SWITCH_CPU_WIDE]		= "SWITCH_CPU_WIDE",
 	[PERF_RECORD_NAMESPACES]		= "NAMESPACES",
+	[PERF_RECORD_BPF_EVENT]			= "BPF_EVENT",
 	[PERF_RECORD_HEADER_ATTR]		= "ATTR",
 	[PERF_RECORD_HEADER_EVENT_TYPE]		= "EVENT_TYPE",
 	[PERF_RECORD_HEADER_TRACING_DATA]	= "TRACING_DATA",
@@ -1333,6 +1335,14 @@ int perf_event__process_switch(struct perf_tool *tool __maybe_unused,
 	return machine__process_switch_event(machine, event);
 }
 
+int perf_event__process_bpf_event(struct perf_tool *tool __maybe_unused,
+				  union perf_event *event,
+				  struct perf_sample *sample __maybe_unused,
+				  struct machine *machine)
+{
+	return machine__process_bpf_event(machine, event);
+}
+
 size_t perf_event__fprintf_mmap(union perf_event *event, FILE *fp)
 {
 	return fprintf(fp, " %d/%d: [%#" PRIx64 "(%#" PRIx64 ") @ %#" PRIx64 "]: %c %s\n",
@@ -1465,6 +1475,13 @@ static size_t perf_event__fprintf_lost(union perf_event *event, FILE *fp)
 	return fprintf(fp, " lost %" PRIu64 "\n", event->lost.lost);
 }
 
+size_t perf_event__fprintf_bpf_event(union perf_event *event, FILE *fp)
+{
+	return fprintf(fp, " bpf event with type %u, flags %u, id %u\n",
+		       event->bpf_event.type, event->bpf_event.flags,
+		       event->bpf_event.id);
+}
+
 size_t perf_event__fprintf(union perf_event *event, FILE *fp)
 {
 	size_t ret = fprintf(fp, "PERF_RECORD_%s",
@@ -1500,6 +1517,9 @@ size_t perf_event__fprintf(union perf_event *event, FILE *fp)
 	case PERF_RECORD_LOST:
 		ret += perf_event__fprintf_lost(event, fp);
 		break;
+	case PERF_RECORD_BPF_EVENT:
+		ret += perf_event__fprintf_bpf_event(event, fp);
+		break;
 	default:
 		ret += fprintf(fp, "\n");
 	}
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index bfa60bcafbde..13a0c64dd0ed 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -84,6 +84,15 @@ struct throttle_event {
 	u64 stream_id;
 };
 
+
+/* Record different types of bpf events, see enum perf_bpf_event_type */
+struct bpf_event {
+	struct perf_event_header header;
+	u16 type;
+	u16 flags;
+	u32 id;
+};
+
 #define PERF_SAMPLE_MASK				\
 	(PERF_SAMPLE_IP | PERF_SAMPLE_TID |		\
 	 PERF_SAMPLE_TIME | PERF_SAMPLE_ADDR |		\
@@ -651,6 +660,7 @@ union perf_event {
 	struct stat_round_event		stat_round;
 	struct time_conv_event		time_conv;
 	struct feature_event		feat;
+	struct bpf_event		bpf_event;
 };
 
 void perf_event__print_totals(void);
@@ -750,6 +760,10 @@ int perf_event__process_exit(struct perf_tool *tool,
 			     union perf_event *event,
 			     struct perf_sample *sample,
 			     struct machine *machine);
+int perf_event__process_bpf_event(struct perf_tool *tool,
+				  union perf_event *event,
+				  struct perf_sample *sample,
+				  struct machine *machine);
 int perf_tool__process_synth_event(struct perf_tool *tool,
 				   union perf_event *event,
 				   struct machine *machine,
@@ -814,6 +828,7 @@ size_t perf_event__fprintf_switch(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf_thread_map(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf_cpu_map(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf_namespaces(union perf_event *event, FILE *fp);
+size_t perf_event__fprintf_bpf_event(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf(union perf_event *event, FILE *fp);
 
 int kallsyms__get_function_start(const char *kallsyms_filename,
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index cb7f01059940..af9d539e4b6a 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1635,6 +1635,7 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
 	PRINT_ATTRf(context_switch, p_unsigned);
 	PRINT_ATTRf(write_backward, p_unsigned);
 	PRINT_ATTRf(namespaces, p_unsigned);
+	PRINT_ATTRf(bpf_event, p_unsigned);
 
 	PRINT_ATTRn("{ wakeup_events, wakeup_watermark }", wakeup_events, p_unsigned);
 	PRINT_ATTRf(bp_type, p_unsigned);
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 111ae858cbcb..74c295f6fdc4 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -681,6 +681,14 @@ int machine__process_switch_event(struct machine *machine __maybe_unused,
 	return 0;
 }
 
+int machine__process_bpf_event(struct machine *machine __maybe_unused,
+			       union perf_event *event)
+{
+	if (dump_trace)
+		perf_event__fprintf_bpf_event(event, stdout);
+	return 0;
+}
+
 static void dso__adjust_kmod_long_name(struct dso *dso, const char *filename)
 {
 	const char *dup_filename;
@@ -1795,6 +1803,8 @@ int machine__process_event(struct machine *machine, union perf_event *event,
 	case PERF_RECORD_SWITCH:
 	case PERF_RECORD_SWITCH_CPU_WIDE:
 		ret = machine__process_switch_event(machine, event); break;
+	case PERF_RECORD_BPF_EVENT:
+		ret = machine__process_bpf_event(machine, event); break;
 	default:
 		ret = -1;
 		break;
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index d856b85862e2..0ed237d6fd0a 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -127,6 +127,8 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event
 				struct perf_sample *sample);
 int machine__process_mmap2_event(struct machine *machine, union perf_event *event,
 				 struct perf_sample *sample);
+int machine__process_bpf_event(struct machine *machine,
+			       union perf_event *event);
 int machine__process_event(struct machine *machine, union perf_event *event,
 				struct perf_sample *sample);
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 7d2c8ce6cfad..dffe5120d2d3 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -371,6 +371,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
 		tool->itrace_start = perf_event__process_itrace_start;
 	if (tool->context_switch == NULL)
 		tool->context_switch = perf_event__process_switch;
+	if (tool->bpf_event == NULL)
+		tool->bpf_event = perf_event__process_bpf_event;
 	if (tool->read == NULL)
 		tool->read = process_event_sample_stub;
 	if (tool->throttle == NULL)
@@ -1300,6 +1302,8 @@ static int machines__deliver_event(struct machines *machines,
 	case PERF_RECORD_SWITCH:
 	case PERF_RECORD_SWITCH_CPU_WIDE:
 		return tool->context_switch(tool, event, sample, machine);
+	case PERF_RECORD_BPF_EVENT:
+		return tool->bpf_event(tool, event, sample, machine);
 	default:
 		++evlist->stats.nr_unknown_events;
 		return -1;
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index 56e4ca54020a..69ae898ca024 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -53,7 +53,9 @@ struct perf_tool {
 			itrace_start,
 			context_switch,
 			throttle,
-			unthrottle;
+			unthrottle,
+			bpf_event;
+
 	event_attr_op	attr;
 	event_attr_op	event_update;
 	event_op2	tracing_data;
-- 
2.17.1

^ permalink raw reply related

* [PATCH perf,bpf 4/5] perf util: introduce bpf_prog_info_event
From: Song Liu @ 2018-11-21 19:55 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: Song Liu, ast, daniel, acme, peterz, kernel-team
In-Reply-To: <20181121195502.3259930-1-songliubraving@fb.com>

This patch introduces struct bpf_prog_info_event to union perf_event.

struct bpf_prog_info_event {
       struct perf_event_header        header;
       u32                             prog_info_len;
       u32                             ksym_table_len;
       u64                             ksym_table;
       struct bpf_prog_info            prog_info;
       char                            data[];
};

struct bpf_prog_info_event contains information about a bpf program.
These events are written to perf.data by perf-record, and processed by
perf-report.

struct bpf_prog_info_event uses arrays for some data (ksym_table, and
arrays in struct bpf_prog_info). To make these arrays easy to serialize,
we allocate continuous memory (data). These array pointers are translated
to offset in bpf_prog_info_event before written to file. And vice-versa
when the event is read from file.

This patch enables synthesizing these events at the beginning of
perf-record run. Next patch will process short living bpf programs that
are created during perf-record.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/perf/builtin-record.c |   8 +
 tools/perf/builtin-report.c |   2 +
 tools/perf/util/Build       |   2 +
 tools/perf/util/bpf-info.c  | 305 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/bpf-info.h  |  29 ++++
 tools/perf/util/event.c     |   1 +
 tools/perf/util/event.h     |  14 ++
 tools/perf/util/session.c   |   4 +
 tools/perf/util/tool.h      |   3 +-
 9 files changed, 367 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/util/bpf-info.c
 create mode 100644 tools/perf/util/bpf-info.h

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 0980dfe3396b..86dfba937e4e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -41,6 +41,7 @@
 #include "util/perf-hooks.h"
 #include "util/time-utils.h"
 #include "util/units.h"
+#include "util/bpf-info.h"
 #include "asm/bug.h"
 
 #include <errno.h>
@@ -966,6 +967,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	if (err < 0)
 		goto out_child;
 
+	err = perf_event__synthesize_bpf_prog_info(
+		&rec->tool, process_synthesized_event,
+		&rec->session->machines.host);
+	if (err < 0)
+		goto out_child;
+
 	if (rec->realtime_prio) {
 		struct sched_param param;
 
@@ -1531,6 +1538,7 @@ static struct record record = {
 		.namespaces	= perf_event__process_namespaces,
 		.mmap		= perf_event__process_mmap,
 		.mmap2		= perf_event__process_mmap2,
+		.bpf_prog_info	= perf_event__process_bpf_prog_info,
 		.ordered_events	= true,
 	},
 };
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index c0703979c51d..4a9a3e8da4e0 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -41,6 +41,7 @@
 #include "util/auxtrace.h"
 #include "util/units.h"
 #include "util/branch.h"
+#include "util/bpf-info.h"
 
 #include <dlfcn.h>
 #include <errno.h>
@@ -981,6 +982,7 @@ int cmd_report(int argc, const char **argv)
 			.auxtrace_info	 = perf_event__process_auxtrace_info,
 			.auxtrace	 = perf_event__process_auxtrace,
 			.feature	 = process_feature_event,
+			.bpf_prog_info	 = perf_event__process_bpf_prog_info,
 			.ordered_events	 = true,
 			.ordering_requires_timestamps = true,
 		},
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index ecd9f9ceda77..624c7281217c 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -150,6 +150,8 @@ endif
 
 libperf-y += perf-hooks.o
 
+libperf-$(CONFIG_LIBBPF) += bpf-info.o
+
 libperf-$(CONFIG_CXX) += c++/
 
 CFLAGS_config.o   += -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
diff --git a/tools/perf/util/bpf-info.c b/tools/perf/util/bpf-info.c
new file mode 100644
index 000000000000..2e0005677c43
--- /dev/null
+++ b/tools/perf/util/bpf-info.c
@@ -0,0 +1,305 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Facebook
+ */
+#include <errno.h>
+#include <stdio.h>
+#include <bpf/bpf.h>
+#include "bpf-info.h"
+#include "debug.h"
+#include "session.h"
+
+#define KSYM_NAME_LEN 128
+#define BPF_PROG_INFO_MIN_SIZE 128  /* minimal require jited_func_lens */
+
+static inline __u64 ptr_to_u64(const void *ptr)
+{
+	return (__u64) (unsigned long) ptr;
+}
+
+static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
+{
+	int ret = 0;
+	size_t i;
+
+	for (i = 0; i < len; i++)
+		ret += snprintf(buf + ret, size - ret, "%02x", data[i]);
+	return ret;
+}
+
+/* fetch information of the bpf program via bpf syscall. */
+struct bpf_prog_info_event *perf_bpf_info__get_bpf_prog_info_event(int fd)
+{
+	struct bpf_prog_info_event *prog_info_event = NULL;
+	struct bpf_prog_info info = {};
+	u32 info_len = sizeof(info);
+	u32 event_len, i;
+	void *ptr;
+	int err;
+
+	err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
+	if (err) {
+		pr_debug("can't get prog info: %s", strerror(errno));
+		return NULL;
+	}
+	if (info_len < BPF_PROG_INFO_MIN_SIZE) {
+		pr_debug("kernel is too old to support proper prog info\n");
+		return NULL;
+	}
+
+	/* calculate size of bpf_prog_info_event */
+	event_len = sizeof(struct bpf_prog_info_event);
+	event_len += info_len;
+	event_len -= sizeof(info);
+	event_len += info.jited_prog_len;
+	event_len += info.xlated_prog_len;
+	event_len += info.nr_map_ids * sizeof(u32);
+	event_len += info.nr_jited_ksyms * sizeof(u64);
+	event_len += info.nr_jited_func_lens * sizeof(u32);
+	event_len += info.nr_jited_ksyms * KSYM_NAME_LEN;
+
+	prog_info_event = (struct bpf_prog_info_event *) malloc(event_len);
+	if (!prog_info_event)
+		return NULL;
+
+	/* assign pointers for map_ids, jited_prog_insns, etc. */
+	ptr = prog_info_event->data;
+	info.map_ids = ptr_to_u64(ptr);
+	ptr += info.nr_map_ids * sizeof(u32);
+	info.jited_prog_insns = ptr_to_u64(ptr);
+	ptr += info.jited_prog_len;
+	info.xlated_prog_insns = ptr_to_u64(ptr);
+	ptr += info.xlated_prog_len;
+	info.jited_ksyms = ptr_to_u64(ptr);
+	ptr += info.nr_jited_ksyms * sizeof(u64);
+	info.jited_func_lens = ptr_to_u64(ptr);
+	ptr += info.nr_jited_func_lens * sizeof(u32);
+
+	err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
+	if (err) {
+		pr_err("can't get prog info: %s\n", strerror(errno));
+		free(prog_info_event);
+		return NULL;
+	}
+
+	/* fill data in prog_info_event */
+	prog_info_event->header.type = PERF_RECORD_BPF_PROG_INFO;
+	prog_info_event->header.misc = 0;
+	prog_info_event->prog_info_len = info_len;
+
+	memcpy(&prog_info_event->prog_info, &info, info_len);
+
+	prog_info_event->ksym_table_len = 0;
+	prog_info_event->ksym_table = ptr_to_u64(ptr);
+
+	/* fill in fake symbol name for now, add real name after BTF */
+	if (info.nr_jited_func_lens == 1 && info.name) {  /* only main prog */
+		size_t l;
+
+		assert(info.nr_jited_ksyms == 1);
+		l = snprintf(ptr, KSYM_NAME_LEN, "bpf_prog_");
+		l += snprintf_hex(ptr + l, KSYM_NAME_LEN - l,
+				  prog_info_event->prog_info.tag,
+				  BPF_TAG_SIZE);
+		l += snprintf(ptr + l, KSYM_NAME_LEN - l, "_%s", info.name);
+		prog_info_event->ksym_table_len += l + 1;
+		ptr += l + 1;
+
+	} else {
+		assert(info.nr_jited_ksyms == info.nr_jited_func_lens);
+
+		for (i = 0; i < info.nr_jited_ksyms; i++) {
+			size_t l;
+
+			l = snprintf(ptr, KSYM_NAME_LEN, "bpf_prog_");
+			l += snprintf_hex(ptr + l, KSYM_NAME_LEN - l,
+					  prog_info_event->prog_info.tag,
+					  BPF_TAG_SIZE);
+			l += snprintf(ptr + l, KSYM_NAME_LEN - l, "_F");
+			prog_info_event->ksym_table_len += l + 1;
+			ptr += l + 1;
+		}
+	}
+
+	prog_info_event->header.size = ptr - (void *)prog_info_event;
+
+	return prog_info_event;
+}
+
+static size_t fprintf_bpf_prog_info(
+	struct bpf_prog_info_event *prog_info_event, FILE *fp)
+{
+	struct bpf_prog_info *info = &prog_info_event->prog_info;
+	unsigned long *jited_ksyms = (unsigned long *)(info->jited_ksyms);
+	char *name_ptr = (char *) prog_info_event->ksym_table;
+	unsigned int i;
+	size_t ret;
+
+	ret = fprintf(fp, "bpf_prog: type: %u id: %u ", info->type, info->id);
+	ret += fprintf(fp, "nr_jited_ksyms: %u\n", info->nr_jited_ksyms);
+
+	for (i = 0; i < info->nr_jited_ksyms; i++) {
+		ret += fprintf(fp, "jited_ksyms[%u]: %lx %s\n",
+			       i, jited_ksyms[i], name_ptr);
+		name_ptr += strlen(name_ptr);
+	}
+	return ret;
+}
+
+size_t perf_event__fprintf_bpf_prog_info(union perf_event *event, FILE *fp)
+{
+	return fprintf_bpf_prog_info(&event->bpf_prog_info, fp);
+}
+
+/*
+ * translate all array ptr to offset from base address, called before
+ * writing the event to file
+ */
+void perf_bpf_info__ptr_to_offset(
+	struct bpf_prog_info_event *prog_info_event)
+{
+	u64 base = ptr_to_u64(prog_info_event);
+
+	prog_info_event->ksym_table -= base;
+	prog_info_event->prog_info.jited_prog_insns -= base;
+	prog_info_event->prog_info.xlated_prog_insns -= base;
+	prog_info_event->prog_info.map_ids -= base;
+	prog_info_event->prog_info.jited_ksyms -= base;
+	prog_info_event->prog_info.jited_func_lens -= base;
+}
+
+/*
+ * translate offset from base address to array pointer, called after
+ * reading the event from file
+ */
+void perf_bpf_info__offset_to_ptr(
+	struct bpf_prog_info_event *prog_info_event)
+{
+	u64 base = ptr_to_u64(prog_info_event);
+
+	prog_info_event->ksym_table += base;
+	prog_info_event->prog_info.jited_prog_insns += base;
+	prog_info_event->prog_info.xlated_prog_insns += base;
+	prog_info_event->prog_info.map_ids += base;
+	prog_info_event->prog_info.jited_ksyms += base;
+	prog_info_event->prog_info.jited_func_lens += base;
+}
+
+int perf_event__synthesize_one_bpf_prog_info(struct perf_tool *tool,
+					     perf_event__handler_t process,
+					     struct machine *machine,
+					     int fd)
+{
+	struct bpf_prog_info_event *prog_info_event;
+
+	prog_info_event = perf_bpf_info__get_bpf_prog_info_event(fd);
+
+	if (!prog_info_event) {
+		pr_err("Failed to get prog_info_event\n");
+		return -1;
+	}
+	perf_bpf_info__ptr_to_offset(prog_info_event);
+
+	if (perf_tool__process_synth_event(
+		    tool, (union perf_event *)prog_info_event,
+		    machine, process) != 0) {
+		free(prog_info_event);
+		return -1;
+	}
+
+	free(prog_info_event);
+	return 0;
+}
+
+int perf_event__synthesize_bpf_prog_info(struct perf_tool *tool,
+					 perf_event__handler_t process,
+					 struct machine *machine)
+{
+	__u32 id = 0;
+	int err = 0;
+	int fd;
+
+	while (true) {
+		err = bpf_prog_get_next_id(id, &id);
+		if (err) {
+			if (errno == ENOENT) {
+				err = 0;
+				break;
+			}
+			pr_err("can't get next program: %s%s",
+			       strerror(errno),
+			       errno == EINVAL ? " -- kernel too old?" : "");
+			err = -1;
+			break;
+		}
+		fd = bpf_prog_get_fd_by_id(id);
+		if (fd < 0) {
+			pr_debug("Failed to get fd for prog_id %u\n", id);
+			continue;
+		}
+
+		err = perf_event__synthesize_one_bpf_prog_info(
+			tool, process, machine, fd);
+		close(fd);
+		if (err)
+			break;
+	}
+	return err;
+}
+
+int perf_event__process_bpf_prog_info(struct perf_session *session,
+				      union perf_event *event)
+{
+	struct machine *machine = &session->machines.host;
+	struct bpf_prog_info_event *prog_info_event;
+	struct bpf_prog_info *info;
+	struct symbol *sym;
+	struct map *map;
+	char *name_ptr;
+	int ret = 0;
+	u64 *addrs;
+	u32 *lens;
+	u32 i;
+
+	prog_info_event = (struct bpf_prog_info_event *)
+		malloc(event->header.size);
+	if (!prog_info_event)
+		return -ENOMEM;
+
+	/* copy the data to rw memeory so we can modify it */
+	memcpy(prog_info_event,  &event->bpf_prog_info, event->header.size);
+	info = &prog_info_event->prog_info;
+
+	perf_bpf_info__offset_to_ptr(prog_info_event);
+	name_ptr = (char *) prog_info_event->ksym_table;
+	addrs = (u64 *)info->jited_ksyms;
+	lens = (u32 *)info->jited_func_lens;
+	for (i = 0; i < info->nr_jited_ksyms; i++) {
+		u32 len = info->nr_jited_func_lens == 1 ?
+			len = info->jited_prog_len : lens[i];
+
+		map = map_groups__find(&machine->kmaps, addrs[i]);
+		if (!map) {
+			map = dso__new_map("bpf_prog");
+			if (!map) {
+				ret = -ENOMEM;
+				break;
+			}
+			map->start = addrs[i];
+			map->pgoff = map->start;
+			map->end = map->start + len;
+			map_groups__insert(&machine->kmaps, map);
+		}
+
+		sym = symbol__new(addrs[i], len, 0, 0, name_ptr);
+		if (!sym) {
+			ret = -ENOMEM;
+			break;
+		}
+		dso__insert_symbol(map->dso, sym);
+		name_ptr += strlen(name_ptr) + 1;
+	}
+
+	free(prog_info_event);
+	return ret;
+}
diff --git a/tools/perf/util/bpf-info.h b/tools/perf/util/bpf-info.h
new file mode 100644
index 000000000000..39d5da102017
--- /dev/null
+++ b/tools/perf/util/bpf-info.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PERF_BPF_INFO_H
+#define __PERF_BPF_INFO_H
+
+#include "event.h"
+#include "machine.h"
+#include "tool.h"
+#include "symbol.h"
+
+struct bpf_prog_info_event *perf_bpf_info__get_bpf_prog_info_event(int fd);
+
+size_t perf_event__fprintf_bpf_prog_info(union perf_event *event, FILE *fp);
+
+int perf_event__synthesize_one_bpf_prog_info(struct perf_tool *tool,
+					     perf_event__handler_t process,
+					     struct machine *machine,
+					     int fd);
+
+int perf_event__synthesize_bpf_prog_info(struct perf_tool *tool,
+					 perf_event__handler_t process,
+					 struct machine *machine);
+
+void perf_bpf_info__ptr_to_offset(struct bpf_prog_info_event *prog_info_event);
+void perf_bpf_info__offset_to_ptr(struct bpf_prog_info_event *prog_info_event);
+
+int perf_event__process_bpf_prog_info(struct perf_session *session,
+				      union perf_event *event);
+
+#endif /* __PERF_BPF_INFO_H */
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 04104a4ffe93..de2a89faf445 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -62,6 +62,7 @@ static const char *perf_event__names[] = {
 	[PERF_RECORD_EVENT_UPDATE]		= "EVENT_UPDATE",
 	[PERF_RECORD_TIME_CONV]			= "TIME_CONV",
 	[PERF_RECORD_HEADER_FEATURE]		= "FEATURE",
+	[PERF_RECORD_BPF_PROG_INFO]		= "BPF_PROG_INFO",
 };
 
 static const char *perf_ns__names[] = {
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 13a0c64dd0ed..dc64d800eaa6 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -5,6 +5,7 @@
 #include <limits.h>
 #include <stdio.h>
 #include <linux/kernel.h>
+#include <linux/bpf.h>
 
 #include "../perf.h"
 #include "build-id.h"
@@ -258,6 +259,7 @@ enum perf_user_event_type { /* above any possible kernel type */
 	PERF_RECORD_EVENT_UPDATE		= 78,
 	PERF_RECORD_TIME_CONV			= 79,
 	PERF_RECORD_HEADER_FEATURE		= 80,
+	PERF_RECORD_BPF_PROG_INFO		= 81,
 	PERF_RECORD_HEADER_MAX
 };
 
@@ -629,6 +631,17 @@ struct feature_event {
 	char				data[];
 };
 
+#define KSYM_NAME_LEN 128
+
+struct bpf_prog_info_event {
+	struct perf_event_header	header;
+	u32				prog_info_len;
+	u32				ksym_table_len;
+	u64				ksym_table;
+	struct bpf_prog_info		prog_info;
+	char				data[];
+};
+
 union perf_event {
 	struct perf_event_header	header;
 	struct mmap_event		mmap;
@@ -661,6 +674,7 @@ union perf_event {
 	struct time_conv_event		time_conv;
 	struct feature_event		feat;
 	struct bpf_event		bpf_event;
+	struct bpf_prog_info_event	bpf_prog_info;
 };
 
 void perf_event__print_totals(void);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index dffe5120d2d3..5365ee1dfbec 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -415,6 +415,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
 		tool->time_conv = process_event_op2_stub;
 	if (tool->feature == NULL)
 		tool->feature = process_event_op2_stub;
+	if (tool->bpf_prog_info == NULL)
+		tool->bpf_prog_info = process_event_op2_stub;
 }
 
 static void swap_sample_id_all(union perf_event *event, void *data)
@@ -1397,6 +1399,8 @@ static s64 perf_session__process_user_event(struct perf_session *session,
 		return tool->time_conv(session, event);
 	case PERF_RECORD_HEADER_FEATURE:
 		return tool->feature(session, event);
+	case PERF_RECORD_BPF_PROG_INFO:
+		return tool->bpf_prog_info(session, event);
 	default:
 		return -EINVAL;
 	}
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index 69ae898ca024..739a4b1188f7 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -70,7 +70,8 @@ struct perf_tool {
 			stat_config,
 			stat,
 			stat_round,
-			feature;
+			feature,
+			bpf_prog_info;
 	event_op3	auxtrace;
 	bool		ordered_events;
 	bool		ordering_requires_timestamps;
-- 
2.17.1

^ permalink raw reply related

* [PATCH net 2/2] virtio-net: fail XDP set if guest csum is negotiated
From: Jason Wang @ 2018-11-22  6:36 UTC (permalink / raw)
  To: mst, jasowang, davem, virtualization, netdev, linux-kernel
  Cc: Jesper Dangaard Brouer, Pavel Popa, David Ahern
In-Reply-To: <20181122063631.14452-1-jasowang@redhat.com>

We don't support partial csumed packet since its metadata will be lost
or incorrect during XDP processing. So fail the XDP set if guest_csum
feature is negotiated.

Fixes: f600b6905015 ("virtio_net: Add XDP support")
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Pavel Popa <pashinho1990@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9b5ace538824..cecfd77c9f3c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2361,8 +2361,9 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 	    && (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
 	        virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
 	        virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ECN) ||
-		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_UFO))) {
-		NL_SET_ERR_MSG_MOD(extack, "Can't set XDP while host is implementing LRO, disable LRO first");
+		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_UFO) ||
+		virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_CSUM))) {
+		NL_SET_ERR_MSG_MOD(extack, "Can't set XDP while host is implementing LRO/CSUM, disable LRO/CSUM first");
 		return -EOPNOTSUPP;
 	}
 
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH bpf-next] bpf: fix a libbpf loader issue
From: Martin Lau @ 2018-11-21 20:02 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Alexei Starovoitov, daniel@iogearbox.net, netdev@vger.kernel.org,
	Kernel Team
In-Reply-To: <20181121192242.1802994-1-yhs@fb.com>

On Wed, Nov 21, 2018 at 11:22:42AM -0800, Yonghong Song wrote:
> Commit 2993e0515bb4 ("tools/bpf: add support to read .BTF.ext sections")
> added support to read .BTF.ext sections from an object file, create
> and pass prog_btf_fd and func_info to the kernel.
> 
> The program btf_fd (prog->btf_fd) is initialized to be -1 to please
> zclose so we do not need special handling dur prog close.
> Passing -1 to the kernel, however, will cause loading error.
> Passing btf_fd 0 to the kernel if prog->btf_fd is invalid
> fixed the problem.
Acked-by: Martin KaFai Lau <kafai@fb.com>

^ permalink raw reply

* [PATCH net-next] net: mvneta: remove redundant check for eee->tx_lpi_timer < 0
From: YueHaibing @ 2018-11-22  6:42 UTC (permalink / raw)
  To: davem, thomas.petazzoni; +Cc: linux-kernel, netdev, YueHaibing

fixes the smatch warning:

drivers/net/ethernet/marvell/mvneta.c:4252 mvneta_ethtool_set_eee() warn:
 unsigned 'eee->tx_lpi_timer' is never less than zero.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/marvell/mvneta.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index e5397c8..46a0f6b 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -4248,8 +4248,7 @@ static int mvneta_ethtool_set_eee(struct net_device *dev,
 
 	/* The Armada 37x documents do not give limits for this other than
 	 * it being an 8-bit register. */
-	if (eee->tx_lpi_enabled &&
-	    (eee->tx_lpi_timer < 0 || eee->tx_lpi_timer > 255))
+	if (eee->tx_lpi_enabled && eee->tx_lpi_timer > 255)
 		return -EINVAL;
 
 	lpi_ctl0 = mvreg_read(pp, MVNETA_LPI_CTRL_0);
-- 
2.7.0

^ permalink raw reply related

* Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
From: Yonghong Song @ 2018-11-21 20:18 UTC (permalink / raw)
  To: Andrey Ignatov, netdev@vger.kernel.org
  Cc: ast@kernel.org, daniel@iogearbox.net, Kernel Team
In-Reply-To: <7feafef04f3fc122540ea0592bb5384dc5691894.1542821624.git.rdna@fb.com>



On 11/21/18 9:40 AM, Andrey Ignatov wrote:
> More and more projects use libbpf and one day it'll likely be packaged
> and distributed as DSO and that requires ABI versioning so that both
> compatible and incompatible changes to ABI can be introduced in a safe
> way in the future without breaking executables dynamically linked with a
> previous version of the library.
> 
> Usual way to do ABI versioning is version script for the linker. Add
> such a script for libbpf. All global symbols currently exported via
> LIBBPF_API macro are added to the version script libbpf.map.
> 
> The version name LIBBPF_0.0.1 is constructed from the name of the
> library + version specified by $(LIBBPF_VERSION) in Makefile.
> 
> Version script does not duplicate the work done by LIBBPF_API macro, it
> rather complements it. The macro is used at compile time and can be used
> by compiler to do optimization that can't be done at link time, it is
> purely about global symbol visibility. The version script, in turn, is
> used at link time and takes care of ABI versioning. Both techniques are
> described in details in [1].
> 
> Whenever ABI is changed in the future, version script should be changed
> appropriately.

Maybe we should clarify the policy of how version numbers should be 
change? Each commit which changes default global symbol ABI? Each kernel 
release?

> 
> [1] https://www.akkadia.org/drepper/dsohowto.pdf
> 
> Signed-off-by: Andrey Ignatov <rdna@fb.com>
> ---
>   tools/lib/bpf/Makefile   |   4 +-
>   tools/lib/bpf/libbpf.map | 120 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 123 insertions(+), 1 deletion(-)
>   create mode 100644 tools/lib/bpf/libbpf.map
> 
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 425b480bda75..d76c41fa2d39 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -145,6 +145,7 @@ include $(srctree)/tools/build/Makefile.include
>   
>   BPF_IN    := $(OUTPUT)libbpf-in.o
>   LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE))
> +VERSION_SCRIPT := libbpf.map
>   
>   CMD_TARGETS = $(LIB_FILE)
>   
> @@ -170,7 +171,8 @@ $(BPF_IN): force elfdep bpfdep
>   	$(Q)$(MAKE) $(build)=libbpf
>   
>   $(OUTPUT)libbpf.so: $(BPF_IN)
> -	$(QUIET_LINK)$(CC) --shared $^ -o $@
> +	$(QUIET_LINK)$(CC) --shared -Wl,--version-script=$(VERSION_SCRIPT) \
> +		$^ -o $@
>   
>   $(OUTPUT)libbpf.a: $(BPF_IN)
>   	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> new file mode 100644
> index 000000000000..9fe416b68c7d
> --- /dev/null
> +++ b/tools/lib/bpf/libbpf.map
> @@ -0,0 +1,120 @@
> +LIBBPF_0.0.1 {
> +	global:
> +		bpf_btf_get_fd_by_id;

Do you think we could use this opportunities to
make naming more consistent? For example,
bpf_btf_get_fd_by_id => btf__get_fd_by_id?

> +		bpf_create_map;
> +		bpf_create_map_in_map;
> +		bpf_create_map_in_map_node;
> +		bpf_create_map_name;
> +		bpf_create_map_node;
> +		bpf_create_map_xattr;
> +		bpf_load_btf;
> +		bpf_load_program;
> +		bpf_load_program_xattr;
> +		bpf_map__btf_key_type_id;
> +		bpf_map__btf_value_type_id;
> +		bpf_map__def;
> +		bpf_map_delete_elem; > +		bpf_map__fd;
> +		bpf_map_get_fd_by_id;
> +		bpf_map_get_next_id;
> +		bpf_map_get_next_key; > +		bpf_map__is_offload_neutral;
> +		bpf_map_lookup_and_delete_elem;
> +		bpf_map_lookup_elem;
> +		bpf_map__name;
> +		bpf_map__next;
> +		bpf_map__pin;
> +		bpf_map__prev;
> +		bpf_map__priv;
> +		bpf_map__reuse_fd;
> +		bpf_map__set_ifindex;
> +		bpf_map__set_priv;
> +		bpf_map__unpin;
> +		bpf_map_update_elem;
> +		bpf_object__btf_fd;
> +		bpf_object__close;
> +		bpf_object__find_map_by_name;
> +		bpf_object__find_map_by_offset;
> +		bpf_object__find_program_by_title;
> +		bpf_object__kversion;
> +		bpf_object__load;
> +		bpf_object__name;
> +		bpf_object__next;
> +		bpf_object__open;
> +		bpf_object__open_buffer;
> +		bpf_object__open_xattr;
> +		bpf_object__pin;
> +		bpf_object__pin_maps;
> +		bpf_object__pin_programs;
> +		bpf_object__priv;
> +		bpf_object__set_priv;
> +		bpf_object__unload;
> +		bpf_object__unpin_maps;
> +		bpf_object__unpin_programs;
> +		bpf_obj_get;
> +		bpf_obj_get_info_by_fd;
> +		bpf_obj_pin;
> +		bpf_perf_event_read_simple;
> +		bpf_prog_attach;
> +		bpf_prog_detach;
> +		bpf_prog_detach2;
> +		bpf_prog_get_fd_by_id;
> +		bpf_prog_get_next_id;
> +		bpf_prog_load;
> +		bpf_prog_load_xattr;
> +		bpf_prog_query;
> +		bpf_program__fd;
> +		bpf_program__is_kprobe;
> +		bpf_program__is_perf_event;
> +		bpf_program__is_raw_tracepoint;
> +		bpf_program__is_sched_act;
> +		bpf_program__is_sched_cls;
> +		bpf_program__is_socket_filter;
> +		bpf_program__is_tracepoint;
> +		bpf_program__is_xdp;
> +		bpf_program__load;
> +		bpf_program__next;
> +		bpf_program__nth_fd;
> +		bpf_program__pin;
> +		bpf_program__pin_instance;
> +		bpf_program__prev;
> +		bpf_program__priv;
> +		bpf_program__set_expected_attach_type;
> +		bpf_program__set_ifindex;
> +		bpf_program__set_kprobe;
> +		bpf_program__set_perf_event;
> +		bpf_program__set_prep;
> +		bpf_program__set_priv;
> +		bpf_program__set_raw_tracepoint;
> +		bpf_program__set_sched_act;
> +		bpf_program__set_sched_cls;
> +		bpf_program__set_socket_filter;
> +		bpf_program__set_tracepoint;
> +		bpf_program__set_type;
> +		bpf_program__set_xdp;
> +		bpf_program__title;
> +		bpf_program__unload;
> +		bpf_program__unpin;
> +		bpf_program__unpin_instance;
> +		bpf_prog_test_run;
> +		bpf_raw_tracepoint_open;
> +		bpf_set_link_xdp_fd;
> +		bpf_task_fd_query;
> +		bpf_verify_program;
> +		btf__fd;
> +		btf__find_by_name;
> +		btf__free;
> +		btf_get_from_id;
btf_get_from_id => btf__get_from_id?
> +		btf__name_by_offset;
> +		btf__new;
> +		btf__resolve_size;
> +		btf__resolve_type;
> +		btf__type_by_id;
> +		libbpf_attach_type_by_name;
> +		libbpf_get_error;
> +		libbpf_prog_type_by_name;
> +		libbpf_set_print;
> +		libbpf_strerror;

Anything else? We have btf__, bpf_program__ prefixes with double "_"
while with libbpf_, bpf_<wrapper> as single "_". Not sure whether they 
need change or not.

> +	local:
> +		*;
> +};
> 

^ permalink raw reply

* Re: [PATCH v2 0/2] bpf: permit JIT allocations to be served outside the module region
From: Ard Biesheuvel @ 2018-11-21 20:36 UTC (permalink / raw)
  To: Rick Edgecombe
  Cc: linux-arm-kernel, Linux Kernel Mailing List, Daniel Borkmann,
	Kees Cook, Jessica Yu, Jann Horn, Alexei Starovoitov,
	Mark Rutland, Catalin Marinas, Will Deacon,
	<netdev@vger.kernel.org>, Eric Dumazet, David S. Miller,
	Arnd Bergmann
In-Reply-To: <9d7f77959e1be0a9a3ff511a8fc45518068c85a6.camel@intel.com>

On Wed, 21 Nov 2018 at 20:48, Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
>
> On Wed, 2018-11-21 at 14:17 +0100, Ard Biesheuvel wrote:
> > On arm64, modules are allocated from a 128 MB window which is close to
> > the core kernel, so that relative direct branches are guaranteed to be
> > in range (except in some KASLR configurations). Also, module_alloc()
> > is in charge of allocating KASAN shadow memory when running with KASAN
> > enabled.
> >
> > This means that the way BPF reuses module_alloc()/module_memfree() is
> > undesirable on arm64 (and potentially other architectures as well),
> > and so this series refactors BPF's use of those functions to permit
> > architectures to change this behavior.
> >
> Hi Ard,
>
> I am looking at adding optional BPF JIT in vmalloc functionality for x86 that
> would use this refactor. In fact I have done the same thing with just different
> names.
>
> My implementation intends to use the module space until a usage limit is reached
> and then overflow into vmalloc, so it would be an additional knob like
> "bpf_jit_limit". Wondering if that should be a cross-arch concept that connects
> to this. Does it fit in with what you are trying to do for arm64 here?
>

Hi Rick,

As I understand it, x86 requires the BPF allocations to be located
within 2 GB of the core kernel, so that RIP-relative 32-bit jumps are
in range (I read that in a comment somewhere, or a git commit log
perhaps)

That requirement does not exist on arm64: ordinary function calls and
tail calls emitted by the BPF JIT code have unlimited range, and so
there is simply no reason to prefer the module region for these
allocations. I guess we could achieve the same when reusing your
approach by setting the threshold to zero.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox