Netdev List
 help / color / mirror / Atom feed
* RE: [PATCH][PATCH net-next] hv_sock: Add the support of hibernation
From: Dexuan Cui @ 2019-09-13 20:13 UTC (permalink / raw)
  To: David Miller, sashal@kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Michael Kelley
In-Reply-To: <20190913.210343.724088723062134961.davem@davemloft.net>

> From: David Miller <davem@davemloft.net>
> Sent: Friday, September 13, 2019 1:04 PM
> 
> From: Dexuan Cui <decui@microsoft.com>
> Date: Wed, 11 Sep 2019 23:37:27 +0000
> > I request this patch should go through Sasha's tree rather than the
> > net-next tree.
> 
> That's fine:
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Thanks, David!

@Sasha: I found a few typos in my comment below. I'll post a v2.

> > +/* hv_sock connections can not persist across hibernation, and all the hv_sock
> >  + * channels are forceed to be rescinded before hibernation: see

forceed -> forced

> >  + * are only needed because hibernation requires that every device's driver

every device's driver -> every vmbus device's driver

Thanks,
-- Dexuan

^ permalink raw reply

* Re: [PATCH bpf-next 02/11] samples: bpf: makefile: fix cookie_uid_helper_example obj build
From: Yonghong Song @ 2019-09-13 20:48 UTC (permalink / raw)
  To: Ivan Khoronzhuk, ast@kernel.org, daniel@iogearbox.net,
	davem@davemloft.net, jakub.kicinski@netronome.com,
	hawk@kernel.org, john.fastabend@gmail.com
  Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	bpf@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <20190910103830.20794-3-ivan.khoronzhuk@linaro.org>



On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
> Don't list userspace "cookie_uid_helper_example" object in list for
> bpf objects.
> 
> per_socket_stats_example-opjs is used to list additional dependencies

s/opjs/objs

> for user space binary from hostprogs-y list. Kbuild system creates
> rules for objects listed this way anyway and no need to worry about
> this. Despite on it, the samples bpf uses logic that hostporgs-y are
> build for userspace with includes needed for this, but "always"
> target, if it's not in hostprog-y list, uses CLANG-bpf rule and is
> intended to create bpf obj but not arch obj and uses only kernel
> includes for that. So correct it, as it breaks cross-compiling at
> least.

The above description is a little tricky to understand.
Maybe something like:
    'always' target is for bpf programs.
    'cookie_uid_helper_example.o' is a user space ELF file, and
    covered by rule `per_socket_stats_example`.
    Let us remove `always += cookie_uid_helper_example.o`,
    which avoids breaking cross compilation due to
    mismatched includes.

> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>   samples/bpf/Makefile | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index f50ca852c2a8..43dee90dffa4 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -145,7 +145,6 @@ always += sampleip_kern.o
>   always += lwt_len_hist_kern.o
>   always += xdp_tx_iptunnel_kern.o
>   always += test_map_in_map_kern.o
> -always += cookie_uid_helper_example.o
>   always += tcp_synrto_kern.o
>   always += tcp_rwnd_kern.o
>   always += tcp_bufs_kern.o
> 

^ permalink raw reply

* [GIT] Networking
From: David Miller @ 2019-09-13 20:55 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, netdev, linux-kernel


1) Don't corrupt xfrm_interface parms before validation, from Nicolas
   Dichtel.

2) Revert use of usb-wakeup in btusb, from Mario Limonciello.

3) Block ipv6 packets in bridge netfilter if ipv6 is disabled,
   from Leonardo Bras.

4) IPS_OFFLOAD not honored in ctnetlink, from Pablo Neira Ayuso.

5) Missing ULP check in sock_map, from John Fastabend.

6) Fix receive statistic handling in forcedeth, from Zhu Yanjun.

7) Fix length of SKB allocated in 6pack driver, from Christophe
   JAILLET.

8) ip6_route_info_create() returns an error pointer, not NULL.
   From Maciej Żenczykowski.

9) Only add RDS sock to the hashes after rs_transport is set, from
   Ka-Cheong Poon.

10) Don't double clean TX descriptors in ixgbe, from Ilya Maximets.

11) Presence of transmit IPSEC offload in an SKB is not tested for
    correctly in ixgbe and ixgbevf.  From Steffen Klassert and
    Jeff Kirsher.

12) Need rcu_barrier() when register_netdevice() takes one of the
    notifier based failure paths, from Subash Abhinov Kasiviswanathan.

13) Fix leak in sctp_do_bind(), from Mao Wenan.

Please pull, thanks a lot!

The following changes since commit 089cf7f6ecb266b6a4164919a2e69bd2f938374a:

  Linux 5.3-rc7 (2019-09-02 09:57:40 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git 

for you to fetch changes up to 4d7ffcf3bf1be98d876c570cab8fc31d9fa92725:

  cdc_ether: fix rndis support for Mediatek based smartphones (2019-09-13 22:08:13 +0200)

----------------------------------------------------------------
Alexander Duyck (1):
      ixgbe: Prevent u8 wrapping of ITR value to something less than 10us

Alexei Starovoitov (1):
      bpf: fix precision tracking of stack slots

Bjørn Mork (1):
      cdc_ether: fix rndis support for Mediatek based smartphones

Christophe JAILLET (3):
      net/hamradio/6pack: Fix the size of a sk_buff used in 'sp_bump()'
      ipv6: Fix the link time qualifier of 'ping_v6_proc_exit_net()'
      sctp: Fix the link time qualifier of 'sctp_ctrlsock_exit()'

Colin Ian King (4):
      NFC: st95hf: fix spelling mistake "receieve" -> "receive"
      net: lmc: fix spelling mistake "runnin" -> "running"
      net: hns3: fix spelling mistake "undeflow" -> "underflow"
      mlx4: fix spelling mistake "veify" -> "verify"

Cong Wang (2):
      net_sched: check cops->tcf_block in tc_bind_tclass()
      sch_hhf: ensure quantum and hhf_non_hh_weight are non-zero

David Ahern (2):
      ipv6: Fix RTA_MULTIPATH with nexthop objects
      selftest: A few cleanups for fib_nexthops.sh

David Howells (1):
      rxrpc: Fix misplaced traceline

David S. Miller (8):
      Merge git://git.kernel.org/.../pablo/nf
      Merge branch 'for-upstream' of git://git.kernel.org/.../bluetooth/bluetooth
      Merge branch 'nexthops-Fix-multipath-notifications-for-IPv6-and-selftests'
      Merge branch 'master' of git://git.kernel.org/.../klassert/ipsec
      Merge tag 'wireless-drivers-for-davem-2019-09-05' of git://git.kernel.org/.../kvalo/wireless-drivers
      Merge git://git.kernel.org/.../bpf/bpf
      Merge branch '10GbE' of git://git.kernel.org/.../jkirsher/net-queue
      Merge branch 'sctp_do_bind-leak'

Donald Sharp (1):
      net: Properly update v4 routes with v6 nexthop

Eric Biggers (1):
      isdn/capi: check message length in capi_write()

Eric Dumazet (1):
      net: sched: fix reordering issues

Fernando Fernandez Mancera (1):
      netfilter: nft_socket: fix erroneous socket assignment

Florian Westphal (1):
      xfrm: policy: avoid warning splat when merging nodes

Fred Lotter (1):
      nfp: flower: cmsg rtnl locks can timeout reify messages

Harish Bandi (1):
      Bluetooth: hci_qca: disable irqs when spinlock is acquired

Hui Peng (1):
      rsi: fix a double free bug in rsi_91x_deinit()

Ilya Maximets (1):
      ixgbe: fix double clean of Tx descriptors with xdp

Jeff Kirsher (1):
      ixgbevf: Fix secpath usage for IPsec Tx offload

Jian-Hong Pan (1):
      Bluetooth: btrtl: Additional Realtek 8822CE Bluetooth devices

John Fastabend (1):
      net: sock_map, fix missing ulp check in sock hash case

Jouni Malinen (1):
      mac80211: Do not send Layer 2 Update frame before authorization

Juliet Kim (1):
      net/ibmvnic: free reset work of removed device from queue

Ka-Cheong Poon (1):
      net/rds: An rds_sock is added too early to the hash table

Leonardo Bras (2):
      netfilter: bridge: Drops IPv6 packets if IPv6 module is not loaded
      netfilter: nft_fib_netdev: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

Luca Coelho (1):
      iwlwifi: assign directly to iwl_trans->cfg in QuZ detection

Maciej Żenczykowski (2):
      net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)
      ipv6: addrconf_f6i_alloc - fix non-null pointer check to !IS_ERR()

Mao Wenan (5):
      net: sonic: return NETDEV_TX_OK if failed to map buffer
      net: sonic: replace dev_kfree_skb in sonic_send_packet
      sctp: change return type of sctp_get_port_local
      sctp: remove redundant assignment when call sctp_get_port_local
      sctp: destroy bucket if failed to bind addr

Marcel Holtmann (1):
      Revert "Bluetooth: validate BLE connection interval updates"

Mario Limonciello (1):
      Revert "Bluetooth: btusb: driver to enable the usb-wakeup feature"

Michal Suchanek (1):
      net/ibmvnic: Fix missing { in __ibmvnic_reset

Moritz Fischer (1):
      net: fixed_phy: Add forward declaration for struct gpio_desc;

Navid Emamdoost (3):
      Bluetooth: bpa10x: change return value
      wimax: i2400: fix memory leak
      net: qrtr: fix memort leak in qrtr_tun_write_iter

Neal Cardwell (1):
      tcp: fix tcp_ecn_withdraw_cwr() to clear TCP_ECN_QUEUE_CWR

Nicolas Dichtel (5):
      xfrm interface: avoid corruption on changelink
      xfrm interface: ifname may be wrong in logs
      xfrm interface: fix list corruption for x-netns
      xfrm interface: fix management of phydev
      bridge/mdb: remove wrong use of NLM_F_MULTI

Pablo Neira Ayuso (2):
      netfilter: ctnetlink: honor IPS_OFFLOAD flag
      netfilter: nf_flow_table: set default timeout after successful insertion

Radhey Shyam Pandey (1):
      MAINTAINERS: add myself as maintainer for xilinx axiethernet driver

Randy Dunlap (1):
      lib/Kconfig: fix OBJAGG in lib/ menu structure

Shmulik Ladkani (1):
      net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list

Stanislaw Gruszka (4):
      mt76: mt76x0e: don't use hw encryption for MT7630E
      mt76: mt76x0e: disable 5GHz band for MT7630E
      rt2x00: clear up IV's on key removal
      Revert "rt2800: enable TX_PIN_CFG_LNA_PE_ bits per band"

Stefan Chulski (1):
      net: phylink: Fix flow control resolution

Steffen Klassert (1):
      ixgbe: Fix secpath usage for IPsec TX offload.

Subash Abhinov Kasiviswanathan (1):
      net: Fix null de-reference of device refcount

Wen Huang (1):
      mwifiex: Fix three heap overflow at parsing element in cfg80211_ap_settings

Xin Long (3):
      sctp: use transport pf_retrans in sctp_do_8_2_transport_strike
      tipc: add NULL pointer check before calling kfree_rcu
      sctp: fix the missing put_user when dumping transport thresholds

Yang Yingliang (1):
      tun: fix use-after-free when register netdev failed

Yizhuo (1):
      net: stmmac: dwmac-sun8i: Variable "val" in function sun8i_dwmac_set_syscon() could be uninitialized

Zhu Yanjun (1):
      forcedeth: use per cpu to collect xmit/recv statistics

 MAINTAINERS                                            |   3 +--
 drivers/bluetooth/bpa10x.c                             |   2 +-
 drivers/bluetooth/btusb.c                              |   8 +++----
 drivers/bluetooth/hci_qca.c                            |  10 ++++----
 drivers/isdn/capi/capi.c                               |  10 +++++++-
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c |   2 +-
 drivers/net/ethernet/ibm/ibmvnic.c                     |   9 ++++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c          |   7 ++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c           |  29 +++++++++--------------
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c      |   3 ++-
 drivers/net/ethernet/mellanox/mlx4/main.c              |   2 +-
 drivers/net/ethernet/natsemi/sonic.c                   |   6 ++---
 drivers/net/ethernet/netronome/nfp/flower/cmsg.c       |  10 ++++----
 drivers/net/ethernet/nvidia/forcedeth.c                | 143 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c      |   7 +++++-
 drivers/net/hamradio/6pack.c                           |   4 ++--
 drivers/net/phy/phylink.c                              |   6 ++---
 drivers/net/tun.c                                      |  16 +++++++++----
 drivers/net/usb/cdc_ether.c                            |  10 +++++++-
 drivers/net/wan/lmc/lmc_main.c                         |   2 +-
 drivers/net/wimax/i2400m/op-rfkill.c                   |   1 +
 drivers/net/wireless/intel/iwlwifi/pcie/drv.c          |  24 +++++++++----------
 drivers/net/wireless/marvell/mwifiex/ie.c              |   3 +++
 drivers/net/wireless/marvell/mwifiex/uap_cmd.c         |   9 ++++++-
 drivers/net/wireless/mediatek/mt76/mt76x0/eeprom.c     |   5 ++++
 drivers/net/wireless/mediatek/mt76/mt76x0/pci.c        |  15 +++++++++++-
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c         |  37 ++++++++++++++---------------
 drivers/net/wireless/rsi/rsi_91x_usb.c                 |   1 -
 drivers/nfc/st95hf/core.c                              |   2 +-
 include/linux/phy_fixed.h                              |   1 +
 include/net/ip_fib.h                                   |   4 ++--
 include/net/nexthop.h                                  |   5 ++--
 include/net/xfrm.h                                     |   2 --
 include/uapi/linux/isdn/capicmd.h                      |   1 +
 kernel/bpf/verifier.c                                  |  23 +++++++++++-------
 lib/Kconfig                                            |   6 ++---
 net/bluetooth/hci_event.c                              |   5 ----
 net/bluetooth/l2cap_core.c                             |   9 +------
 net/bridge/br_mdb.c                                    |   2 +-
 net/bridge/br_netfilter_hooks.c                        |   4 ++++
 net/core/dev.c                                         |   2 ++
 net/core/skbuff.c                                      |  19 +++++++++++++++
 net/core/sock_map.c                                    |   3 +++
 net/ipv4/fib_semantics.c                               |  15 ++++++------
 net/ipv4/tcp_input.c                                   |   2 +-
 net/ipv6/ping.c                                        |   2 +-
 net/ipv6/route.c                                       |  21 ++++++++++-------
 net/mac80211/cfg.c                                     |  14 ++++-------
 net/mac80211/sta_info.c                                |   4 ++++
 net/netfilter/nf_conntrack_netlink.c                   |   7 ++++--
 net/netfilter/nf_flow_table_core.c                     |   2 +-
 net/netfilter/nft_fib_netdev.c                         |   3 +++
 net/netfilter/nft_socket.c                             |   6 ++---
 net/qrtr/tun.c                                         |   5 +++-
 net/rds/bind.c                                         |  40 ++++++++++++++------------------
 net/rxrpc/input.c                                      |   2 +-
 net/sched/sch_api.c                                    |   2 ++
 net/sched/sch_generic.c                                |   9 +++++--
 net/sched/sch_hhf.c                                    |   2 +-
 net/sctp/protocol.c                                    |   2 +-
 net/sctp/sm_sideeffect.c                               |   2 +-
 net/sctp/socket.c                                      |  24 ++++++++++---------
 net/tipc/name_distr.c                                  |   3 ++-
 net/xfrm/xfrm_interface.c                              |  56 ++++++++++++++++++++------------------------
 net/xfrm/xfrm_policy.c                                 |   6 +++--
 tools/testing/selftests/net/fib_nexthops.sh            |  24 ++++++++++---------
 tools/testing/selftests/net/xfrm_policy.sh             |   7 ++++++
 67 files changed, 443 insertions(+), 289 deletions(-)

^ permalink raw reply

* Re: [PATCH v4 1/2] tcp: Add TCP_INFO counter for packets received out-of-order
From: Neal Cardwell @ 2019-09-13 20:55 UTC (permalink / raw)
  To: Thomas Higdon
  Cc: netdev@vger.kernel.org, Jonathan Lemon, Dave Jones, Eric Dumazet,
	Dave Taht, Yuchung Cheng, Soheil Hassas Yeganeh
In-Reply-To: <20190913193629.55201-1-tph@fb.com>

On Fri, Sep 13, 2019 at 3:37 PM Thomas Higdon <tph@fb.com> wrote:
>
> For receive-heavy cases on the server-side, we want to track the
> connection quality for individual client IPs. This counter, similar to
> the existing system-wide TCPOFOQueue counter in /proc/net/netstat,
> tracks out-of-order packet reception. By providing this counter in
> TCP_INFO, it will allow understanding to what degree receive-heavy
> sockets are experiencing out-of-order delivery and packet drops
> indicating congestion.
>
> Please note that this is similar to the counter in NetBSD TCP_INFO, and
> has the same name.
>
> Signed-off-by: Thomas Higdon <tph@fb.com>
> ---
>
> no changes from v3
>
>  include/linux/tcp.h      | 2 ++
>  include/uapi/linux/tcp.h | 2 ++
>  net/ipv4/tcp.c           | 2 ++
>  net/ipv4/tcp_input.c     | 1 +
>  4 files changed, 7 insertions(+)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index f3a85a7fb4b1..a01dc78218f1 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -393,6 +393,8 @@ struct tcp_sock {
>          */
>         struct request_sock *fastopen_rsk;
>         u32     *saved_syn;
> +
> +       u32 rcv_ooopack; /* Received out-of-order packets, for tcpinfo */

Thanks for adding this.

A thought: putting the new rcv_ooopack field here makes struct
tcp_sock bigger, and increases the odds of taking a cache miss
(according to "pahole" this field is the only one in a new cache
line).

I'd suggest putting the new rcv_ooopack field immediately before
rcv_rtt_last_tsecr. That would use up a 4-byte hole, and would place
it in a cache line already used on TCP receivers (for rcv_rtt logic).
This would make it less likely this new field causes more cache misses
or uses more space.

Details: looking at the output of "pahole" for tcp_sock in various cases:

net-next before this patch:
-------------------------------------
...
        u8                         bpf_sock_ops_cb_flags; /*  2076     1 */

        /* XXX 3 bytes hole, try to pack */

        u32                        rcv_rtt_last_tsecr;   /*  2080     4 */

        /* XXX 4 bytes hole, try to pack */

        struct {
                u32                rtt_us;               /*  2088     4 */
                u32                seq;                  /*  2092     4 */
                u64                time;                 /*  2096     8 */
        } rcv_rtt_est;                                   /*  2088    16 */
...
        /* size: 2176, cachelines: 34, members: 134 */
        /* sum members: 2164, holes: 4, sum holes: 12 */
        /* paddings: 3, sum paddings: 12 */


net-next with this patch:
-------------------------------------
...
        u32 *                      saved_syn;            /*  2168     8 */
        /* --- cacheline 34 boundary (2176 bytes) --- */
        u32                        rcv_ooopack;          /*  2176     4 */
...
        /* size: 2184, cachelines: 35, members: 135 */
        /* sum members: 2168, holes: 4, sum holes: 12 */
        /* padding: 4 */
        /* paddings: 3, sum paddings: 12 */
        /* last cacheline: 8 bytes */


net-next with this field in the suggested spot:
-------------------------------------
...
       /* XXX 3 bytes hole, try to pack */

        u32                        rcv_ooopack;          /*  2080     4 */
        u32                        rcv_rtt_last_tsecr;   /*  2084     4 */
        struct {
                u32                rtt_us;               /*  2088     4 */
                u32                seq;                  /*  2092     4 */
                u64                time;                 /*  2096     8 */
        } rcv_rtt_est;                                   /*  2088    16 */
...
        /* size: 2176, cachelines: 34, members: 135 */
        /* sum members: 2168, holes: 3, sum holes: 8 */
        /* paddings: 3, sum paddings: 12 */

neal


neal

^ permalink raw reply

* Re: [PATCH v4 2/2] tcp: Add snd_wnd to TCP_INFO
From: Neal Cardwell @ 2019-09-13 21:02 UTC (permalink / raw)
  To: Thomas Higdon
  Cc: netdev@vger.kernel.org, Jonathan Lemon, Dave Jones, Eric Dumazet,
	Dave Taht, Yuchung Cheng, Soheil Hassas Yeganeh
In-Reply-To: <20190913193629.55201-2-tph@fb.com>

On Fri, Sep 13, 2019 at 3:36 PM Thomas Higdon <tph@fb.com> wrote:
>
> Neal Cardwell mentioned that snd_wnd would be useful for diagnosing TCP
> performance problems --
> > (1) Usually when we're diagnosing TCP performance problems, we do so
> > from the sender, since the sender makes most of the
> > performance-critical decisions (cwnd, pacing, TSO size, TSQ, etc).
> > From the sender-side the thing that would be most useful is to see
> > tp->snd_wnd, the receive window that the receiver has advertised to
> > the sender.
>
> This serves the purpose of adding an additional __u32 to avoid the
> would-be hole caused by the addition of the tcpi_rcvi_ooopack field.
>
> Signed-off-by: Thomas Higdon <tph@fb.com>
> ---
> changes from v3:
>  - changed from rcv_wnd to snd_wnd
>
>  include/uapi/linux/tcp.h | 1 +
>  net/ipv4/tcp.c           | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index 20237987ccc8..240654f22d98 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -272,6 +272,7 @@ struct tcp_info {
>         __u32   tcpi_reord_seen;     /* reordering events seen */
>
>         __u32   tcpi_rcv_ooopack;    /* Out-of-order packets received */
> +       __u32   tcpi_snd_wnd;        /* Remote peer's advertised recv window size */
>  };

Thanks for adding this!

My run of ./scripts/checkpatch.pl is showing a warning on this line:

WARNING: line over 80 characters
#19: FILE: include/uapi/linux/tcp.h:273:
+       __u32   tcpi_snd_wnd;        /* Remote peer's advertised recv
window size */

What if the comment is shortened up to fit in 80 columns and the units
(bytes) are added, something like:

        __u32   tcpi_snd_wnd;        /* peer's advertised recv window (bytes) */

neal

^ permalink raw reply

* Re: [PATCH bpf-next 05/11] samples: bpf: makefile: use D vars from KBUILD_CFLAGS to handle headers
From: Yonghong Song @ 2019-09-13 21:12 UTC (permalink / raw)
  To: Ivan Khoronzhuk, ast@kernel.org, daniel@iogearbox.net,
	davem@davemloft.net, jakub.kicinski@netronome.com,
	hawk@kernel.org, john.fastabend@gmail.com
  Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	bpf@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <20190910103830.20794-6-ivan.khoronzhuk@linaro.org>



On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
> The kernel headers are reused from samples bpf, and autoconf.h is not
> enough to reflect complete arch configuration for clang. But CLANG-bpf
> cmds are sensitive for assembler part taken from linux headers and -D
> vars, usually used in CFLAGS, should be carefully added for each arch.
> For that, for CLANG-bpf, lets filter them only for arm arch as it
> definitely requires __LINUX_ARM_ARCH__ to be set, but ignore for
> others till it's really needed. For arm, -D__LINUX_ARM_ARCH__ is min
> version used as instruction set selector. In another case errors
> like "SMP is not supported" for arm and bunch of other errors are
> issued resulting to incorrect final object.
> 
> Later D_OPTIONS can be used for gcc part.
> ---
>   samples/bpf/Makefile | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 8ecc5d0c2d5b..6492b7e65c08 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -185,6 +185,15 @@ HOSTLDLIBS_map_perf_test	+= -lrt
>   HOSTLDLIBS_test_overhead	+= -lrt
>   HOSTLDLIBS_xdpsock		+= -pthread
>   
> +# Strip all expet -D options needed to handle linux headers
> +# for arm it's __LINUX_ARM_ARCH__ and potentially others fork vars
> +D_OPTIONS = $(shell echo "$(KBUILD_CFLAGS) " | sed 's/[[:blank:]]/\n/g' | \
> +	sed '/^-D/!d' | tr '\n' ' ')
> +
> +ifeq ($(ARCH), arm)
> +CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
> +endif

Do you need this for native compilation?

so arm64 compilation does not need this?
If only -D__LINUX_ARM_ARCH__ is needed, maybe just
with
    CLANG_EXTRA_CFLAGS := -D__LINUX_ARM_ARCH__
Otherwise, people will wonder whether this is needed for
other architectures. Or just do
    CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
for all cross compilation?

> +
>   # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>   #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>   LLC ?= llc
> 

^ permalink raw reply

* [PATCH v4.14-stable 1/2] tcp: Reset send_head when removing skb from write-queue
From: Christoph Paasch @ 2019-09-13 20:08 UTC (permalink / raw)
  To: stable, netdev, gregkh, Sasha Levin
  Cc: David Miller, Eric Dumazet, Jason Baron, Vladimir Rutsky,
	Soheil Hassas Yeganeh, Neal Cardwell
In-Reply-To: <20190913200819.32686-1-cpaasch@apple.com>

syzkaller is not happy since commit fdfc5c8594c2 ("tcp: remove empty skb
from write queue in error cases"):

CPU: 1 PID: 13814 Comm: syz-executor.4 Not tainted 4.14.143 #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.5.1 01/01/2011
task: ffff888040105c00 task.stack: ffff8880649c0000
RIP: 0010:tcp_sendmsg_locked+0x6b4/0x4390 net/ipv4/tcp.c:1350
RSP: 0018:ffff8880649cf718 EFLAGS: 00010206
RAX: 0000000000000014 RBX: 000000000000001e RCX: ffffc90000717000
RDX: 0000000000000077 RSI: ffffffff82e760f7 RDI: 00000000000000a0
RBP: ffff8880649cfaa8 R08: 1ffff1100c939e7a R09: ffff8880401063c8
R10: 0000000000000003 R11: 0000000000000001 R12: dffffc0000000000
R13: ffff888043d74750 R14: ffff888043d74500 R15: 000000000000001e
FS:  00007f0afcb6d700(0000) GS:ffff88806cf00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000001b2ca22000 CR3: 0000000040496004 CR4: 00000000003606e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 tcp_sendmsg+0x2a/0x40 net/ipv4/tcp.c:1533
 inet_sendmsg+0x173/0x4e0 net/ipv4/af_inet.c:784
 sock_sendmsg_nosec net/socket.c:646 [inline]
 sock_sendmsg+0xc3/0x100 net/socket.c:656
 SYSC_sendto+0x35d/0x5e0 net/socket.c:1766
 do_syscall_64+0x241/0x680 arch/x86/entry/common.c:292
 entry_SYSCALL_64_after_hwframe+0x42/0xb7

The problem is that we are removing an skb from the write-queue that
could have been referenced by the sk_send_head. Thus, we need to check
for the send_head's sanity after removing it.

This patch needs to be backported only to 4.14 and older (among those
that applied the backport of fdfc5c8594c2).

Fixes: fdfc5c8594c2 ("tcp: remove empty skb from write queue in error cases")
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jason Baron <jbaron@akamai.com>
Cc: Vladimir Rutsky <rutsky@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Christoph Paasch <cpaasch@apple.com>
---
 net/ipv4/tcp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5ce069ce2a97..efe767e20d01 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -924,8 +924,7 @@ static void tcp_remove_empty_skb(struct sock *sk, struct sk_buff *skb)
 {
 	if (skb && !skb->len) {
 		tcp_unlink_write_queue(skb, sk);
-		if (tcp_write_queue_empty(sk))
-			tcp_chrono_stop(sk, TCP_CHRONO_BUSY);
+		tcp_check_send_head(sk, skb);
 		sk_wmem_free_skb(sk, skb);
 	}
 }
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH bpf-next 05/11] samples: bpf: makefile: use D vars from KBUILD_CFLAGS to handle headers
From: Ivan Khoronzhuk @ 2019-09-13 21:24 UTC (permalink / raw)
  To: Yonghong Song
  Cc: ast@kernel.org, daniel@iogearbox.net, davem@davemloft.net,
	jakub.kicinski@netronome.com, hawk@kernel.org,
	john.fastabend@gmail.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	clang-built-linux@googlegroups.com
In-Reply-To: <97ca4228-145a-2449-b4ba-8e79380a54f4@fb.com>

On Fri, Sep 13, 2019 at 09:12:01PM +0000, Yonghong Song wrote:
>
>
>On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
>> The kernel headers are reused from samples bpf, and autoconf.h is not
>> enough to reflect complete arch configuration for clang. But CLANG-bpf
>> cmds are sensitive for assembler part taken from linux headers and -D
>> vars, usually used in CFLAGS, should be carefully added for each arch.
>> For that, for CLANG-bpf, lets filter them only for arm arch as it
>> definitely requires __LINUX_ARM_ARCH__ to be set, but ignore for
>> others till it's really needed. For arm, -D__LINUX_ARM_ARCH__ is min
>> version used as instruction set selector. In another case errors
>> like "SMP is not supported" for arm and bunch of other errors are
>> issued resulting to incorrect final object.
>>
>> Later D_OPTIONS can be used for gcc part.
>> ---
>>   samples/bpf/Makefile | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 8ecc5d0c2d5b..6492b7e65c08 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -185,6 +185,15 @@ HOSTLDLIBS_map_perf_test	+= -lrt
>>   HOSTLDLIBS_test_overhead	+= -lrt
>>   HOSTLDLIBS_xdpsock		+= -pthread
>>
>> +# Strip all expet -D options needed to handle linux headers
>> +# for arm it's __LINUX_ARM_ARCH__ and potentially others fork vars
>> +D_OPTIONS = $(shell echo "$(KBUILD_CFLAGS) " | sed 's/[[:blank:]]/\n/g' | \
>> +	sed '/^-D/!d' | tr '\n' ' ')
>> +
>> +ifeq ($(ARCH), arm)
>> +CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
>> +endif
>
>Do you need this for native compilation?
Yes, native "arm" also requires it.

>
>so arm64 compilation does not need this?
yes, now only arm

>If only -D__LINUX_ARM_ARCH__ is needed, maybe just
>with
>    CLANG_EXTRA_CFLAGS := -D__LINUX_ARM_ARCH__
Value also needed: -D__LINUX_ARM_ARCH_=7 or -D__LINUX_ARM_ARCH_=6
So, need retrieve it.

>Otherwise, people will wonder whether this is needed for
>other architectures. Or just do
>    CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
>for all cross compilation?
arm, cross and native requires it.

Will do this:

# Strip all expet -D options needed to handle linux headers
# for arm it's __LINUX_ARM_ARCH__ and potentially others fork vars
ifeq ($(ARCH), arm)
D_OPTIONS = $(shell echo "$(KBUILD_CFLAGS) " | sed 's/[[:blank:]]/\n/g' | \
	sed '/^-D/!d' | tr '\n' ' ')
endif

CLANG_EXTRA_CFLAGS := $(D_OPTIONS)



>
>> +
>>   # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>>   #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>>   LLC ?= llc
>>

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* Re: [PATCH bpf-next 02/11] samples: bpf: makefile: fix cookie_uid_helper_example obj build
From: Ivan Khoronzhuk @ 2019-09-13 21:25 UTC (permalink / raw)
  To: Yonghong Song
  Cc: ast@kernel.org, daniel@iogearbox.net, davem@davemloft.net,
	jakub.kicinski@netronome.com, hawk@kernel.org,
	john.fastabend@gmail.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	clang-built-linux@googlegroups.com
In-Reply-To: <7f556c1c-abee-41a9-af83-1d72fc33af4b@fb.com>

On Fri, Sep 13, 2019 at 08:48:37PM +0000, Yonghong Song wrote:
>
>
>On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
>> Don't list userspace "cookie_uid_helper_example" object in list for
>> bpf objects.
>>
>> per_socket_stats_example-opjs is used to list additional dependencies
>
>s/opjs/objs
>
>> for user space binary from hostprogs-y list. Kbuild system creates
>> rules for objects listed this way anyway and no need to worry about
>> this. Despite on it, the samples bpf uses logic that hostporgs-y are
>> build for userspace with includes needed for this, but "always"
>> target, if it's not in hostprog-y list, uses CLANG-bpf rule and is
>> intended to create bpf obj but not arch obj and uses only kernel
>> includes for that. So correct it, as it breaks cross-compiling at
>> least.
>
>The above description is a little tricky to understand.
>Maybe something like:
>    'always' target is for bpf programs.
>    'cookie_uid_helper_example.o' is a user space ELF file, and
>    covered by rule `per_socket_stats_example`.
>    Let us remove `always += cookie_uid_helper_example.o`,
>    which avoids breaking cross compilation due to
>    mismatched includes.

Yes, looks better, thanks.

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* [PATCH v4.14-stable 2/2] tcp: Don't dequeue SYN/FIN-segments from write-queue
From: Christoph Paasch @ 2019-09-13 20:08 UTC (permalink / raw)
  To: stable, netdev, gregkh, Sasha Levin
  Cc: David Miller, Eric Dumazet, Jason Baron, Vladimir Rutsky,
	Soheil Hassas Yeganeh, Neal Cardwell
In-Reply-To: <20190913200819.32686-1-cpaasch@apple.com>

If a SYN/FIN-segment is on the write-queue, skb->len is 0, but the
segment actually has been transmitted. end_seq and seq of the tcp_skb_cb
in that case will indicate this difference.

We should not remove such segments from the write-queue as we might be
in SYN_SENT-state and a retransmission-timer is running. When that one
fires, packets_out will be 1, but the write-queue would be empty,
resulting in:

[   61.280214] ------------[ cut here ]------------
[   61.281307] WARNING: CPU: 0 PID: 0 at net/ipv4/tcp_timer.c:429 tcp_retransmit_timer+0x18f9/0x2660
[   61.283498] Modules linked in:
[   61.284084] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.142 #58
[   61.285214] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.5.1 01/01/2011
[   61.286644] task: ffffffff8401e1c0 task.stack: ffffffff84000000
[   61.287758] RIP: 0010:tcp_retransmit_timer+0x18f9/0x2660
[   61.288715] RSP: 0018:ffff88806ce07cb8 EFLAGS: 00010206
[   61.289669] RAX: ffffffff8401e1c0 RBX: ffff88805c998b00 RCX: 0000000000000006
[   61.290968] RDX: 0000000000000100 RSI: 0000000000000000 RDI: ffff88805c9994d8
[   61.292314] RBP: ffff88805c99919a R08: ffff88807fff901c R09: ffff88807fff9008
[   61.293547] R10: ffff88807fff9017 R11: ffff88807fff9010 R12: ffff88805c998b30
[   61.294834] R13: ffffffff844b9380 R14: 0000000000000000 R15: ffff88805c99930c
[   61.296086] FS:  0000000000000000(0000) GS:ffff88806ce00000(0000) knlGS:0000000000000000
[   61.297523] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   61.298646] CR2: 00007f721da50ff8 CR3: 0000000004014002 CR4: 00000000001606f0
[   61.299944] Call Trace:
[   61.300403]  <IRQ>
[   61.300806]  ? kvm_sched_clock_read+0x21/0x30
[   61.301689]  ? sched_clock+0x5/0x10
[   61.302433]  ? sched_clock_cpu+0x18/0x170
[   61.303173]  tcp_write_timer_handler+0x2c1/0x7a0
[   61.304038]  tcp_write_timer+0x13e/0x160
[   61.304794]  call_timer_fn+0x14a/0x5f0
[   61.305480]  ? tcp_write_timer_handler+0x7a0/0x7a0
[   61.306364]  ? __next_timer_interrupt+0x140/0x140
[   61.307229]  ? _raw_spin_unlock_irq+0x24/0x40
[   61.308033]  ? tcp_write_timer_handler+0x7a0/0x7a0
[   61.308887]  ? tcp_write_timer_handler+0x7a0/0x7a0
[   61.309760]  run_timer_softirq+0xc41/0x1080
[   61.310539]  ? trigger_dyntick_cpu.isra.33+0x180/0x180
[   61.311506]  ? ktime_get+0x13f/0x1c0
[   61.312232]  ? clockevents_program_event+0x10d/0x2f0
[   61.313158]  __do_softirq+0x20b/0x96b
[   61.313889]  irq_exit+0x1a7/0x1e0
[   61.314513]  smp_apic_timer_interrupt+0xfc/0x4d0
[   61.315386]  apic_timer_interrupt+0x8f/0xa0
[   61.316129]  </IRQ>

Followed by a panic.

So, before removing an skb with skb->len == 0, let's make sure that the
skb is really empty by checking the end_seq and seq.

This patch needs to be backported only to 4.14 and older (among those
that applied the backport of fdfc5c8594c2).

Fixes: fdfc5c8594c2 ("tcp: remove empty skb from write queue in error cases")
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jason Baron <jbaron@akamai.com>
Cc: Vladimir Rutsky <rutsky@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Christoph Paasch <cpaasch@apple.com>
---
 net/ipv4/tcp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index efe767e20d01..c1f59a53f68f 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -922,7 +922,8 @@ static int tcp_send_mss(struct sock *sk, int *size_goal, int flags)
  */
 static void tcp_remove_empty_skb(struct sock *sk, struct sk_buff *skb)
 {
-	if (skb && !skb->len) {
+	if (skb && !skb->len &&
+	    TCP_SKB_CB(skb)->end_seq == TCP_SKB_CB(skb)->seq) {
 		tcp_unlink_write_queue(skb, sk);
 		tcp_check_send_head(sk, skb);
 		sk_wmem_free_skb(sk, skb);
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH v4 2/2] tcp: Add snd_wnd to TCP_INFO
From: Yuchung Cheng @ 2019-09-13 21:28 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: Thomas Higdon, netdev@vger.kernel.org, Jonathan Lemon, Dave Jones,
	Eric Dumazet, Dave Taht, Soheil Hassas Yeganeh
In-Reply-To: <CADVnQymKS6-jztAbLu_QYWiPYMqoTf5ODzSg3UPJxH+vBt=bmw@mail.gmail.com>

On Fri, Sep 13, 2019 at 2:02 PM Neal Cardwell <ncardwell@google.com> wrote:
>
> On Fri, Sep 13, 2019 at 3:36 PM Thomas Higdon <tph@fb.com> wrote:
> >
> > Neal Cardwell mentioned that snd_wnd would be useful for diagnosing TCP
> > performance problems --
> > > (1) Usually when we're diagnosing TCP performance problems, we do so
> > > from the sender, since the sender makes most of the
> > > performance-critical decisions (cwnd, pacing, TSO size, TSQ, etc).
> > > From the sender-side the thing that would be most useful is to see
> > > tp->snd_wnd, the receive window that the receiver has advertised to
> > > the sender.
> >
> > This serves the purpose of adding an additional __u32 to avoid the
> > would-be hole caused by the addition of the tcpi_rcvi_ooopack field.
> >
> > Signed-off-by: Thomas Higdon <tph@fb.com>
> > ---
> > changes from v3:
> >  - changed from rcv_wnd to snd_wnd
> >
> >  include/uapi/linux/tcp.h | 1 +
> >  net/ipv4/tcp.c           | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> > index 20237987ccc8..240654f22d98 100644
> > --- a/include/uapi/linux/tcp.h
> > +++ b/include/uapi/linux/tcp.h
> > @@ -272,6 +272,7 @@ struct tcp_info {
> >         __u32   tcpi_reord_seen;     /* reordering events seen */
> >
> >         __u32   tcpi_rcv_ooopack;    /* Out-of-order packets received */
> > +       __u32   tcpi_snd_wnd;        /* Remote peer's advertised recv window size */
> >  };
>
> Thanks for adding this!
>
> My run of ./scripts/checkpatch.pl is showing a warning on this line:
>
> WARNING: line over 80 characters
> #19: FILE: include/uapi/linux/tcp.h:273:
> +       __u32   tcpi_snd_wnd;        /* Remote peer's advertised recv
> window size */
>
> What if the comment is shortened up to fit in 80 columns and the units
> (bytes) are added, something like:
>
>         __u32   tcpi_snd_wnd;        /* peer's advertised recv window (bytes) */
just a thought: will tcpi_peer_rcv_wnd be more self-explanatory?
>
> neal

^ permalink raw reply

* Re: [PATCH v3 0/5] Introduce variable length mdev alias
From: Alex Williamson @ 2019-09-13 21:32 UTC (permalink / raw)
  To: Parav Pandit
  Cc: Jiri Pirko, kwankhede@nvidia.com, cohuck@redhat.com,
	davem@davemloft.net, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <AM0PR05MB48667E374853D485788D8159D1B10@AM0PR05MB4866.eurprd05.prod.outlook.com>

On Wed, 11 Sep 2019 16:38:49 +0000
Parav Pandit <parav@mellanox.com> wrote:

> > -----Original Message-----
> > From: linux-kernel-owner@vger.kernel.org <linux-kernel-  
> > owner@vger.kernel.org> On Behalf Of Parav Pandit  
> > Sent: Wednesday, September 11, 2019 10:31 AM
> > To: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Jiri Pirko <jiri@mellanox.com>; kwankhede@nvidia.com;
> > cohuck@redhat.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > kernel@vger.kernel.org; netdev@vger.kernel.org
> > Subject: RE: [PATCH v3 0/5] Introduce variable length mdev alias
> > 
> > Hi Alex,
> >   
> > > -----Original Message-----
> > > From: Alex Williamson <alex.williamson@redhat.com>
> > > Sent: Wednesday, September 11, 2019 8:56 AM
> > > To: Parav Pandit <parav@mellanox.com>
> > > Cc: Jiri Pirko <jiri@mellanox.com>; kwankhede@nvidia.com;
> > > cohuck@redhat.com; davem@davemloft.net; kvm@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; netdev@vger.kernel.org
> > > Subject: Re: [PATCH v3 0/5] Introduce variable length mdev alias
> > >
> > > On Mon, 9 Sep 2019 20:42:32 +0000
> > > Parav Pandit <parav@mellanox.com> wrote:
> > >  
> > > > Hi Alex,
> > > >  
> > > > > -----Original Message-----
> > > > > From: Parav Pandit <parav@mellanox.com>
> > > > > Sent: Sunday, September 1, 2019 11:25 PM
> > > > > To: alex.williamson@redhat.com; Jiri Pirko <jiri@mellanox.com>;
> > > > > kwankhede@nvidia.com; cohuck@redhat.com; davem@davemloft.net
> > > > > Cc: kvm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > > netdev@vger.kernel.org; Parav Pandit <parav@mellanox.com>
> > > > > Subject: [PATCH v3 0/5] Introduce variable length mdev alias
> > > > >
> > > > > To have consistent naming for the netdevice of a mdev and to have
> > > > > consistent naming of the devlink port [1] of a mdev, which is
> > > > > formed using phys_port_name of the devlink port, current UUID is
> > > > > not usable because UUID is too long.
> > > > >
> > > > > UUID in string format is 36-characters long and in binary 128-bit.
> > > > > Both formats are not able to fit within 15 characters limit of
> > > > > netdev  
> > > name.  
> > > > >
> > > > > It is desired to have mdev device naming consistent using UUID.
> > > > > So that widely used user space framework such as ovs [2] can make
> > > > > use of mdev representor in similar way as PCIe SR-IOV VF and PF  
> > > representors.  
> > > > >
> > > > > Hence,
> > > > > (a) mdev alias is created which is derived using sha1 from the
> > > > > mdev  
> > > name.  
> > > > > (b) Vendor driver describes how long an alias should be for the
> > > > > child mdev created for a given parent.
> > > > > (c) Mdev aliases are unique at system level.
> > > > > (d) alias is created optionally whenever parent requested.
> > > > > This ensures that non networking mdev parents can function without
> > > > > alias creation overhead.
> > > > >
> > > > > This design is discussed at [3].
> > > > >
> > > > > An example systemd/udev extension will have,
> > > > >
> > > > > 1. netdev name created using mdev alias available in sysfs.
> > > > >
> > > > > mdev UUID=83b8f4f2-509f-382f-3c1e-e6bfe0fa1001
> > > > > mdev 12 character alias=cd5b146a80a5
> > > > >
> > > > > netdev name of this mdev = enmcd5b146a80a5 Here en = Ethernet link
> > > > > m = mediated device
> > > > >
> > > > > 2. devlink port phys_port_name created using mdev alias.
> > > > > devlink phys_port_name=pcd5b146a80a5
> > > > >
> > > > > This patchset enables mdev core to maintain unique alias for a mdev.
> > > > >
> > > > > Patch-1 Introduces mdev alias using sha1.
> > > > > Patch-2 Ensures that mdev alias is unique in a system.
> > > > > Patch-3 Exposes mdev alias in a sysfs hirerchy, update
> > > > > Documentation
> > > > > Patch-4 Introduces mdev_alias() API.
> > > > > Patch-5 Extends mtty driver to optionally provide alias generation.
> > > > > This also enables to test UUID based sha1 collision and trigger
> > > > > error handling for duplicate sha1 results.
> > > > >
> > > > > [1] http://man7.org/linux/man-pages/man8/devlink-port.8.html
> > > > > [2] https://docs.openstack.org/os-vif/latest/user/plugins/ovs.html
> > > > > [3] https://patchwork.kernel.org/cover/11084231/
> > > > >
> > > > > ---
> > > > > Changelog:
> > > > > v2->v3:
> > > > >  - Addressed comment from Yunsheng Lin
> > > > >  - Changed strcmp() ==0 to !strcmp()
> > > > >  - Addressed comment from Cornelia Hunk
> > > > >  - Merged sysfs Documentation patch with syfs patch
> > > > >  - Added more description for alias return value  
> > > >
> > > > Did you get a chance review this updated series?
> > > > I addressed Cornelia's and yours comment.
> > > > I do not think allocating alias memory twice, once for comparison
> > > > and once for storing is good idea or moving alias generation logic
> > > > inside the mdev_list_lock(). So I didn't address that suggestion of  
> > Cornelia.  
> > >
> > > Sorry, I'm at LPC this week.  I agree, I don't think the double
> > > allocation is necessary, I thought the comment was sufficient to
> > > clarify null'ing the variable.  It's awkward, but seems correct.
> > >
> > > I'm not sure what we do with this patch series though, has the real
> > > consumer of this even been proposed?    
> 
> Jiri already acked to use mdev_alias() to generate phys_port_name several days back in the discussion we had in [1].
> After concluding in the thread [1], I proceed with mdev_alias().
> mlx5_core patches are not yet present on netdev mailing list, but we
> all agree to use it in mdev_alias() in devlink phys_port_name
> generation. So we have collective agreement on how to proceed
> forward. I wasn't probably clear enough in previous email reply about
> it, so adding link here.
> 
> [1] https://patchwork.kernel.org/cover/11084231/#22838955

Jiri may have agreed to the concept, but without patches on the list
proving an end to end solution, I think it's too early for us to commit
to this by preemptively adding it to our API.  "Acked" and "collective
agreement" seem like they overstate something that seems not to have
seen the light of day yet.  Instead I'll say, it looks reasonable, come
back when the real consumer has actually been proposed upstream and has
more buy-in from the community and we'll see if it still looks like the
right approach from an mdev perspective then.  Thanks,

Alex

^ permalink raw reply

* Re: [PATCH bpf-next 07/11] samples: bpf: add makefile.prog for separate CC build
From: Yonghong Song @ 2019-09-13 21:33 UTC (permalink / raw)
  To: Ivan Khoronzhuk, ast@kernel.org, daniel@iogearbox.net,
	davem@davemloft.net, jakub.kicinski@netronome.com,
	hawk@kernel.org, john.fastabend@gmail.com
  Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	bpf@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <20190910103830.20794-8-ivan.khoronzhuk@linaro.org>



On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
> The makefile.prog is added only, will be used in sample/bpf/Makefile
> later in order to switch cross-compiling on CC from HOSTCC.
> 
> The HOSTCC is supposed to build binaries and tools running on the host
> afterwards, in order to simplify build or so, like "fixdep" or else.
> In case of cross compiling "fixdep" is executed on host when the rest
> samples should run on target arch. In order to build binaries for
> target arch with CC and tools running on host with HOSTCC, lets add
> Makefile.prog for simplicity, having definition and routines similar
> to ones, used in script/Makefile.host. This allows later add
> cross-compilation to samples/bpf with minimum changes.

So this is really Makefile.host or Makefile.user, right?
In BPF, 'prog' can refers to user prog or bpf prog.
To avoid ambiguity, maybe Makefile.host?

> 
> Makefile.prog contains only stuff needed for samples/bpf, potentially
> can be reused and extended for other prog sets later and now needed

What do you mean 'extended for other prog sets'? I am wondering whether
we could just include 'scripts/Makefile.host'? How hard it is?

> only for unblocking tricky samples/bpf cross compilation.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>   samples/bpf/Makefile.prog | 77 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 77 insertions(+)
>   create mode 100644 samples/bpf/Makefile.prog
> 
> diff --git a/samples/bpf/Makefile.prog b/samples/bpf/Makefile.prog
> new file mode 100644
> index 000000000000..3781999b9193
> --- /dev/null
> +++ b/samples/bpf/Makefile.prog
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# ==========================================================================
> +# Building binaries on the host system
> +# Binaries are not used during the compilation of the kernel, and intendent
> +# to be build for target board, target board can be host ofc. Added to build
> +# binaries to run not on host system.
> +#
> +# Only C is supported, but can be extended for C++.

The above comment is not needed.
samples/bpf/ only have C now. I am wondering whether your below scripts 
can be simplified, e.g., removing cxxobjs.

> +#
> +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference)
> +# progs-y := xsk_example
> +# Will compile xdpsock_example.c and create an executable named xsk_example
> +#
> +# progs-y    := xdpsock
> +# xdpsock-objs := xdpsock_1.o xdpsock_2.o
> +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable
> +# xdpsock, based on xdpsock_1.o and xdpsock_2.o
> +#
> +# Inherited from scripts/Makefile.host
> +#
> +__progs := $(sort $(progs-y))
> +
> +# C code
> +# Executables compiled from a single .c file
> +prog-csingle	:= $(foreach m,$(__progs), \
> +			$(if $($(m)-objs)$($(m)-cxxobjs),,$(m)))
> +
> +# C executables linked based on several .o files
> +prog-cmulti	:= $(foreach m,$(__progs),\
> +		   $(if $($(m)-cxxobjs),,$(if $($(m)-objs),$(m))))
> +
> +# Object (.o) files compiled from .c files
> +prog-cobjs	:= $(sort $(foreach m,$(__progs),$($(m)-objs)))
> +
> +prog-csingle	:= $(addprefix $(obj)/,$(prog-csingle))
> +prog-cmulti	:= $(addprefix $(obj)/,$(prog-cmulti))
> +prog-cobjs	:= $(addprefix $(obj)/,$(prog-cobjs))
> +
> +#####
> +# Handle options to gcc. Support building with separate output directory
> +
> +_progc_flags   = $(PROGS_CFLAGS) \
> +                 $(PROGCFLAGS_$(basetarget).o)
> +
> +# $(objtree)/$(obj) for including generated headers from checkin source files
> +ifeq ($(KBUILD_EXTMOD),)
> +ifdef building_out_of_srctree
> +_progc_flags   += -I $(objtree)/$(obj)
> +endif
> +endif
> +
> +progc_flags    = -Wp,-MD,$(depfile) $(_progc_flags)
> +
> +# Create executable from a single .c file
> +# prog-csingle -> Executable
> +quiet_cmd_prog-csingle 	= CC  $@
> +      cmd_prog-csingle	= $(CC) $(progc_flags) $(PROGS_LDFLAGS) -o $@ $< \
> +		$(PROGS_LDLIBS) $(PROGLDLIBS_$(@F))
> +$(prog-csingle): $(obj)/%: $(src)/%.c FORCE
> +	$(call if_changed_dep,prog-csingle)
> +
> +# Link an executable based on list of .o files, all plain c
> +# prog-cmulti -> executable
> +quiet_cmd_prog-cmulti	= LD  $@
> +      cmd_prog-cmulti	= $(CC) $(progc_flags) $(PROGS_LDFLAGS) -o $@ \
> +			  $(addprefix $(obj)/,$($(@F)-objs)) \
> +			  $(PROGS_LDLIBS) $(PROGLDLIBS_$(@F))
> +$(prog-cmulti): $(prog-cobjs) FORCE
> +	$(call if_changed,prog-cmulti)
> +$(call multi_depend, $(prog-cmulti), , -objs)
> +
> +# Create .o file from a single .c file
> +# prog-cobjs -> .o
> +quiet_cmd_prog-cobjs	= CC  $@
> +      cmd_prog-cobjs	= $(CC) $(progc_flags) -c -o $@ $<
> +$(prog-cobjs): $(obj)/%.o: $(src)/%.c FORCE
> +	$(call if_changed_dep,prog-cobjs)
> 

^ permalink raw reply

* Re: [PATCH bpf-next 08/11] samples: bpf: makefile: base progs build on makefile.progs
From: Yonghong Song @ 2019-09-13 21:41 UTC (permalink / raw)
  To: Ivan Khoronzhuk, ast@kernel.org, daniel@iogearbox.net,
	davem@davemloft.net, jakub.kicinski@netronome.com,
	hawk@kernel.org, john.fastabend@gmail.com
  Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	bpf@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <20190910103830.20794-9-ivan.khoronzhuk@linaro.org>



On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
> The main reason for that - HOSTCC and CC have different aims.
> It was tested for arm cross compilation, based on linaro toolchain,
> but should work for others.
> 
> In order to split cross compilation (CC) with host build (HOSTCC),
> lets base bpf samples on Makefile.progs. It allows to cross-compile
> samples/bpf progs with CC while auxialry tools running on host built
> with HOSTCC.

I got a compilation failure with the following error

$ make samples/bpf/
   ...
   LD  samples/bpf/hbm
   CC      samples/bpf/syscall_nrs.s
gcc: error: -pg and -fomit-frame-pointer are incompatible
make[2]: *** [samples/bpf/syscall_nrs.s] Error 1
make[1]: *** [samples/bpf/] Error 2
make: *** [sub-make] Error 2

Could you take a look?

> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>   samples/bpf/Makefile | 138 +++++++++++++++++++++++--------------------
>   1 file changed, 73 insertions(+), 65 deletions(-)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index f5dbf3d0c5f3..625a71f2e9d2 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -4,55 +4,53 @@ BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src))
>   TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools
>   
>   # List of programs to build
> -hostprogs-y := test_lru_dist
> -hostprogs-y += sock_example
> -hostprogs-y += fds_example
> -hostprogs-y += sockex1
> -hostprogs-y += sockex2
> -hostprogs-y += sockex3
> -hostprogs-y += tracex1
> -hostprogs-y += tracex2
> -hostprogs-y += tracex3
> -hostprogs-y += tracex4
> -hostprogs-y += tracex5
> -hostprogs-y += tracex6
> -hostprogs-y += tracex7
> -hostprogs-y += test_probe_write_user
> -hostprogs-y += trace_output
> -hostprogs-y += lathist
> -hostprogs-y += offwaketime
> -hostprogs-y += spintest
> -hostprogs-y += map_perf_test
> -hostprogs-y += test_overhead
> -hostprogs-y += test_cgrp2_array_pin
> -hostprogs-y += test_cgrp2_attach
> -hostprogs-y += test_cgrp2_sock
> -hostprogs-y += test_cgrp2_sock2
> -hostprogs-y += xdp1
> -hostprogs-y += xdp2
> -hostprogs-y += xdp_router_ipv4
> -hostprogs-y += test_current_task_under_cgroup
> -hostprogs-y += trace_event
> -hostprogs-y += sampleip
> -hostprogs-y += tc_l2_redirect
> -hostprogs-y += lwt_len_hist
> -hostprogs-y += xdp_tx_iptunnel
> -hostprogs-y += test_map_in_map
> -hostprogs-y += per_socket_stats_example
> -hostprogs-y += xdp_redirect
> -hostprogs-y += xdp_redirect_map
> -hostprogs-y += xdp_redirect_cpu
> -hostprogs-y += xdp_monitor
> -hostprogs-y += xdp_rxq_info
> -hostprogs-y += syscall_tp
> -hostprogs-y += cpustat
> -hostprogs-y += xdp_adjust_tail
> -hostprogs-y += xdpsock
> -hostprogs-y += xdp_fwd
> -hostprogs-y += task_fd_query
> -hostprogs-y += xdp_sample_pkts
> -hostprogs-y += ibumad
> -hostprogs-y += hbm
> +progs-y := test_lru_dist
> +progs-y += sock_example
> +progs-y += fds_example
> +progs-y += sockex1
> +progs-y += sockex2
> +progs-y += sockex3
> +progs-y += tracex1
> +progs-y += tracex2
> +progs-y += tracex3
> +progs-y += tracex4
> +progs-y += tracex5
> +progs-y += tracex6
> +progs-y += tracex7
> +progs-y += test_probe_write_user
> +progs-y += trace_output
> +progs-y += lathist
> +progs-y += offwaketime
> +progs-y += spintest
> +progs-y += map_perf_test
> +progs-y += test_overhead
> +progs-y += test_cgrp2_array_pin
> +progs-y += test_cgrp2_attach
> +progs-y += test_cgrp2_sock
> +progs-y += test_cgrp2_sock2
> +progs-y += xdp1
> +progs-y += xdp2
> +progs-y += xdp_router_ipv4
> +progs-y += test_current_task_under_cgroup
> +progs-y += trace_event
> +progs-y += sampleip
> +progs-y += tc_l2_redirect
> +progs-y += lwt_len_hist
> +progs-y += xdp_tx_iptunnel
> +progs-y += test_map_in_map
> +progs-y += xdp_redirect_map
> +progs-y += xdp_redirect_cpu
> +progs-y += xdp_monitor
> +progs-y += xdp_rxq_info
> +progs-y += syscall_tp
> +progs-y += cpustat
> +progs-y += xdp_adjust_tail
> +progs-y += xdpsock
> +progs-y += xdp_fwd
> +progs-y += task_fd_query
> +progs-y += xdp_sample_pkts
> +progs-y += ibumad
> +progs-y += hbm
>   
>   # Libbpf dependencies
>   LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
> @@ -111,7 +109,7 @@ ibumad-objs := bpf_load.o ibumad_user.o $(TRACE_HELPERS)
>   hbm-objs := bpf_load.o hbm.o $(CGROUP_HELPERS)
>   
>   # Tell kbuild to always build the programs
> -always := $(hostprogs-y)
> +always := $(progs-y)
>   always += sockex1_kern.o
>   always += sockex2_kern.o
>   always += sockex3_kern.o
> @@ -170,21 +168,6 @@ always += ibumad_kern.o
>   always += hbm_out_kern.o
>   always += hbm_edt_kern.o
>   
> -KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include
> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/bpf/
> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf
> -
> -HOSTCFLAGS_bpf_load.o += -Wno-unused-variable
> -
> -KBUILD_HOSTLDLIBS		+= $(LIBBPF) -lelf
> -HOSTLDLIBS_tracex4		+= -lrt
> -HOSTLDLIBS_trace_output	+= -lrt
> -HOSTLDLIBS_map_perf_test	+= -lrt
> -HOSTLDLIBS_test_overhead	+= -lrt
> -HOSTLDLIBS_xdpsock		+= -pthread
> -
>   # Strip all expet -D options needed to handle linux headers
>   # for arm it's __LINUX_ARM_ARCH__ and potentially others fork vars
>   D_OPTIONS = $(shell echo "$(KBUILD_CFLAGS) " | sed 's/[[:blank:]]/\n/g' | \
> @@ -194,6 +177,29 @@ ifeq ($(ARCH), arm)
>   CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
>   endif
>   
> +ccflags-y += -I$(objtree)/usr/include
> +ccflags-y += -I$(srctree)/tools/lib/bpf/
> +ccflags-y += -I$(srctree)/tools/testing/selftests/bpf/
> +ccflags-y += -I$(srctree)/tools/lib/
> +ccflags-y += -I$(srctree)/tools/include
> +ccflags-y += -I$(srctree)/tools/perf
> +ccflags-y += $(D_OPTIONS)
> +ccflags-y += -Wall
> +ccflags-y += -fomit-frame-pointer
> +ccflags-y += -Wmissing-prototypes
> +ccflags-y += -Wstrict-prototypes
> +
> +PROGS_CFLAGS := $(ccflags-y)
> +
> +PROGCFLAGS_bpf_load.o += -Wno-unused-variable
> +
> +PROGS_LDLIBS			:= $(LIBBPF) -lelf
> +PROGLDLIBS_tracex4		+= -lrt
> +PROGLDLIBS_trace_output		+= -lrt
> +PROGLDLIBS_map_perf_test	+= -lrt
> +PROGLDLIBS_test_overhead	+= -lrt
> +PROGLDLIBS_xdpsock		+= -pthread
> +
>   # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>   #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>   LLC ?= llc
> @@ -284,6 +290,8 @@ $(obj)/hbm_out_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>   $(obj)/hbm.o: $(src)/hbm.h
>   $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>   
> +-include $(BPF_SAMPLES_PATH)/Makefile.prog
> +
>   # asm/sysreg.h - inline assembly used by it is incompatible with llvm.
>   # But, there is no easy way to fix it, so just exclude it since it is
>   # useless for BPF samples.
> 

^ permalink raw reply

* Re: [PATCH bpf-next 10/11] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets
From: Yonghong Song @ 2019-09-13 21:43 UTC (permalink / raw)
  To: Ivan Khoronzhuk, ast@kernel.org, daniel@iogearbox.net,
	davem@davemloft.net, jakub.kicinski@netronome.com,
	hawk@kernel.org, john.fastabend@gmail.com
  Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	bpf@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <20190910103830.20794-11-ivan.khoronzhuk@linaro.org>



On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
> correctly to build command, for instance when --sysroot is used or
> external libraries are used, like -lelf, wich can be absent in
> toolchain. This is used for samples/bpf cross-compiling allowing to
> get elf lib from sysroot.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>   samples/bpf/Makefile   |  8 +++++++-
>   tools/lib/bpf/Makefile | 11 ++++++++---
>   2 files changed, 15 insertions(+), 4 deletions(-)

Could you separate this patch into two?
One of libbpf and another for samples.

The subject 'libbpf: ...' is not entirely accurate.

> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 79c9aa41832e..4edc5232cfc1 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -186,6 +186,10 @@ ccflags-y += -I$(srctree)/tools/perf
>   ccflags-y += $(D_OPTIONS)
>   ccflags-y += -Wall
>   ccflags-y += -fomit-frame-pointer
> +
> +EXTRA_CXXFLAGS := $(ccflags-y)
> +
> +# options not valid for C++
>   ccflags-y += -Wmissing-prototypes
>   ccflags-y += -Wstrict-prototypes
>   
> @@ -252,7 +256,9 @@ clean:
>   
>   $(LIBBPF): FORCE
>   # Fix up variables inherited from Kbuild that tools/ build system won't like
> -	$(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(BPF_SAMPLES_PATH)/../../ O=
> +	$(MAKE) -C $(dir $@) RM='rm -rf' EXTRA_CFLAGS="$(PROGS_CFLAGS)" \
> +		EXTRA_CXXFLAGS="$(EXTRA_CXXFLAGS)" LDFLAGS=$(PROGS_LDFLAGS) \
> +		srctree=$(BPF_SAMPLES_PATH)/../../ O=
>   
>   $(obj)/syscall_nrs.h:	$(obj)/syscall_nrs.s FORCE
>   	$(call filechk,offsets,__SYSCALL_NRS_H__)
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index c6f94cffe06e..bccfa556ef4e 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -94,6 +94,10 @@ else
>     CFLAGS := -g -Wall
>   endif
>   
> +ifdef EXTRA_CXXFLAGS
> +  CXXFLAGS := $(EXTRA_CXXFLAGS)
> +endif
> +
>   ifeq ($(feature-libelf-mmap), 1)
>     override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
>   endif
> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
>   $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
>   
>   $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
> -	$(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> -				    -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> +	$(QUIET_LINK)$(CC) $(LDFLAGS) \
> +		--shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> +		-Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>   	@ln -sf $(@F) $(OUTPUT)libbpf.so
>   	@ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
>   
> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
>   	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
>   
>   $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
> -	$(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
> +	$(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
>   
>   $(OUTPUT)libbpf.pc:
>   	$(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
> 

^ permalink raw reply

* Re: [PATCH bpf-next 11/11] samples: bpf: makefile: add sysroot support
From: Yonghong Song @ 2019-09-13 21:45 UTC (permalink / raw)
  To: Ivan Khoronzhuk, ast@kernel.org, daniel@iogearbox.net,
	davem@davemloft.net, jakub.kicinski@netronome.com,
	hawk@kernel.org, john.fastabend@gmail.com
  Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	bpf@vger.kernel.org, clang-built-linux@googlegroups.com
In-Reply-To: <20190910103830.20794-12-ivan.khoronzhuk@linaro.org>



On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
> Basically it only enables that was added by previous couple fixes.
> For sure, just make tools/include to be included after sysroot
> headers.
> 
> export ARCH=arm
> export CROSS_COMPILE=arm-linux-gnueabihf-
> make samples/bpf/ SYSROOT="path/to/sysroot"
> 
> Sysroot contains correct libs installed and its headers ofc.
> Useful when working with NFC or virtual machine.
> 
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
>   samples/bpf/Makefile   |  5 +++++
>   samples/bpf/README.rst | 10 ++++++++++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 4edc5232cfc1..68ba78d1dbbe 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -177,6 +177,11 @@ ifeq ($(ARCH), arm)
>   CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
>   endif
>   
> +ifdef SYSROOT
> +ccflags-y += --sysroot=${SYSROOT}
> +PROGS_LDFLAGS := -L${SYSROOT}/usr/lib
> +endif
> +
>   ccflags-y += -I$(objtree)/usr/include
>   ccflags-y += -I$(srctree)/tools/lib/bpf/
>   ccflags-y += -I$(srctree)/tools/testing/selftests/bpf/
> diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst
> index 5f27e4faca50..786d0ab98e8a 100644
> --- a/samples/bpf/README.rst
> +++ b/samples/bpf/README.rst
> @@ -74,3 +74,13 @@ samples for the cross target.
>   export ARCH=arm64
>   export CROSS_COMPILE="aarch64-linux-gnu-"
>   make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> +
> +If need to use environment of target board (headers and libs), the SYSROOT
> +also can be set, pointing on FS of target board:
> +
> +export ARCH=arm64
> +export CROSS_COMPILE="aarch64-linux-gnu-"
> +make samples/bpf/ SYSROOT=~/some_sdk/linux-devkit/sysroots/aarch64-linux-gnu
> +
> +Setting LLC and CLANG is not necessarily if it's installed on HOST and have
> +in its targets appropriate arch triple (usually it has several arches).

You have very good description about how to build and test in cover 
letter. Could you include those instructions here as well? This will
help keep a record so later people can try/test if needed.

^ permalink raw reply

* Re: [PATCH v4 2/2] tcp: Add snd_wnd to TCP_INFO
From: Neal Cardwell @ 2019-09-13 21:53 UTC (permalink / raw)
  To: Yuchung Cheng
  Cc: Thomas Higdon, netdev@vger.kernel.org, Jonathan Lemon, Dave Jones,
	Eric Dumazet, Dave Taht, Soheil Hassas Yeganeh
In-Reply-To: <CAK6E8=ddxo+yg2tTiZm5YEbfPkeVkeZOGwB33+Qfb4Qfj4yDJA@mail.gmail.com>

On Fri, Sep 13, 2019 at 5:29 PM Yuchung Cheng <ycheng@google.com> wrote:
> > What if the comment is shortened up to fit in 80 columns and the units
> > (bytes) are added, something like:
> >
> >         __u32   tcpi_snd_wnd;        /* peer's advertised recv window (bytes) */
> just a thought: will tcpi_peer_rcv_wnd be more self-explanatory?

Good suggestion. I'm on the fence about that one. By itself, I agree
tcpi_peer_rcv_wnd sounds much more clear. But tcpi_snd_wnd has the
virtue of matching both the kernel code (tp->snd_wnd) and RFC 793
(SND.WND). So they both have pros and cons. Maybe someone else feels
more strongly one way or the other.

neal

^ permalink raw reply

* Re: [PATCH bpf-next 07/11] samples: bpf: add makefile.prog for separate CC build
From: Ivan Khoronzhuk @ 2019-09-13 22:14 UTC (permalink / raw)
  To: Yonghong Song
  Cc: ast@kernel.org, daniel@iogearbox.net, davem@davemloft.net,
	jakub.kicinski@netronome.com, hawk@kernel.org,
	john.fastabend@gmail.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	clang-built-linux@googlegroups.com
In-Reply-To: <1720c5a5-5c64-46a3-be2f-56b59614f82a@fb.com>

On Fri, Sep 13, 2019 at 09:33:58PM +0000, Yonghong Song wrote:
>
>
>On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
>> The makefile.prog is added only, will be used in sample/bpf/Makefile
>> later in order to switch cross-compiling on CC from HOSTCC.
>>
>> The HOSTCC is supposed to build binaries and tools running on the host
>> afterwards, in order to simplify build or so, like "fixdep" or else.
>> In case of cross compiling "fixdep" is executed on host when the rest
>> samples should run on target arch. In order to build binaries for
>> target arch with CC and tools running on host with HOSTCC, lets add
>> Makefile.prog for simplicity, having definition and routines similar
>> to ones, used in script/Makefile.host. This allows later add
>> cross-compilation to samples/bpf with minimum changes.
>
>So this is really Makefile.host or Makefile.user, right?
It's cut and modified version of Makefile.host

>In BPF, 'prog' can refers to user prog or bpf prog.
>To avoid ambiguity, maybe Makefile.host?
Makefile.target? as target = host not always.
Makefile.user also looks ok, but doesn't contain target,
maybe "targetuser", but bpf is also target...
Initially I was thinking about Makefile.targetprog, but it looks too long.

>
>>
>> Makefile.prog contains only stuff needed for samples/bpf, potentially
>> can be reused and extended for other prog sets later and now needed
>
>What do you mean 'extended for other prog sets'? I am wondering whether
Another kind of samples...with c++ for instance.

>we could just include 'scripts/Makefile.host'? How hard it is?
It's bound to HOSTCC and it's environment. It is included by default and
was used before this patchset, blocking cross complication.

>
>> only for unblocking tricky samples/bpf cross compilation.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>   samples/bpf/Makefile.prog | 77 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 77 insertions(+)
>>   create mode 100644 samples/bpf/Makefile.prog
>>
>> diff --git a/samples/bpf/Makefile.prog b/samples/bpf/Makefile.prog
>> new file mode 100644
>> index 000000000000..3781999b9193
>> --- /dev/null
>> +++ b/samples/bpf/Makefile.prog
>> @@ -0,0 +1,77 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# ==========================================================================
>> +# Building binaries on the host system
>> +# Binaries are not used during the compilation of the kernel, and intendent
>> +# to be build for target board, target board can be host ofc. Added to build
>> +# binaries to run not on host system.
>> +#
>> +# Only C is supported, but can be extended for C++.
>
>The above comment is not needed.
will drop it.

>samples/bpf/ only have C now. I am wondering whether your below scripts
>can be simplified, e.g., removing cxxobjs.
I think yes, will try

>
>> +#
>> +# Sample syntax (see Documentation/kbuild/makefiles.rst for reference)
>> +# progs-y := xsk_example
>> +# Will compile xdpsock_example.c and create an executable named xsk_example
>> +#
>> +# progs-y    := xdpsock
>> +# xdpsock-objs := xdpsock_1.o xdpsock_2.o
>> +# Will compile xdpsock_1.c and xdpsock_2.c, and then link the executable
>> +# xdpsock, based on xdpsock_1.o and xdpsock_2.o
>> +#
>> +# Inherited from scripts/Makefile.host
>> +#
>> +__progs := $(sort $(progs-y))
>> +
>> +# C code
>> +# Executables compiled from a single .c file
>> +prog-csingle	:= $(foreach m,$(__progs), \
>> +			$(if $($(m)-objs)$($(m)-cxxobjs),,$(m)))
>> +
>> +# C executables linked based on several .o files
>> +prog-cmulti	:= $(foreach m,$(__progs),\
>> +		   $(if $($(m)-cxxobjs),,$(if $($(m)-objs),$(m))))
>> +
>> +# Object (.o) files compiled from .c files
>> +prog-cobjs	:= $(sort $(foreach m,$(__progs),$($(m)-objs)))
>> +
>> +prog-csingle	:= $(addprefix $(obj)/,$(prog-csingle))
>> +prog-cmulti	:= $(addprefix $(obj)/,$(prog-cmulti))
>> +prog-cobjs	:= $(addprefix $(obj)/,$(prog-cobjs))
>> +
>> +#####
>> +# Handle options to gcc. Support building with separate output directory
>> +
>> +_progc_flags   = $(PROGS_CFLAGS) \
>> +                 $(PROGCFLAGS_$(basetarget).o)
>> +
>> +# $(objtree)/$(obj) for including generated headers from checkin source files
>> +ifeq ($(KBUILD_EXTMOD),)
>> +ifdef building_out_of_srctree
>> +_progc_flags   += -I $(objtree)/$(obj)
>> +endif
>> +endif
>> +
>> +progc_flags    = -Wp,-MD,$(depfile) $(_progc_flags)
>> +
>> +# Create executable from a single .c file
>> +# prog-csingle -> Executable
>> +quiet_cmd_prog-csingle 	= CC  $@
>> +      cmd_prog-csingle	= $(CC) $(progc_flags) $(PROGS_LDFLAGS) -o $@ $< \
>> +		$(PROGS_LDLIBS) $(PROGLDLIBS_$(@F))
>> +$(prog-csingle): $(obj)/%: $(src)/%.c FORCE
>> +	$(call if_changed_dep,prog-csingle)
>> +
>> +# Link an executable based on list of .o files, all plain c
>> +# prog-cmulti -> executable
>> +quiet_cmd_prog-cmulti	= LD  $@
>> +      cmd_prog-cmulti	= $(CC) $(progc_flags) $(PROGS_LDFLAGS) -o $@ \
>> +			  $(addprefix $(obj)/,$($(@F)-objs)) \
>> +			  $(PROGS_LDLIBS) $(PROGLDLIBS_$(@F))
>> +$(prog-cmulti): $(prog-cobjs) FORCE
>> +	$(call if_changed,prog-cmulti)
>> +$(call multi_depend, $(prog-cmulti), , -objs)
>> +
>> +# Create .o file from a single .c file
>> +# prog-cobjs -> .o
>> +quiet_cmd_prog-cobjs	= CC  $@
>> +      cmd_prog-cobjs	= $(CC) $(progc_flags) -c -o $@ $<
>> +$(prog-cobjs): $(obj)/%.o: $(src)/%.c FORCE
>> +	$(call if_changed_dep,prog-cobjs)
>>

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* Re: [PATCH bpf-next 08/11] samples: bpf: makefile: base progs build on makefile.progs
From: Ivan Khoronzhuk @ 2019-09-13 22:25 UTC (permalink / raw)
  To: Yonghong Song
  Cc: ast@kernel.org, daniel@iogearbox.net, davem@davemloft.net,
	jakub.kicinski@netronome.com, hawk@kernel.org,
	john.fastabend@gmail.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	clang-built-linux@googlegroups.com
In-Reply-To: <dd4cd83f-7e35-ad07-8a53-d34c13c074a5@fb.com>

On Fri, Sep 13, 2019 at 09:41:25PM +0000, Yonghong Song wrote:
>
>
>On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
>> The main reason for that - HOSTCC and CC have different aims.
>> It was tested for arm cross compilation, based on linaro toolchain,
>> but should work for others.
>>
>> In order to split cross compilation (CC) with host build (HOSTCC),
>> lets base bpf samples on Makefile.progs. It allows to cross-compile
>> samples/bpf progs with CC while auxialry tools running on host built
>> with HOSTCC.
>
>I got a compilation failure with the following error
>
>$ make samples/bpf/
>   ...
>   LD  samples/bpf/hbm
>   CC      samples/bpf/syscall_nrs.s
>gcc: error: -pg and -fomit-frame-pointer are incompatible
>make[2]: *** [samples/bpf/syscall_nrs.s] Error 1
>make[1]: *** [samples/bpf/] Error 2
>make: *** [sub-make] Error 2
>
>Could you take a look?
Yes, sure.
Ilias also observer this, interesting that on my setup for cross and
native build on arm and arm64 doesn't have this error. Now I see log
and know how to proceed, I will fix it, maybe by using another var
for cflags.

Despite of it, just to be sure in order to avoid smth like this at least
for native build, I will add one more patch like:

"
    samples: bpf: makefile: don't use host cflags when cross compile

    While compile natively, the hosts cflags and ldflags are equal to ones
    used from HOSTCFLAGS and HOSTLDFLAGS. When cross compiling it should
    have own, used for target arch. While verification, for arm, arm64 and
    x86_64 the following flags were used alsways:

    -Wall
    -O2
    -fomit-frame-pointer
    -Wmissing-prototypes
    -Wstrict-prototypes

    So, add them as they were verified and used before adding
    Makefile.progs, but anyway limit it only for cross compile options as
    for host can be some configurations when another options can be used,
    So, for host arch samples left all as is, it allows to avoid potential
    option mistmatches for existent environments.
"

+ifdef CROSS_COMPILE
+ccflags-y += -Wall
+ccflags-y += -O2
+ccflags-y += -fomit-frame-pointer
+ccflags-y += -Wmissing-prototypes
+ccflags-y += -Wstrict-prototypes
+else
+ccflags-y += $(KBUILD_HOSTCFLAGS) $(HOST_EXTRACFLAGS)
+PROGS_LDLIBS := $(KBUILD_HOSTLDLIBS)
+endif

>
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>   samples/bpf/Makefile | 138 +++++++++++++++++++++++--------------------
>>   1 file changed, 73 insertions(+), 65 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index f5dbf3d0c5f3..625a71f2e9d2 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -4,55 +4,53 @@ BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src))
>>   TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools
>>
>>   # List of programs to build
>> -hostprogs-y := test_lru_dist
>> -hostprogs-y += sock_example
>> -hostprogs-y += fds_example
>> -hostprogs-y += sockex1
>> -hostprogs-y += sockex2
>> -hostprogs-y += sockex3
>> -hostprogs-y += tracex1
>> -hostprogs-y += tracex2
>> -hostprogs-y += tracex3
>> -hostprogs-y += tracex4
>> -hostprogs-y += tracex5
>> -hostprogs-y += tracex6
>> -hostprogs-y += tracex7
>> -hostprogs-y += test_probe_write_user
>> -hostprogs-y += trace_output
>> -hostprogs-y += lathist
>> -hostprogs-y += offwaketime
>> -hostprogs-y += spintest
>> -hostprogs-y += map_perf_test
>> -hostprogs-y += test_overhead
>> -hostprogs-y += test_cgrp2_array_pin
>> -hostprogs-y += test_cgrp2_attach
>> -hostprogs-y += test_cgrp2_sock
>> -hostprogs-y += test_cgrp2_sock2
>> -hostprogs-y += xdp1
>> -hostprogs-y += xdp2
>> -hostprogs-y += xdp_router_ipv4
>> -hostprogs-y += test_current_task_under_cgroup
>> -hostprogs-y += trace_event
>> -hostprogs-y += sampleip
>> -hostprogs-y += tc_l2_redirect
>> -hostprogs-y += lwt_len_hist
>> -hostprogs-y += xdp_tx_iptunnel
>> -hostprogs-y += test_map_in_map
>> -hostprogs-y += per_socket_stats_example
>> -hostprogs-y += xdp_redirect
>> -hostprogs-y += xdp_redirect_map
>> -hostprogs-y += xdp_redirect_cpu
>> -hostprogs-y += xdp_monitor
>> -hostprogs-y += xdp_rxq_info
>> -hostprogs-y += syscall_tp
>> -hostprogs-y += cpustat
>> -hostprogs-y += xdp_adjust_tail
>> -hostprogs-y += xdpsock
>> -hostprogs-y += xdp_fwd
>> -hostprogs-y += task_fd_query
>> -hostprogs-y += xdp_sample_pkts
>> -hostprogs-y += ibumad
>> -hostprogs-y += hbm
>> +progs-y := test_lru_dist
>> +progs-y += sock_example
>> +progs-y += fds_example
>> +progs-y += sockex1
>> +progs-y += sockex2
>> +progs-y += sockex3
>> +progs-y += tracex1
>> +progs-y += tracex2
>> +progs-y += tracex3
>> +progs-y += tracex4
>> +progs-y += tracex5
>> +progs-y += tracex6
>> +progs-y += tracex7
>> +progs-y += test_probe_write_user
>> +progs-y += trace_output
>> +progs-y += lathist
>> +progs-y += offwaketime
>> +progs-y += spintest
>> +progs-y += map_perf_test
>> +progs-y += test_overhead
>> +progs-y += test_cgrp2_array_pin
>> +progs-y += test_cgrp2_attach
>> +progs-y += test_cgrp2_sock
>> +progs-y += test_cgrp2_sock2
>> +progs-y += xdp1
>> +progs-y += xdp2
>> +progs-y += xdp_router_ipv4
>> +progs-y += test_current_task_under_cgroup
>> +progs-y += trace_event
>> +progs-y += sampleip
>> +progs-y += tc_l2_redirect
>> +progs-y += lwt_len_hist
>> +progs-y += xdp_tx_iptunnel
>> +progs-y += test_map_in_map
>> +progs-y += xdp_redirect_map
>> +progs-y += xdp_redirect_cpu
>> +progs-y += xdp_monitor
>> +progs-y += xdp_rxq_info
>> +progs-y += syscall_tp
>> +progs-y += cpustat
>> +progs-y += xdp_adjust_tail
>> +progs-y += xdpsock
>> +progs-y += xdp_fwd
>> +progs-y += task_fd_query
>> +progs-y += xdp_sample_pkts
>> +progs-y += ibumad
>> +progs-y += hbm
>>
>>   # Libbpf dependencies
>>   LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
>> @@ -111,7 +109,7 @@ ibumad-objs := bpf_load.o ibumad_user.o $(TRACE_HELPERS)
>>   hbm-objs := bpf_load.o hbm.o $(CGROUP_HELPERS)
>>
>>   # Tell kbuild to always build the programs
>> -always := $(hostprogs-y)
>> +always := $(progs-y)
>>   always += sockex1_kern.o
>>   always += sockex2_kern.o
>>   always += sockex3_kern.o
>> @@ -170,21 +168,6 @@ always += ibumad_kern.o
>>   always += hbm_out_kern.o
>>   always += hbm_edt_kern.o
>>
>> -KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include
>> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/bpf/
>> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
>> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/ -I$(srctree)/tools/include
>> -KBUILD_HOSTCFLAGS += -I$(srctree)/tools/perf
>> -
>> -HOSTCFLAGS_bpf_load.o += -Wno-unused-variable
>> -
>> -KBUILD_HOSTLDLIBS		+= $(LIBBPF) -lelf
>> -HOSTLDLIBS_tracex4		+= -lrt
>> -HOSTLDLIBS_trace_output	+= -lrt
>> -HOSTLDLIBS_map_perf_test	+= -lrt
>> -HOSTLDLIBS_test_overhead	+= -lrt
>> -HOSTLDLIBS_xdpsock		+= -pthread
>> -
>>   # Strip all expet -D options needed to handle linux headers
>>   # for arm it's __LINUX_ARM_ARCH__ and potentially others fork vars
>>   D_OPTIONS = $(shell echo "$(KBUILD_CFLAGS) " | sed 's/[[:blank:]]/\n/g' | \
>> @@ -194,6 +177,29 @@ ifeq ($(ARCH), arm)
>>   CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
>>   endif
>>
>> +ccflags-y += -I$(objtree)/usr/include
>> +ccflags-y += -I$(srctree)/tools/lib/bpf/
>> +ccflags-y += -I$(srctree)/tools/testing/selftests/bpf/
>> +ccflags-y += -I$(srctree)/tools/lib/
>> +ccflags-y += -I$(srctree)/tools/include
>> +ccflags-y += -I$(srctree)/tools/perf
>> +ccflags-y += $(D_OPTIONS)
>> +ccflags-y += -Wall
>> +ccflags-y += -fomit-frame-pointer
>> +ccflags-y += -Wmissing-prototypes
>> +ccflags-y += -Wstrict-prototypes
>> +
>> +PROGS_CFLAGS := $(ccflags-y)
>> +
>> +PROGCFLAGS_bpf_load.o += -Wno-unused-variable
>> +
>> +PROGS_LDLIBS			:= $(LIBBPF) -lelf
>> +PROGLDLIBS_tracex4		+= -lrt
>> +PROGLDLIBS_trace_output		+= -lrt
>> +PROGLDLIBS_map_perf_test	+= -lrt
>> +PROGLDLIBS_test_overhead	+= -lrt
>> +PROGLDLIBS_xdpsock		+= -pthread
>> +
>>   # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>>   #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>>   LLC ?= llc
>> @@ -284,6 +290,8 @@ $(obj)/hbm_out_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>   $(obj)/hbm.o: $(src)/hbm.h
>>   $(obj)/hbm_edt_kern.o: $(src)/hbm.h $(src)/hbm_kern.h
>>
>> +-include $(BPF_SAMPLES_PATH)/Makefile.prog
>> +
>>   # asm/sysreg.h - inline assembly used by it is incompatible with llvm.
>>   # But, there is no easy way to fix it, so just exclude it since it is
>>   # useless for BPF samples.
>>

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* Re: [PATCH bpf-next 10/11] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets
From: Ivan Khoronzhuk @ 2019-09-13 22:33 UTC (permalink / raw)
  To: Yonghong Song
  Cc: ast@kernel.org, daniel@iogearbox.net, davem@davemloft.net,
	jakub.kicinski@netronome.com, hawk@kernel.org,
	john.fastabend@gmail.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	clang-built-linux@googlegroups.com
In-Reply-To: <0ad42019-2614-b70c-f93e-527c136bba83@fb.com>

On Fri, Sep 13, 2019 at 09:43:22PM +0000, Yonghong Song wrote:
>
>
>On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
>> In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
>> correctly to build command, for instance when --sysroot is used or
>> external libraries are used, like -lelf, wich can be absent in
>> toolchain. This is used for samples/bpf cross-compiling allowing to
>> get elf lib from sysroot.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>   samples/bpf/Makefile   |  8 +++++++-
>>   tools/lib/bpf/Makefile | 11 ++++++++---
>>   2 files changed, 15 insertions(+), 4 deletions(-)
>
>Could you separate this patch into two?
>One of libbpf and another for samples.
>
>The subject 'libbpf: ...' is not entirely accurate.
Yes, ofc.
But there is too many patches already, but better a lot of small
changes then couple huge.

>
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 79c9aa41832e..4edc5232cfc1 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -186,6 +186,10 @@ ccflags-y += -I$(srctree)/tools/perf
>>   ccflags-y += $(D_OPTIONS)
>>   ccflags-y += -Wall
>>   ccflags-y += -fomit-frame-pointer
>> +
>> +EXTRA_CXXFLAGS := $(ccflags-y)
>> +
>> +# options not valid for C++
>>   ccflags-y += -Wmissing-prototypes
>>   ccflags-y += -Wstrict-prototypes
>>
>> @@ -252,7 +256,9 @@ clean:
>>
>>   $(LIBBPF): FORCE
>>   # Fix up variables inherited from Kbuild that tools/ build system won't like
>> -	$(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(BPF_SAMPLES_PATH)/../../ O=
>> +	$(MAKE) -C $(dir $@) RM='rm -rf' EXTRA_CFLAGS="$(PROGS_CFLAGS)" \
>> +		EXTRA_CXXFLAGS="$(EXTRA_CXXFLAGS)" LDFLAGS=$(PROGS_LDFLAGS) \
>> +		srctree=$(BPF_SAMPLES_PATH)/../../ O=
>>
>>   $(obj)/syscall_nrs.h:	$(obj)/syscall_nrs.s FORCE
>>   	$(call filechk,offsets,__SYSCALL_NRS_H__)
>> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
>> index c6f94cffe06e..bccfa556ef4e 100644
>> --- a/tools/lib/bpf/Makefile
>> +++ b/tools/lib/bpf/Makefile
>> @@ -94,6 +94,10 @@ else
>>     CFLAGS := -g -Wall
>>   endif
>>
>> +ifdef EXTRA_CXXFLAGS
>> +  CXXFLAGS := $(EXTRA_CXXFLAGS)
>> +endif
>> +
>>   ifeq ($(feature-libelf-mmap), 1)
>>     override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
>>   endif
>> @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
>>   $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
>>
>>   $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
>> -	$(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
>> -				    -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>> +	$(QUIET_LINK)$(CC) $(LDFLAGS) \
>> +		--shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
>> +		-Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
>>   	@ln -sf $(@F) $(OUTPUT)libbpf.so
>>   	@ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
>>
>> @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
>>   	$(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
>>
>>   $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
>> -	$(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
>> +	$(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
>>
>>   $(OUTPUT)libbpf.pc:
>>   	$(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
>>

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* Re: [bpf-next,v3] samples: bpf: add max_pckt_size option at xdp_adjust_tail
From: Yonghong Song @ 2019-09-13 22:34 UTC (permalink / raw)
  To: Daniel T. Lee, Daniel Borkmann, Alexei Starovoitov
  Cc: netdev@vger.kernel.org, bpf@vger.kernel.org
In-Reply-To: <20190911190218.22628-1-danieltimlee@gmail.com>



On 9/11/19 8:02 PM, Daniel T. Lee wrote:
> Currently, at xdp_adjust_tail_kern.c, MAX_PCKT_SIZE is limited
> to 600. To make this size flexible, a new map 'pcktsz' is added.
> 
> By updating new packet size to this map from the userland,
> xdp_adjust_tail_kern.o will use this value as a new max_pckt_size.
> 
> If no '-P <MAX_PCKT_SIZE>' option is used, the size of maximum packet
> will be 600 as a default.
> 
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> 
> ---
> Changes in v2:
>      - Change the helper to fetch map from 'bpf_map__next' to
>      'bpf_object__find_map_fd_by_name'.
>   
>   samples/bpf/xdp_adjust_tail_kern.c | 23 +++++++++++++++++++----
>   samples/bpf/xdp_adjust_tail_user.c | 28 ++++++++++++++++++++++------
>   2 files changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/samples/bpf/xdp_adjust_tail_kern.c b/samples/bpf/xdp_adjust_tail_kern.c
> index 411fdb21f8bc..d6d84ffe6a7a 100644
> --- a/samples/bpf/xdp_adjust_tail_kern.c
> +++ b/samples/bpf/xdp_adjust_tail_kern.c
> @@ -25,6 +25,13 @@
>   #define ICMP_TOOBIG_SIZE 98
>   #define ICMP_TOOBIG_PAYLOAD_SIZE 92
>   
> +struct bpf_map_def SEC("maps") pcktsz = {
> +	.type = BPF_MAP_TYPE_ARRAY,
> +	.key_size = sizeof(__u32),
> +	.value_size = sizeof(__u32),
> +	.max_entries = 1,
> +};

We have new map definition format like in
tools/testing/selftests/bpf/progs/bpf_flow.c.
But looks like most samples/bpf still use SEC("maps").
I guess we can leave it for now, and if needed,
later on a massive conversion for all samples/bpf/
bpf programs can be done.

> +
>   struct bpf_map_def SEC("maps") icmpcnt = {
>   	.type = BPF_MAP_TYPE_ARRAY,
>   	.key_size = sizeof(__u32),
> @@ -64,7 +71,8 @@ static __always_inline void ipv4_csum(void *data_start, int data_size,
>   	*csum = csum_fold_helper(*csum);
>   }
>   
> -static __always_inline int send_icmp4_too_big(struct xdp_md *xdp)
> +static __always_inline int send_icmp4_too_big(struct xdp_md *xdp,
> +					      __u32 max_pckt_size)
>   {
>   	int headroom = (int)sizeof(struct iphdr) + (int)sizeof(struct icmphdr);
>   
> @@ -92,7 +100,7 @@ static __always_inline int send_icmp4_too_big(struct xdp_md *xdp)
>   	orig_iph = data + off;
>   	icmp_hdr->type = ICMP_DEST_UNREACH;
>   	icmp_hdr->code = ICMP_FRAG_NEEDED;
> -	icmp_hdr->un.frag.mtu = htons(MAX_PCKT_SIZE-sizeof(struct ethhdr));
> +	icmp_hdr->un.frag.mtu = htons(max_pckt_size - sizeof(struct ethhdr));
>   	icmp_hdr->checksum = 0;
>   	ipv4_csum(icmp_hdr, ICMP_TOOBIG_PAYLOAD_SIZE, &csum);
>   	icmp_hdr->checksum = csum;
> @@ -118,14 +126,21 @@ static __always_inline int handle_ipv4(struct xdp_md *xdp)
>   {
>   	void *data_end = (void *)(long)xdp->data_end;
>   	void *data = (void *)(long)xdp->data;
> +	__u32 max_pckt_size = MAX_PCKT_SIZE;
> +	__u32 *pckt_sz;
> +	__u32 key = 0;

The above two new definitions may the code not in
reverse Christmas definition order, could you fix it?

>   	int pckt_size = data_end - data;
>   	int offset;
>   
> -	if (pckt_size > MAX_PCKT_SIZE) {
> +	pckt_sz = bpf_map_lookup_elem(&pcktsz, &key);
> +	if (pckt_sz && *pckt_sz)
> +		max_pckt_size = *pckt_sz;
> +
> +	if (pckt_size > max_pckt_size) {
>   		offset = pckt_size - ICMP_TOOBIG_SIZE;
>   		if (bpf_xdp_adjust_tail(xdp, 0 - offset))
>   			return XDP_PASS;

We could have the following scenario:
   max_pckt_size = 1
   pckt_size = 2
   offset = -96
   bpf_xdp_adjust_tail return -EINVAL
   so we return XDP_PASS now

Maybe you want to do
    if (pckt_size > max(max_pckt_size, ICMP_TOOBIG_SIZE)) {
       ...
    }
as in original code, bpf_xdp_adjust_tail(...) already succeeds.

> -		return send_icmp4_too_big(xdp);
> +		return send_icmp4_too_big(xdp, max_pckt_size);
>   	}
>   	return XDP_PASS;
>   }
> diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
> index a3596b617c4c..aef6c69a48a7 100644
> --- a/samples/bpf/xdp_adjust_tail_user.c
> +++ b/samples/bpf/xdp_adjust_tail_user.c
> @@ -23,6 +23,7 @@
>   #include "libbpf.h"
>   
>   #define STATS_INTERVAL_S 2U
> +#define MAX_PCKT_SIZE 600
>   
>   static int ifindex = -1;
>   static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> @@ -72,6 +73,7 @@ static void usage(const char *cmd)
>   	printf("Usage: %s [...]\n", cmd);
>   	printf("    -i <ifname|ifindex> Interface\n");
>   	printf("    -T <stop-after-X-seconds> Default: 0 (forever)\n");
> +	printf("    -P <MAX_PCKT_SIZE> Default: %u\n", MAX_PCKT_SIZE);
>   	printf("    -S use skb-mode\n");
>   	printf("    -N enforce native mode\n");
>   	printf("    -F force loading prog\n");
> @@ -85,13 +87,14 @@ int main(int argc, char **argv)
>   		.prog_type	= BPF_PROG_TYPE_XDP,
>   	};
>   	unsigned char opt_flags[256] = {};
> -	const char *optstr = "i:T:SNFh";
> +	const char *optstr = "i:T:P:SNFh";
>   	struct bpf_prog_info info = {};
>   	__u32 info_len = sizeof(info);
> +	__u32 max_pckt_size = 0;
> +	__u32 key = 0;
>   	unsigned int kill_after_s = 0;
>   	int i, prog_fd, map_fd, opt;
>   	struct bpf_object *obj;
> -	struct bpf_map *map;
>   	char filename[256];
>   	int err;
>   
> @@ -110,6 +113,9 @@ int main(int argc, char **argv)
>   		case 'T':
>   			kill_after_s = atoi(optarg);
>   			break;
> +		case 'P':
> +			max_pckt_size = atoi(optarg);
> +			break;
>   		case 'S':
>   			xdp_flags |= XDP_FLAGS_SKB_MODE;
>   			break;
> @@ -150,12 +156,22 @@ int main(int argc, char **argv)
>   	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
>   		return 1;
>   
> -	map = bpf_map__next(NULL, obj);
> -	if (!map) {
> -		printf("finding a map in obj file failed\n");
> +	/* update pcktsz map */
> +	if (max_pckt_size) {
> +		map_fd = bpf_object__find_map_fd_by_name(obj, "pcktsz");
> +		if (!map_fd) {

Let us test map_fd and below prog_fd with '< 0" instead of "!= 0'.
In this particular sample, "! = 0" is okay since we did not close
stdin. But in programs if stdin is closed, the fd 0 may be reused
for map_fd. Let us just keep good coding practice here.

> +			printf("finding a pcktsz map in obj file failed\n");
> +			return 1;
> +		}
> +		bpf_map_update_elem(map_fd, &key, &max_pckt_size, BPF_ANY);
> +	}
> +
> +	/* fetch icmpcnt map */
> +	map_fd = bpf_object__find_map_fd_by_name(obj, "icmpcnt");
> +	if (!map_fd) {
> +		printf("finding a icmpcnt map in obj file failed\n");
>   		return 1;
>   	}
> -	map_fd = bpf_map__fd(map);
>   
>   	if (!prog_fd) {
>   		printf("load_bpf_file: %s\n", strerror(errno));
> 

^ permalink raw reply

* Re: [PATCH bpf-next 11/11] samples: bpf: makefile: add sysroot support
From: Ivan Khoronzhuk @ 2019-09-13 22:36 UTC (permalink / raw)
  To: Yonghong Song
  Cc: ast@kernel.org, daniel@iogearbox.net, davem@davemloft.net,
	jakub.kicinski@netronome.com, hawk@kernel.org,
	john.fastabend@gmail.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	clang-built-linux@googlegroups.com
In-Reply-To: <e967744b-0286-d92f-9fc8-70f5759cc4a1@fb.com>

On Fri, Sep 13, 2019 at 09:45:31PM +0000, Yonghong Song wrote:
>
>
>On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
>> Basically it only enables that was added by previous couple fixes.
>> For sure, just make tools/include to be included after sysroot
>> headers.
>>
>> export ARCH=arm
>> export CROSS_COMPILE=arm-linux-gnueabihf-
>> make samples/bpf/ SYSROOT="path/to/sysroot"
>>
>> Sysroot contains correct libs installed and its headers ofc.
>> Useful when working with NFC or virtual machine.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>>   samples/bpf/Makefile   |  5 +++++
>>   samples/bpf/README.rst | 10 ++++++++++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 4edc5232cfc1..68ba78d1dbbe 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -177,6 +177,11 @@ ifeq ($(ARCH), arm)
>>   CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
>>   endif
>>
>> +ifdef SYSROOT
>> +ccflags-y += --sysroot=${SYSROOT}
>> +PROGS_LDFLAGS := -L${SYSROOT}/usr/lib
>> +endif
>> +
>>   ccflags-y += -I$(objtree)/usr/include
>>   ccflags-y += -I$(srctree)/tools/lib/bpf/
>>   ccflags-y += -I$(srctree)/tools/testing/selftests/bpf/
>> diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst
>> index 5f27e4faca50..786d0ab98e8a 100644
>> --- a/samples/bpf/README.rst
>> +++ b/samples/bpf/README.rst
>> @@ -74,3 +74,13 @@ samples for the cross target.
>>   export ARCH=arm64
>>   export CROSS_COMPILE="aarch64-linux-gnu-"
>>   make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>> +
>> +If need to use environment of target board (headers and libs), the SYSROOT
>> +also can be set, pointing on FS of target board:
>> +
>> +export ARCH=arm64
>> +export CROSS_COMPILE="aarch64-linux-gnu-"
>> +make samples/bpf/ SYSROOT=~/some_sdk/linux-devkit/sysroots/aarch64-linux-gnu
>> +
>> +Setting LLC and CLANG is not necessarily if it's installed on HOST and have
>> +in its targets appropriate arch triple (usually it has several arches).
>
>You have very good description about how to build and test in cover
>letter. Could you include those instructions here as well? This will
>help keep a record so later people can try/test if needed.

I will try.
Thanks!!!

-- 
Regards,
Ivan Khoronzhuk

^ permalink raw reply

* Re: [PATCH v4 2/2] tcp: Add snd_wnd to TCP_INFO
From: Yuchung Cheng @ 2019-09-13 22:41 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: Thomas Higdon, netdev@vger.kernel.org, Jonathan Lemon, Dave Jones,
	Eric Dumazet, Dave Taht, Soheil Hassas Yeganeh
In-Reply-To: <CADVnQy=aU=veBnZF=5OgwkT6EWA+hmmu8w9eq2d83eReSjAxEw@mail.gmail.com>

On Fri, Sep 13, 2019 at 2:53 PM Neal Cardwell <ncardwell@google.com> wrote:
>
> On Fri, Sep 13, 2019 at 5:29 PM Yuchung Cheng <ycheng@google.com> wrote:
> > > What if the comment is shortened up to fit in 80 columns and the units
> > > (bytes) are added, something like:
> > >
> > >         __u32   tcpi_snd_wnd;        /* peer's advertised recv window (bytes) */
> > just a thought: will tcpi_peer_rcv_wnd be more self-explanatory?
>
> Good suggestion. I'm on the fence about that one. By itself, I agree
> tcpi_peer_rcv_wnd sounds much more clear. But tcpi_snd_wnd has the
> virtue of matching both the kernel code (tp->snd_wnd) and RFC 793
> (SND.WND). So they both have pros and cons. Maybe someone else feels
> more strongly one way or the other.
no strong preference -- snd_wnd is just fine too with proper comment
(for consistency).

also it might be good to comment whether it is scaled or not. it may
not be obvious if the actual value and scale are small.
>
> neal

^ permalink raw reply

* Re: [PATCH bpf-next] libbpf: add xsk_umem__adjust_offset
From: Yonghong Song @ 2019-09-13 22:49 UTC (permalink / raw)
  To: Kevin Laatz, netdev@vger.kernel.org, ast@kernel.org,
	daniel@iogearbox.net, bjorn.topel@intel.com,
	magnus.karlsson@intel.com, jonathan.lemon@gmail.com
  Cc: bruce.richardson@intel.com, ciara.loftus@intel.com,
	bpf@vger.kernel.org
In-Reply-To: <20190912072840.20947-1-kevin.laatz@intel.com>



On 9/12/19 8:28 AM, Kevin Laatz wrote:
> Currently, xsk_umem_adjust_offset exists as a kernel internal function.
> This patch adds xsk_umem__adjust_offset to libbpf so that it can be used
> from userspace. This will take the responsibility of properly storing the
> offset away from the application, making it less error prone.
> 
> Since xsk_umem__adjust_offset is called on a per-packet basis, we need to
> inline the function to avoid any performance regressions.  In order to
> inline xsk_umem__adjust_offset, we need to add it to xsk.h. Unfortunately
> this means that we can't dereference the xsk_umem_config struct directly
> since it is defined only in xsk.c. We therefore add an extra API to return
> the flags field to the user from the structure, and have the inline
> function use this flags field directly.

Could you add an example (either in samples/bpf or in 
tools/testing/selftests/bpf) to use xsk_umem__adjust_offset()? This will 
make it
easier to check whether the interface is appropriate or not and
what is the performance implication as you stated in the above.

> 
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> ---
>   tools/lib/bpf/libbpf.map |  1 +
>   tools/lib/bpf/xsk.c      |  5 +++++
>   tools/lib/bpf/xsk.h      | 14 ++++++++++++++
>   3 files changed, 20 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index d04c7cb623ed..760350c9b81c 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -189,4 +189,5 @@ LIBBPF_0.0.4 {
>   LIBBPF_0.0.5 {
>   	global:
>   		bpf_btf_get_next_id;
> +		xsk_umem__get_flags;
>   } LIBBPF_0.0.4;
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index 842c4fd55859..a4250a721ea6 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -84,6 +84,11 @@ int xsk_socket__fd(const struct xsk_socket *xsk)
>   	return xsk ? xsk->fd : -EINVAL;
>   }
>   
> +__u32 xsk_umem__get_flags(struct xsk_umem *umem)
> +{
> +	return umem->config.flags;
> +}
> +
>   static bool xsk_page_aligned(void *buffer)
>   {
>   	unsigned long addr = (unsigned long)buffer;
> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> index 584f6820a639..bf782facb274 100644
> --- a/tools/lib/bpf/xsk.h
> +++ b/tools/lib/bpf/xsk.h
> @@ -183,8 +183,22 @@ static inline __u64 xsk_umem__add_offset_to_addr(__u64 addr)
>   	return xsk_umem__extract_addr(addr) + xsk_umem__extract_offset(addr);
>   }
>   
> +/* Handle the offset appropriately depending on aligned or unaligned mode.
> + * For unaligned mode, we store the offset in the upper 16-bits of the address.
> + * For aligned mode, we simply add the offset to the address.
> + */
> +static inline __u64 xsk_umem__adjust_offset(__u32 umem_flags, __u64 addr,
> +					    __u64 offset)
> +{
> +	if (umem_flags & XDP_UMEM_UNALIGNED_CHUNK_FLAG)
> +		return addr + (offset << XSK_UNALIGNED_BUF_OFFSET_SHIFT);
> +	else
> +		return addr + offset;
> +}
> +
>   LIBBPF_API int xsk_umem__fd(const struct xsk_umem *umem);
>   LIBBPF_API int xsk_socket__fd(const struct xsk_socket *xsk);
> +LIBBPF_API __u32 xsk_umem__get_flags(struct xsk_umem *umem);
>   
>   #define XSK_RING_CONS__DEFAULT_NUM_DESCS      2048
>   #define XSK_RING_PROD__DEFAULT_NUM_DESCS      2048
> 

^ permalink raw reply

* [PATCH v2] net: mdio: switch to using gpiod_get_optional()
From: Dmitry Torokhov @ 2019-09-13 22:55 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit
  Cc: David S. Miller, Linus Walleij, Andy Shevchenko, netdev,
	linux-kernel

The MDIO device reset line is optional and now that gpiod_get_optional()
returns proper value when GPIO support is compiled out, there is no
reason to use fwnode_get_named_gpiod() that I plan to hide away.

Let's switch to using more standard gpiod_get_optional() and
gpiod_set_consumer_name() to keep the nice "PHY reset" label.

Also there is no reason to only try to fetch the reset GPIO when we have
OF node, gpiolib can fetch GPIO data from firmwares as well.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Note this is an update to a patch titled "[PATCH 05/11] net: mdio:
switch to using fwnode_gpiod_get_index()" that no longer uses the new
proposed API and instead works with already existing ones.

 drivers/net/phy/mdio_bus.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index ce940871331e..2e29ab841b4d 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -42,21 +42,17 @@
 
 static int mdiobus_register_gpiod(struct mdio_device *mdiodev)
 {
-	struct gpio_desc *gpiod = NULL;
+	int error;
 
 	/* Deassert the optional reset signal */
-	if (mdiodev->dev.of_node)
-		gpiod = fwnode_get_named_gpiod(&mdiodev->dev.of_node->fwnode,
-					       "reset-gpios", 0, GPIOD_OUT_LOW,
-					       "PHY reset");
-	if (IS_ERR(gpiod)) {
-		if (PTR_ERR(gpiod) == -ENOENT || PTR_ERR(gpiod) == -ENOSYS)
-			gpiod = NULL;
-		else
-			return PTR_ERR(gpiod);
-	}
-
-	mdiodev->reset_gpio = gpiod;
+	mdiodev->reset_gpio = gpiod_get_optional(&mdiodev->dev,
+						 "reset", GPIOD_OUT_LOW);
+	error = PTR_ERR_OR_ZERO(mdiodev->reset_gpio);
+	if (error)
+		return error;
+
+	if (mdiodev->reset_gpio)
+		gpiod_set_consumer_name(mdiodev->reset_gpio, "PHY reset");
 
 	return 0;
 }
-- 
2.23.0.237.gc6a4ce50a0-goog


-- 
Dmitry

^ permalink raw reply related


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