Netdev List
 help / color / mirror / Atom feed
* Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()
From: Cong Wang @ 2018-06-10 19:43 UTC (permalink / raw)
  To: David Miller
  Cc: Linux Kernel Network Developers, shankarapailoor, Tetsuo Handa,
	Lorenzo Colitti, Al Viro
In-Reply-To: <20180610.122704.2150654250277031669.davem@davemloft.net>

On Sun, Jun 10, 2018 at 12:27 PM, David Miller <davem@davemloft.net> wrote:
> I'm applying this for now, it is at least a step towards fixing
> all of these issues.
>
> If it is really offensive, I can revert, just tell me.

Thanks, David!

Unless there is something fundamentally broken, there is no
reason to revert it. The only risk here is some possible deadlock,
but I am ready to fix any deadlock report caused by this
patch. :)

^ permalink raw reply

* [PATCH net] ipv6: allow PMTU exceptions to local routes
From: Julian Anastasov @ 2018-06-10 23:02 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Martin KaFai Lau, kernel-team, lvs-devel

IPVS setups with local client and remote tunnel server need
to create exception for the local virtual IP. What we do is to
change PMTU from 64KB (on "lo") to 1460 in the common case.

Suggested-by: Martin KaFai Lau <kafai@fb.com>
Fixes: 45e4fd26683c ("ipv6: Only create RTF_CACHE routes after encountering pmtu exception")
Fixes: 7343ff31ebf0 ("ipv6: Don't create clones of host routes.")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
 net/ipv6/route.c | 3 ---
 1 file changed, 3 deletions(-)

Note: I failed to build 2.6.38 kernel for the test but I think
commit 7343ff31ebf0 looks as the one that added the restriction
to change PMTU for local IPs.

So, I'm not sure from how long time the issue with local IPVS
clients and tunnelling method exists. May be it worked between
2.6.28 and 2.6.37.

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index fb95698..86a0e43 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2307,9 +2307,6 @@ static void __ip6_rt_update_pmtu(struct dst_entry *dst, const struct sock *sk,
 	const struct in6_addr *daddr, *saddr;
 	struct rt6_info *rt6 = (struct rt6_info *)dst;
 
-	if (rt6->rt6i_flags & RTF_LOCAL)
-		return;
-
 	if (dst_metric_locked(dst, RTAX_MTU))
 		return;
 
-- 
2.9.5

^ permalink raw reply related

* [PATCH net-next] vlan: implement vlan id and protocol changes
From: Chas Williams @ 2018-06-10 23:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Charles (Chas) Williams, Chas Williams

From: "Charles (Chas) Williams" <ciwillia@mail.eng.vyatta.net>

vlan_changelink silently ignores attempts to change the vlan id
or protocol id of an existing vlan interface.  Implement by adding
the new vlan id and protocol to the interface's vlan group and then
removing the old vlan id and protocol from the vlan group.

Signed-off-by: Chas Williams <3chas3@gmail.com>
---
 include/linux/netdevice.h |  1 +
 net/8021q/vlan.c          |  4 ++--
 net/8021q/vlan.h          |  2 ++
 net/8021q/vlan_netlink.c  | 38 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3ec9850c7936..a95ae238addf 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2409,6 +2409,7 @@ enum netdev_cmd {
 	NETDEV_CVLAN_FILTER_DROP_INFO,
 	NETDEV_SVLAN_FILTER_PUSH_INFO,
 	NETDEV_SVLAN_FILTER_DROP_INFO,
+	NETDEV_CHANGEVLAN,
 };
 const char *netdev_cmd_to_name(enum netdev_cmd cmd);
 
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 73a65789271b..b5e0ad1a581a 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -51,8 +51,8 @@ const char vlan_version[] = DRV_VERSION;
 
 /* End of global variables definitions. */
 
-static int vlan_group_prealloc_vid(struct vlan_group *vg,
-				   __be16 vlan_proto, u16 vlan_id)
+int vlan_group_prealloc_vid(struct vlan_group *vg,
+			    __be16 vlan_proto, u16 vlan_id)
 {
 	struct net_device **array;
 	unsigned int pidx, vidx;
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 44df1c3df02d..c734dd21d70d 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -116,6 +116,8 @@ int register_vlan_dev(struct net_device *dev, struct netlink_ext_ack *extack);
 void unregister_vlan_dev(struct net_device *dev, struct list_head *head);
 bool vlan_dev_inherit_address(struct net_device *dev,
 			      struct net_device *real_dev);
+int vlan_group_prealloc_vid(struct vlan_group *vg,
+			    __be16 vlan_proto, u16 vlan_id);
 
 static inline u32 vlan_get_ingress_priority(struct net_device *dev,
 					    u16 vlan_tci)
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 9b60c1e399e2..0e59babe6651 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -107,10 +107,48 @@ static int vlan_changelink(struct net_device *dev, struct nlattr *tb[],
 			   struct nlattr *data[],
 			   struct netlink_ext_ack *extack)
 {
+	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
 	struct ifla_vlan_flags *flags;
 	struct ifla_vlan_qos_mapping *m;
 	struct nlattr *attr;
 	int rem;
+	int err;
+	__be16 vlan_proto = vlan->vlan_proto;
+	u16 vlan_id = vlan->vlan_id;
+
+	if (data[IFLA_VLAN_ID])
+		vlan_id = nla_get_u16(data[IFLA_VLAN_ID]);
+
+	if (data[IFLA_VLAN_PROTOCOL])
+		vlan_proto = nla_get_be16(data[IFLA_VLAN_PROTOCOL]);
+
+	if (vlan->vlan_id != vlan_id || vlan->vlan_proto != vlan_proto) {
+		struct net_device *real_dev = vlan->real_dev;
+		struct vlan_info *vlan_info;
+		struct vlan_group *grp;
+		__be16 old_vlan_proto = vlan->vlan_proto;
+		u16 old_vlan_id = vlan->vlan_id;
+
+		err = vlan_vid_add(real_dev, vlan_proto, vlan_id);
+		if (err)
+			return err;
+		vlan_info = rtnl_dereference(real_dev->vlan_info);
+		grp = &vlan_info->grp;
+		err = vlan_group_prealloc_vid(grp, vlan_proto, vlan_id);
+		if (err < 0) {
+			vlan_vid_del(real_dev, vlan_proto, vlan_id);
+			return err;
+		}
+		vlan_group_set_device(grp, vlan_proto, vlan_id, dev);
+		vlan->vlan_proto = vlan_proto;
+		vlan->vlan_id = vlan_id;
+
+		vlan_group_set_device(grp, old_vlan_proto, old_vlan_id, NULL);
+		vlan_vid_del(real_dev, old_vlan_proto, old_vlan_id);
+
+		err = call_netdevice_notifiers(NETDEV_CHANGEVLAN, dev);
+		notifier_to_errno(err);
+	}
 
 	if (data[IFLA_VLAN_FLAGS]) {
 		flags = nla_data(data[IFLA_VLAN_FLAGS]);
-- 
2.14.3

^ permalink raw reply related

* Re: [mmotm:master 192/212] include/uapi/asm-generic/int-ll64.h:20:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'typedef'
From: Randy Dunlap @ 2018-06-11  0:01 UTC (permalink / raw)
  To: kbuild test robot, Andrew Morton
  Cc: kbuild-all, Linux Memory Management List, Souptick Joarder,
	Johannes Weiner, LKML, netdev@vger.kernel.org
In-Reply-To: <201806081045.eZrs5GGH%fengguang.wu@intel.com>

On 06/07/2018 07:06 PM, kbuild test robot wrote:
> tree:   git://git.cmpxchg.org/linux-mmotm.git master
> head:   7393732bae530daa27567988b91d16ecfeef6c62
> commit: b1a8bfbadbcb79644ccdd5f9cd370caa63cb1fa7 [192/212] linux-next-git-rejects
> config: i386-randconfig-s0-201822-CONFIG_DEBUG_INFO_REDUCED (attached as .config)
> compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
> reproduce:
>         git checkout b1a8bfbadbcb79644ccdd5f9cd370caa63cb1fa7
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from include/asm-generic/int-ll64.h:11:0,
>                     from include/uapi/asm-generic/types.h:7,
>                     from arch/x86/include/uapi/asm/types.h:5,
>                     from include/uapi/linux/types.h:5,
>                     from include/linux/types.h:6,
>                     from net/ipv4/ipconfig.c:36:
>>> include/uapi/asm-generic/int-ll64.h:20:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'typedef'
>     typedef __signed__ char __s8;
>     ^~~~~~~

The problem here was that net/ipv4/ipconfig.c begins like this:

q// SPDX-License-Identifier: GPL-2.0

Did anyone fix that?


-- 
~Randy

^ permalink raw reply

* Re: [PATCH net-next] vlan: implement vlan id and protocol changes
From: kbuild test robot @ 2018-06-11  0:15 UTC (permalink / raw)
  To: Chas Williams
  Cc: kbuild-all, davem, netdev, Charles (Chas) Williams, Chas Williams
In-Reply-To: <20180610231912.1543-1-3chas3@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2940 bytes --]

Hi Charles,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Chas-Williams/vlan-implement-vlan-id-and-protocol-changes/20180611-072123
config: parisc-c3000_defconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=parisc 

All warnings (new ones prefixed by >>):

   net/core/dev.c: In function 'netdev_cmd_to_name':
>> net/core/dev.c:1580:2: warning: enumeration value 'NETDEV_CHANGEVLAN' not handled in switch [-Wswitch]
     switch (cmd) {
     ^~~~~~

vim +/NETDEV_CHANGEVLAN +1580 net/core/dev.c

56f5aa77 Michael Chan 2017-12-16  1574  
ede2762d Kirill Tkhai 2018-03-23  1575  const char *netdev_cmd_to_name(enum netdev_cmd cmd)
ede2762d Kirill Tkhai 2018-03-23  1576  {
ede2762d Kirill Tkhai 2018-03-23  1577  #define N(val) 						\
ede2762d Kirill Tkhai 2018-03-23  1578  	case NETDEV_##val:				\
ede2762d Kirill Tkhai 2018-03-23  1579  		return "NETDEV_" __stringify(val);
ede2762d Kirill Tkhai 2018-03-23 @1580  	switch (cmd) {
ede2762d Kirill Tkhai 2018-03-23  1581  	N(UP) N(DOWN) N(REBOOT) N(CHANGE) N(REGISTER) N(UNREGISTER)
ede2762d Kirill Tkhai 2018-03-23  1582  	N(CHANGEMTU) N(CHANGEADDR) N(GOING_DOWN) N(CHANGENAME) N(FEAT_CHANGE)
ede2762d Kirill Tkhai 2018-03-23  1583  	N(BONDING_FAILOVER) N(PRE_UP) N(PRE_TYPE_CHANGE) N(POST_TYPE_CHANGE)
ede2762d Kirill Tkhai 2018-03-23  1584  	N(POST_INIT) N(RELEASE) N(NOTIFY_PEERS) N(JOIN) N(CHANGEUPPER)
ede2762d Kirill Tkhai 2018-03-23  1585  	N(RESEND_IGMP) N(PRECHANGEMTU) N(CHANGEINFODATA) N(BONDING_INFO)
ede2762d Kirill Tkhai 2018-03-23  1586  	N(PRECHANGEUPPER) N(CHANGELOWERSTATE) N(UDP_TUNNEL_PUSH_INFO)
ede2762d Kirill Tkhai 2018-03-23  1587  	N(UDP_TUNNEL_DROP_INFO) N(CHANGE_TX_QUEUE_LEN)
9daae9bd Gal Pressman 2018-03-28  1588  	N(CVLAN_FILTER_PUSH_INFO) N(CVLAN_FILTER_DROP_INFO)
9daae9bd Gal Pressman 2018-03-28  1589  	N(SVLAN_FILTER_PUSH_INFO) N(SVLAN_FILTER_DROP_INFO)
3f5ecd8a Kirill Tkhai 2018-04-26  1590  	}
ede2762d Kirill Tkhai 2018-03-23  1591  #undef N
ede2762d Kirill Tkhai 2018-03-23  1592  	return "UNKNOWN_NETDEV_EVENT";
ede2762d Kirill Tkhai 2018-03-23  1593  }
ede2762d Kirill Tkhai 2018-03-23  1594  EXPORT_SYMBOL_GPL(netdev_cmd_to_name);
ede2762d Kirill Tkhai 2018-03-23  1595  

:::::: The code at line 1580 was first introduced by commit
:::::: ede2762d93ff16e0974f7446516b46b1022db213 net: Make NETDEV_XXX commands enum { }

:::::: TO: Kirill Tkhai <ktkhai@virtuozzo.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 14440 bytes --]

^ permalink raw reply

* [GIT] Networking
From: David Miller @ 2018-06-11  0:42 UTC (permalink / raw)
  To: torvalds; +Cc: akpm, netdev, linux-kernel


1) For several bpfilter/UMH bugs, in particular make the UMH build not
   depend upon X86 specific Kconfig symbols.  From Alexei Starovoitov.

2) Fix handling of modified context pointer in bpf verifier, from
   Daniel Borkmann.

3) Kill regression in ifdown/ifup sequences for hv_netvsc driver, from
   Dexuan Cui.

4) When the bonding primary member name changes, we have to
   re-evaluate the bond->force_primary setting, from Xiangning Yu.

5) Eliminate possible padding beyone end of SKB in cdc_ncm driver,
   from Bjørn Mork.

6) RX queue length reported for UDP sockets in procfs and socket diag
   are inaccurate, from Paolo Abeni.

7) Fix br_fdb_find_port() locking, from Petr Machata.

8) Limit sk_rcvlowat values properly in TCP, from Soheil Hassas
   Yeganeh.

Please pull, thanks a lot!

The following changes since commit 3036bc45364f98515a2c446d7fac2c34dcfbeff4:

  Merge tag 'media/v4.18-2' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media (2018-06-07 12:34:37 -0700)

are available in the git repository at:

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

for you to fetch changes up to 867f816badc01e6da655028810d468c9f935b37c:

  tcp: limit sk_rcvlowat by the maximum receive buffer (2018-06-10 14:12:50 -0700)

----------------------------------------------------------------
Alexei Starovoitov (3):
      umh: fix race condition
      bpfilter: fix OUTPUT_FORMAT
      bpfilter: fix race in pipe access

Alvaro Gamez Machado (1):
      net: phy: dp83822: use BMCR_ANENABLE instead of BMSR_ANEGCAPABLE for DP83620

Bjørn Mork (1):
      cdc_ncm: avoid padding beyond end of skb

Colin Ian King (1):
      net: aquantia: fix unsigned numvecs comparison with less than zero

Cong Wang (1):
      socket: close race condition between sock_close() and sockfs_setattr()

Corentin Labbe (1):
      net: stmmac: fix build failure due to missing COMMON_CLK dependency

Daniel Borkmann (2):
      bpf: reject passing modified ctx to helper functions
      bpf, xdp: fix crash in xdp_umem_unaccount_pages

David S. Miller (1):
      Merge git://git.kernel.org/.../bpf/bpf

Davide Caratti (1):
      net/sched: act_simple: fix parsing of TCA_DEF_DATA

Dexuan Cui (1):
      hv_netvsc: Fix a network regression after ifdown/ifup

Geert Uytterhoeven (2):
      net: mscc: ocelot: Fix uninitialized error in ocelot_netdevice_event()
      xsk: Fix umem fill/completion queue mmap on 32-bit

Pablo Neira Ayuso (1):
      netfilter: nf_tables: add NFT_LOGLEVEL_* enumeration and use it

Paolo Abeni (1):
      udp: fix rx queue len reported by diag and proc interface

Petr Machata (1):
      net: bridge: Fix locking in br_fdb_find_port()

Soheil Hassas Yeganeh (1):
      tcp: limit sk_rcvlowat by the maximum receive buffer

Sultan Alsawaf (1):
      ip_tunnel: Fix name string concatenate in __ip_tunnel_create()

Willem de Bruijn (1):
      net: in virtio_net_hdr only add VLAN_HLEN to csum_start if payload holds vlan

Xiangning Yu (1):
      bonding: re-evaluate force_primary when the primary slave name changes

Yonghong Song (1):
      tools/bpf: fix selftest get_cgroup_id_user

YueHaibing (1):
      net: fddi: fix a possible null-ptr-deref

 arch/um/drivers/vector_transports.c                  |  3 ++-
 drivers/net/bonding/bond_options.c                   |  1 +
 drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 11 +++++------
 drivers/net/ethernet/mscc/ocelot.c                   |  2 +-
 drivers/net/ethernet/stmicro/stmmac/Kconfig          | 10 +++++-----
 drivers/net/fddi/skfp/skfddi.c                       | 55 ++++++++++++++++++++++++++++---------------------------
 drivers/net/hyperv/netvsc_drv.c                      |  4 +++-
 drivers/net/phy/dp83848.c                            | 35 +++++++++++++++++++++++++++++------
 drivers/net/tap.c                                    |  5 ++++-
 drivers/net/tun.c                                    |  3 ++-
 drivers/net/usb/cdc_ncm.c                            |  4 ++--
 drivers/net/virtio_net.c                             |  3 ++-
 include/linux/virtio_net.h                           | 11 ++++-------
 include/net/transp_v6.h                              | 11 +++++++++--
 include/net/udp.h                                    |  5 +++++
 include/uapi/linux/if_xdp.h                          |  4 ++--
 include/uapi/linux/netfilter/nf_tables.h             | 28 +++++++++++++++++++++++++---
 kernel/bpf/verifier.c                                | 48 +++++++++++++++++++++++++++++++-----------------
 kernel/umh.c                                         |  3 +--
 net/bpfilter/Makefile                                |  2 +-
 net/bpfilter/bpfilter_kern.c                         | 10 +++++++---
 net/bridge/br_fdb.c                                  |  4 +++-
 net/ipv4/ip_tunnel.c                                 |  4 ++--
 net/ipv4/tcp.c                                       | 12 +++++++-----
 net/ipv4/udp.c                                       |  2 +-
 net/ipv4/udp_diag.c                                  |  2 +-
 net/ipv6/datagram.c                                  |  6 +++---
 net/ipv6/udp.c                                       |  3 ++-
 net/netfilter/nft_log.c                              | 10 +++++-----
 net/packet/af_packet.c                               |  4 ++--
 net/sched/act_simple.c                               | 15 ++++++---------
 net/socket.c                                         | 18 +++++++++++++++---
 net/xdp/xdp_umem.c                                   |  6 ++++--
 net/xdp/xsk.c                                        |  2 +-
 tools/testing/selftests/bpf/get_cgroup_id_kern.c     | 14 +++++++++++++-
 tools/testing/selftests/bpf/get_cgroup_id_user.c     | 12 ++++++++++--
 tools/testing/selftests/bpf/test_verifier.c          | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 37 files changed, 301 insertions(+), 129 deletions(-)

^ permalink raw reply

* Re: [PATCH v6 net] stmmac: strip all VLAN tag types when kernel 802.1Q support is selected
From: Toshiaki Makita @ 2018-06-11  0:50 UTC (permalink / raw)
  To: David Miller, eladv6
  Cc: Jose.Abreu, f.fainelli, netdev, peppe.cavallaro, alexandre.torgue
In-Reply-To: <20180610.122918.824988605265684146.davem@davemloft.net>

On 2018/06/11 4:29, David Miller wrote:
> From: Elad Nachman <eladv6@gmail.com>
> Date: Fri, 8 Jun 2018 12:19:29 +0300
> 
>> stmmac reception handler calls stmmac_rx_vlan() to strip the vlan before 
>> calling napi_gro_receive().
>>
>> The function assumes VLAN tagged frames are always tagged with 
>> 802.1Q protocol, and assigns ETH_P_8021Q to the skb by hard-coding
>> the parameter on call to __vlan_hwaccel_put_tag() .
>>
>> This causes packets not to be passed to the VLAN slave if it was created 
>> with 802.1AD protocol
>> (ip link add link eth0 eth0.100 type vlan proto 802.1ad id 100).
>>
>> This fix passes the protocol from the VLAN header into 
>> __vlan_hwaccel_put_tag() instead of using the hard-coded value of
>> ETH_P_8021Q.
>>
>> NETIF_F_HW_VLAN_CTAG_RX check was removed and instead the strip action 
>> is dependent upon a preprocessor define which is defined when 802.1Q 
>> support is selected in the kernel config. 
>>
>> NETIF_F_HW_VLAN_STAG_RX feature was added to be in line with the driver 
>> actual abilities.
>>
>> Signed-off-by: Elad Nachman <eladn@gilat.com>
> 
> You can't remove the NETIF_F_* checks.
> 
> If the user doesn't have VLAN offloading enabled, the VLAN tags should
> be left in the packet as-is.
> 
> It can't be controlled by a CPP check.

David, NETIF_F_HW_VALN_*_RX is not user-controllable in this driver
because hw_features does not include them.

But this can break when those features are added to hw_features so if
David does not like this situation I think we need the condition anyway.

-- 
Toshiaki Makita

^ permalink raw reply

* Re: Grant-
From: M. M. Fridman @ 2018-06-09 14:04 UTC (permalink / raw)
  To: Recipients

I Mikhail Fridman. has selected you specially as one of my beneficiaries
for my Charitable Donation, Just as I have declared on May 23, 2016 to give
my fortune as charity.

Check the link below for confirmation:

http://www.ibtimes.co.uk/russias-second-wealthiest-man-mikhail-fridman-plans-leaving-14-2bn-fortune-charity-1561604

Reply as soon as possible with further directives.

Best Regards,
Mikhail Fridman.

^ permalink raw reply

* Re: [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support
From: Michael S. Tsirkin @ 2018-06-11  2:12 UTC (permalink / raw)
  To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <23efe110-f61a-5aee-c0b4-bd3dc5426438@redhat.com>

On Fri, Jun 08, 2018 at 01:07:09PM +0800, Jason Wang wrote:
> 
> 
> On 2018年06月08日 12:46, Michael S. Tsirkin wrote:
> > On Fri, Jun 08, 2018 at 11:50:42AM +0800, Jason Wang wrote:
> > > This feature bit is duplicated with VIRTIO_F_ANY_LAYOUT, this means if
> > > a userpsace want to enable VRITIO_F_ANY_LAYOUT,
> > > VHOST_NET_F_VIRTIO_NET_HDR will be implied too. This is wrong and will
> > > break networking.
> > What breaks networking exactly? VHOST_NET supported ANY_LAYOUT
> > from day one. For this reason it does not need to know about
> > VRITIO_F_ANY_LAYOUT and we reused the bit for other purposes.
> 
> It's the knowledge of vhost_net code it self but not userspace. For
> userspace, it should depends on the value of returned by VHOST_GET_FEATURES.
> So when userspace can set_features with ANY_LAYOUT, vhost may think it wants
> VHOST_NET_F_VIRTIO_NET_HDR.

Yes but that's the admittedly ugly API that we have now.
userspace is supposed to know VRITIO_F_ANY_LAYOUT does
not make sense for vhost.



> > 
> > 
> > 
> > > Fixing this by safely removing
> > > VHOST_NET_F_VIRTIO_NET_HDR support. There should be very few or even
> > > no userspace can use this.
> > Quite possibly, but it is hard to be sure. It seems safer to
> > maintain it unless there's an actual reason something's broken.
> 
> I think not since the feature is negotiated not mandatory?

That doesn't mean much.

> > 
> > > Further cleanups could be done for
> > > -net-next for safety.
> > > 
> > > In the future, we need a vhost dedicated feature set/get ioctl()
> > > instead of reusing virtio ones.
> > Not just in the future, we might want to switch iommu
> > to a sane structure without the 64 bit padding bug
> > right now.
> 
> Yes, I hit this bug when introducing V2 of msg IOTLB message.

Sounds good, so if you like, reserve a bit for
VHOST_NET_F_VIRTIO_NET_HDR in the new ioctl mask and
do not enable it there.

> > 
> > > Fixes: 4e9fa50c6ccbe ("vhost: move features to core")
> > This tag makes no sense here IMHO. Looks like people are using some tool
> > that just looks at the earliest version where patch won't apply. The
> > commit in question just moved some code around.
> 
> Looks not, before this commit, vhost_net won't return ANY_LAYOUT.
> 
> Thanks

Well ANY_LAYOUT just happens to be same as VHOST_NET_F_VIRTIO_NET_HDR
and that has been set since forever.

> > 
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > ---
> > >   drivers/vhost/net.c | 15 +++++----------
> > >   1 file changed, 5 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > index 986058a..83eef52 100644
> > > --- a/drivers/vhost/net.c
> > > +++ b/drivers/vhost/net.c
> > > @@ -69,7 +69,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
> > >   enum {
> > >   	VHOST_NET_FEATURES = VHOST_FEATURES |
> > > -			 (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
> > >   			 (1ULL << VIRTIO_NET_F_MRG_RXBUF) |
> > >   			 (1ULL << VIRTIO_F_IOMMU_PLATFORM)
> > >   };
> > > @@ -1255,15 +1254,11 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
> > >   			       (1ULL << VIRTIO_F_VERSION_1))) ?
> > >   			sizeof(struct virtio_net_hdr_mrg_rxbuf) :
> > >   			sizeof(struct virtio_net_hdr);
> > > -	if (features & (1 << VHOST_NET_F_VIRTIO_NET_HDR)) {
> > > -		/* vhost provides vnet_hdr */
> > > -		vhost_hlen = hdr_len;
> > > -		sock_hlen = 0;
> > > -	} else {
> > > -		/* socket provides vnet_hdr */
> > > -		vhost_hlen = 0;
> > > -		sock_hlen = hdr_len;
> > > -	}
> > > +
> > > +        /* socket provides vnet_hdr */
> > > +	vhost_hlen = 0;
> > > +	sock_hlen = hdr_len;
> > > +
> > >   	mutex_lock(&n->dev.mutex);
> > >   	if ((features & (1 << VHOST_F_LOG_ALL)) &&
> > >   	    !vhost_log_access_ok(&n->dev))
> > > -- 
> > > 2.7.4
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: netdevice notifier and device private data
From: Michael Richardson @ 2018-06-11  2:09 UTC (permalink / raw)
  To: Alexander Aring; +Cc: netdev, linux-wpan, linux-bluetooth
In-Reply-To: <20180610153956.mgzigzfne6shjb4s@x220t>

[-- Attachment #1: Type: text/plain, Size: 1502 bytes --]


Alexander Aring <aring@mojatatu.com> wrote:
    >> It totally seems like broken behaviour.  Maybe it's not even
    >> intentional.  Maybe they are just foobar.

    > They simple don't know what they doing... somebody thought 6LoWPAN need
    > to be 6LoWPAN, but they actually don't use the 6LoWPAN handling inside
    > the kernel. _Except_ they doing out of tree stuff which I don't
    > believe.

So, it seems like this ioctl() should be disabled, or restricted to cases
that actually work.  hate to break their code, but if it's broken anyway, at
least the kernel won't crash under them.

    > According to [0] it also works with tun default (I suppsoe raw IPv6),
    > because ifdef. And they should not change it because they don't use
    > in-kernel 6LoWPAN functionality.

    > I really think that this tun/tap feature makes a lot of trouble for
    > some type changes. I probably introduce lowpan_dev pointer to netdevice
    > and then check if it's really a 6LoPWAN interface, a dev->type will not
    > garantuee anymore you have a 6LoWPAN interface. At least in user space
    > it's not possible to have a check if you really have a 6LoWPAN
    > interface.

    > - Alex

    > [0]
    > https://github.com/openthread/wpantund/blob/master/src/util/tunnel.c#L180
    > [1] https://github.com/reubenhwk/radvd/blob/master/device-linux.c#L75

-- 
Michael Richardson <mcr+IETF@sandelman.ca>, Sandelman Software Works
 -= IPv6 IoT consulting =-




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 464 bytes --]

^ permalink raw reply

* [iproute2-next v2 2/2] tipc: JSON support for tipc link printouts
From: Hoang Le @ 2018-06-11  2:17 UTC (permalink / raw)
  To: netdev, tipc-discussion
In-Reply-To: <1528683420-15006-1-git-send-email-hoang.h.le@dektech.com.au>

Add json output support for tipc link command

Example output:
$tipc -j -p link list

[ {
        "broadcast-link": "up",
        "1.1.1:bridge-1.1.104:eth0": "up",
        "1.1.1:bridge-1.1.105:eth0": "up",
        "1.1.1:bridge-1.1.106:eth0": "up"
    } ]

--------------------
$tipc -j -p link stat show link broadcast-link

[ {
        "link": "broadcast-link",
        "window": 50,
        "rx packets": {
            "rx packets": 0,
            "fragments": 0,
            "fragmented": 0,
            "bundles": 0,
            "bundled": 0
        },
        "tx packets": {
            "tx packets": 0,
            "fragments": 0,
            "fragmented": 0,
            "bundles": 0,
            "bundled": 0
        },
        "rx naks": {
            "rx naks": 0,
            "defs": 0,
            "dups": 0
        },
        "tx naks": {
            "tx naks": 0,
            "acks": 0,
            "retrans": 0
        },
        "congestion link": 0,
        "send queue max": 0,
        "avg": 0
    } ]

v2:
    Replace variable 'json_flag' by 'json' declared in include/utils.h

Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
---
 tipc/link.c | 414 +++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 272 insertions(+), 142 deletions(-)

diff --git a/tipc/link.c b/tipc/link.c
index 02f14aadefa6..26d1293f121a 100644
--- a/tipc/link.c
+++ b/tipc/link.c
@@ -23,6 +23,11 @@
 #include "msg.h"
 #include "link.h"
 #include "bearer.h"
+#include "utils.h"
+
+#define PRIORITY_STR "priority"
+#define TOLERANCE_STR "tolerance"
+#define WINDOW_STR "window"
 
 static int link_list_cb(const struct nlmsghdr *nlh, void *data)
 {
@@ -38,13 +43,14 @@ static int link_list_cb(const struct nlmsghdr *nlh, void *data)
 	if (!attrs[TIPC_NLA_LINK_NAME])
 		return MNL_CB_ERROR;
 
-	printf("%s: ", mnl_attr_get_str(attrs[TIPC_NLA_LINK_NAME]));
-
+	print_string(PRINT_FP, NULL, "%s: ",
+			     mnl_attr_get_str(attrs[TIPC_NLA_LINK_NAME]));
 	if (attrs[TIPC_NLA_LINK_UP])
-		printf("up\n");
+		print_string(PRINT_ANY,
+			 mnl_attr_get_str(attrs[TIPC_NLA_LINK_NAME]),"%s\n", "up");
 	else
-		printf("down\n");
-
+		print_string(PRINT_ANY,
+			 mnl_attr_get_str(attrs[TIPC_NLA_LINK_NAME]), "%s\n", "down");
 	return MNL_CB_OK;
 }
 
@@ -52,6 +58,7 @@ static int cmd_link_list(struct nlmsghdr *nlh, const struct cmd *cmd,
 			 struct cmdl *cmdl, void *data)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
+	int err = 0;
 
 	if (help_flag) {
 		fprintf(stderr, "Usage: %s link list\n", cmdl->argv[0]);
@@ -64,7 +71,12 @@ static int cmd_link_list(struct nlmsghdr *nlh, const struct cmd *cmd,
 		return -1;
 	}
 
-	return msg_dumpit(nlh, link_list_cb, NULL);
+	new_json_obj(json);
+	open_json_object(NULL);
+	err = msg_dumpit(nlh, link_list_cb, NULL);
+	close_json_object();
+	delete_json_obj();
+	return err;
 }
 
 static int link_get_cb(const struct nlmsghdr *nlh, void *data)
@@ -87,8 +99,23 @@ static int link_get_cb(const struct nlmsghdr *nlh, void *data)
 	if (!props[*prop])
 		return MNL_CB_ERROR;
 
-	printf("%u\n", mnl_attr_get_u32(props[*prop]));
-
+	new_json_obj(json);
+	open_json_object(NULL);
+	switch (*prop) {
+		case TIPC_NLA_PROP_PRIO:
+			print_uint(PRINT_ANY, PRIORITY_STR, "%u\n", mnl_attr_get_u32(props[*prop]));
+		break;
+		case TIPC_NLA_PROP_TOL:
+			print_uint(PRINT_ANY, TOLERANCE_STR, "%u\n", mnl_attr_get_u32(props[*prop]));
+		break;
+		case TIPC_NLA_PROP_WIN:
+			print_uint(PRINT_ANY, WINDOW_STR, "%u\n", mnl_attr_get_u32(props[*prop]));
+		break;
+		default:
+			break;
+	}
+	close_json_object();
+	delete_json_obj();
 	return MNL_CB_OK;
 }
 
@@ -103,11 +130,11 @@ static int cmd_link_get_prop(struct nlmsghdr *nlh, const struct cmd *cmd,
 		{ NULL }
 	};
 
-	if (strcmp(cmd->cmd, "priority") == 0)
+	if (strcmp(cmd->cmd, PRIORITY_STR) == 0)
 		prop = TIPC_NLA_PROP_PRIO;
-	else if ((strcmp(cmd->cmd, "tolerance") == 0))
+	else if ((strcmp(cmd->cmd, TOLERANCE_STR) == 0))
 		prop = TIPC_NLA_PROP_TOL;
-	else if ((strcmp(cmd->cmd, "window") == 0))
+	else if ((strcmp(cmd->cmd, WINDOW_STR) == 0))
 		prop = TIPC_NLA_PROP_WIN;
 	else
 		return -EINVAL;
@@ -150,9 +177,9 @@ static int cmd_link_get(struct nlmsghdr *nlh, const struct cmd *cmd,
 			struct cmdl *cmdl, void *data)
 {
 	const struct cmd cmds[] = {
-		{ "priority",	cmd_link_get_prop,	cmd_link_get_help },
-		{ "tolerance",	cmd_link_get_prop,	cmd_link_get_help },
-		{ "window",	cmd_link_get_prop,	cmd_link_get_help },
+		{ PRIORITY_STR,	cmd_link_get_prop,	cmd_link_get_help },
+		{ TOLERANCE_STR,	cmd_link_get_prop,	cmd_link_get_help },
+		{ WINDOW_STR,	cmd_link_get_prop,	cmd_link_get_help },
 		{ NULL }
 	};
 
@@ -211,109 +238,178 @@ static uint32_t perc(uint32_t count, uint32_t total)
 	return (count * 100 + (total / 2)) / total;
 }
 
-static int _show_link_stat(struct nlattr *attrs[], struct nlattr *prop[],
-			   struct nlattr *stats[])
+static int _show_link_stat(const char *name, struct nlattr *attrs[],
+			   struct nlattr *prop[], struct nlattr *stats[])
 {
 	uint32_t proft;
 
+	open_json_object(NULL);
+
+	print_string(PRINT_ANY, "link", "\nLink <%s>\n", name);
+	print_string(PRINT_JSON, "state", "", NULL);
+	open_json_array(PRINT_JSON, NULL);
 	if (attrs[TIPC_NLA_LINK_ACTIVE])
-		printf("  ACTIVE");
+		print_string(PRINT_ANY, NULL, "  %s", "ACTIVE");
 	else if (attrs[TIPC_NLA_LINK_UP])
-		printf("  STANDBY");
+		print_string(PRINT_ANY, NULL, "  %s", "STANDBY");
 	else
-		printf("  DEFUNCT");
-
-	printf("  MTU:%u  Priority:%u  Tolerance:%u ms  Window:%u packets\n",
-	       mnl_attr_get_u32(attrs[TIPC_NLA_LINK_MTU]),
-	       mnl_attr_get_u32(prop[TIPC_NLA_PROP_PRIO]),
-	       mnl_attr_get_u32(prop[TIPC_NLA_PROP_TOL]),
-	       mnl_attr_get_u32(prop[TIPC_NLA_PROP_WIN]));
-
-	printf("  RX packets:%u fragments:%u/%u bundles:%u/%u\n",
-	       mnl_attr_get_u32(attrs[TIPC_NLA_LINK_RX]) -
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_INFO]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_FRAGMENTS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_FRAGMENTED]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_BUNDLES]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_BUNDLED]));
-
-	printf("  TX packets:%u fragments:%u/%u bundles:%u/%u\n",
-	       mnl_attr_get_u32(attrs[TIPC_NLA_LINK_TX]) -
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_INFO]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_FRAGMENTS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_FRAGMENTED]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_BUNDLES]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_BUNDLED]));
+		print_string(PRINT_ANY, NULL, "  %s", "DEFUNCT");
+	close_json_array(PRINT_JSON, NULL);
+
+	print_uint(PRINT_ANY, "mtu", "  MTU:%u",
+			   mnl_attr_get_u32(attrs[TIPC_NLA_LINK_MTU]));
+	print_uint(PRINT_ANY, PRIORITY_STR, "  Priority:%u",
+			   mnl_attr_get_u32(prop[TIPC_NLA_PROP_PRIO]));
+	print_uint(PRINT_ANY, TOLERANCE_STR, "  Tolerance:%u ms",
+			   mnl_attr_get_u32(prop[TIPC_NLA_PROP_TOL]));
+	print_uint(PRINT_ANY, WINDOW_STR, "  Window:%u packets\n",
+			   mnl_attr_get_u32(prop[TIPC_NLA_PROP_WIN]));
+
+	open_json_object("rx packets");
+	print_uint(PRINT_ANY, "rx packets", "  RX packets:%u",
+			   mnl_attr_get_u32(attrs[TIPC_NLA_LINK_RX]) -
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_INFO]));
+	print_uint(PRINT_ANY, "fragments", " fragments:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_FRAGMENTS]));
+	print_uint(PRINT_ANY, "fragmented", "/%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_FRAGMENTED]));
+	print_uint(PRINT_ANY, "bundles", " bundles:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_BUNDLES]));
+	print_uint(PRINT_ANY, "bundled", "/%u\n",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_BUNDLED]));
+	close_json_object();
+
+	open_json_object("tx packets");
+	print_uint(PRINT_ANY, "tx packets", "  TX packets:%u",
+			   mnl_attr_get_u32(attrs[TIPC_NLA_LINK_TX]) -
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_INFO]));
+	print_uint(PRINT_ANY, "fragments", " fragments:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_FRAGMENTS]));
+	print_uint(PRINT_ANY, "fragmented", "/%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_FRAGMENTED]));
+	print_uint(PRINT_ANY, "bundles", " bundles:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_BUNDLES]));
+	print_uint(PRINT_ANY, "bundled", "/%u\n",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_BUNDLED]));
+	close_json_object();
 
 	proft = mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_PROF_TOT]);
-	printf("  TX profile sample:%u packets  average:%u octets\n",
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_CNT]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_TOT]) / proft);
-
-	printf("  0-64:%u%% -256:%u%% -1024:%u%% -4096:%u%% -16384:%u%% -32768:%u%% -66000:%u%%\n",
-	       perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P0]), proft),
-	       perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P1]), proft),
-	       perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P2]), proft),
-	       perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P3]), proft),
-	       perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P4]), proft),
-	       perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P5]), proft),
-	       perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P6]), proft));
-
-	printf("  RX states:%u probes:%u naks:%u defs:%u dups:%u\n",
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_STATES]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_PROBES]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_NACKS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_DEFERRED]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_DUPLICATES]));
-
-	printf("  TX states:%u probes:%u naks:%u acks:%u dups:%u\n",
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_STATES]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_PROBES]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_NACKS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_ACKS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RETRANSMITTED]));
-
-	printf("  Congestion link:%u  Send queue max:%u avg:%u\n",
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_LINK_CONGS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_MAX_QUEUE]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_AVG_QUEUE]));
-
+	print_uint(PRINT_ANY, "tx profile sample", "  TX profile sample:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_CNT]));
+	print_uint(PRINT_ANY, "packets average", " packets average:%u octets\n",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_TOT]) / proft);
+
+	print_uint(PRINT_ANY, "0-64", "  0-64:%u%%",
+			   perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P0]), proft));
+	print_uint(PRINT_ANY, "-256", " -256:%u%%",
+			   perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P1]), proft));
+	print_uint(PRINT_ANY, "-1024", " -1024:%u%%",
+			   perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P2]), proft));
+	print_uint(PRINT_ANY, "-4096", " -4096:%u%%",
+			   perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P3]), proft));
+	print_uint(PRINT_ANY, "-16384", " -16384:%u%%",
+			   perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P4]), proft));
+	print_uint(PRINT_ANY, "-32768", " -32768:%u%%",
+			   perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P5]), proft));
+	print_uint(PRINT_ANY, "-66000", " -66000:%u%%\n",
+			   perc(mnl_attr_get_u32(stats[TIPC_NLA_STATS_MSG_LEN_P6]), proft));
+
+	open_json_object("rx states");
+	print_uint(PRINT_ANY, "rx states", "  RX states:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_STATES]));
+	print_uint(PRINT_ANY, "probes", " probes:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_PROBES]));
+	print_uint(PRINT_ANY, "naks", " naks:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_NACKS]));
+	print_uint(PRINT_ANY, "defs", " defs:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_DEFERRED]));
+	print_uint(PRINT_ANY, "dups", " dups:%u\n",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_DUPLICATES]));
+	close_json_object();
+
+	open_json_object("tx states");
+	print_uint(PRINT_ANY, "tx states", "  TX states:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_STATES]));
+	print_uint(PRINT_ANY, "probes", " probes:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_PROBES]));
+	print_uint(PRINT_ANY, "naks", " naks:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_NACKS]));
+	print_uint(PRINT_ANY, "acks", " acks:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_ACKS]));
+	print_uint(PRINT_ANY, "retrans", " retrans:%u\n",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RETRANSMITTED]));
+	close_json_object();
+
+	print_uint(PRINT_ANY, "congestion link", "  Congestion link:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_LINK_CONGS]));
+	print_uint(PRINT_ANY, "send queue max", "  Send queue max:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_MAX_QUEUE]));
+	print_uint(PRINT_ANY, "avg", " avg:%u\n",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_AVG_QUEUE]));
+
+	close_json_object();
 	return MNL_CB_OK;
 }
 
-static int _show_bc_link_stat(struct nlattr *prop[], struct nlattr *stats[])
+static int _show_bc_link_stat(const char *name, struct nlattr *prop[],
+			   struct nlattr *stats[])
 {
-	printf("  Window:%u packets\n",
-	       mnl_attr_get_u32(prop[TIPC_NLA_PROP_WIN]));
-
-	printf("  RX packets:%u fragments:%u/%u bundles:%u/%u\n",
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_INFO]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_FRAGMENTS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_FRAGMENTED]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_BUNDLES]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_BUNDLED]));
-
-	printf("  TX packets:%u fragments:%u/%u bundles:%u/%u\n",
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_INFO]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_FRAGMENTS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_FRAGMENTED]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_BUNDLES]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_BUNDLED]));
-
-	printf("  RX naks:%u defs:%u dups:%u\n",
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_NACKS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_DEFERRED]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_DUPLICATES]));
-
-	printf("  TX naks:%u acks:%u dups:%u\n",
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_NACKS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_ACKS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_RETRANSMITTED]));
-
-	printf("  Congestion link:%u  Send queue max:%u avg:%u\n",
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_LINK_CONGS]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_MAX_QUEUE]),
-	       mnl_attr_get_u32(stats[TIPC_NLA_STATS_AVG_QUEUE]));
+	open_json_object(NULL);
+	print_string(PRINT_ANY, "link", "Link <%s>\n", name);
+	print_uint(PRINT_ANY, WINDOW_STR, "  Window:%u packets\n",
+			   mnl_attr_get_u32(prop[TIPC_NLA_PROP_WIN]));
+
+	open_json_object("rx packets");
+	print_uint(PRINT_ANY, "rx packets", "  RX packets:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_INFO]));
+	print_uint(PRINT_ANY, "fragments", " fragments:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_FRAGMENTS]));
+	print_uint(PRINT_ANY, "fragmented", "/%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_FRAGMENTED]));
+	print_uint(PRINT_ANY, "bundles", " bundles:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_BUNDLES]));
+	print_uint(PRINT_ANY, "bundled", "/%u\n",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_BUNDLED]));
+	close_json_object();
+
+	open_json_object("tx packets");
+	print_uint(PRINT_ANY, "tx packets", "  TX packets:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_INFO]));
+	print_uint(PRINT_ANY, "fragments", " fragments:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_FRAGMENTS]));
+	print_uint(PRINT_ANY, "fragmented", "/%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_FRAGMENTED]));
+	print_uint(PRINT_ANY, "bundles", " bundles:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_BUNDLES]));
+	print_uint(PRINT_ANY, "bundled", "/%u\n",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_BUNDLED]));
+	close_json_object();
+
+	open_json_object("rx naks");
+	print_uint(PRINT_ANY, "rx naks", "  RX naks:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_NACKS]));
+	print_uint(PRINT_ANY, "defs",  " defs:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RX_DEFERRED]));
+	print_uint(PRINT_ANY, "dups",  " dups:%u\n",
+		   mnl_attr_get_u32(stats[TIPC_NLA_STATS_DUPLICATES]));
+	close_json_object();
+
+	open_json_object("tx naks");
+	print_uint(PRINT_ANY, "tx naks", "  TX naks:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_NACKS]));
+	print_uint(PRINT_ANY, "acks",  " acks:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_TX_ACKS]));
+	print_uint(PRINT_ANY, "retrans",  " retrans:%u\n",
+		   mnl_attr_get_u32(stats[TIPC_NLA_STATS_RETRANSMITTED]));
+	close_json_object();
+
+	print_uint(PRINT_ANY, "congestion link", "  Congestion link:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_LINK_CONGS]));
+	print_uint(PRINT_ANY, "send queue max", "  Send queue max:%u",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_MAX_QUEUE]));
+	print_uint(PRINT_ANY, "avg", " avg:%u\n",
+			   mnl_attr_get_u32(stats[TIPC_NLA_STATS_AVG_QUEUE]));
+	close_json_object();
 
 	return MNL_CB_OK;
 }
@@ -347,13 +443,10 @@ static int link_stat_show_cb(const struct nlmsghdr *nlh, void *data)
 		return MNL_CB_OK;
 
 	if (attrs[TIPC_NLA_LINK_BROADCAST]) {
-		printf("Link <%s>\n", name);
-		return _show_bc_link_stat(prop, stats);
+		return _show_bc_link_stat(name, prop, stats);
 	}
 
-	printf("\nLink <%s>\n", name);
-
-	return _show_link_stat(attrs, prop, stats);
+	return _show_link_stat(name, attrs, prop, stats);
 }
 
 static void cmd_link_stat_show_help(struct cmdl *cmdl)
@@ -372,6 +465,7 @@ static int cmd_link_stat_show(struct nlmsghdr *nlh, const struct cmd *cmd,
 		{ "link",		OPT_KEYVAL,	NULL },
 		{ NULL }
 	};
+	int err = 0;
 
 	if (help_flag) {
 		(cmd->help)(cmdl);
@@ -391,7 +485,10 @@ static int cmd_link_stat_show(struct nlmsghdr *nlh, const struct cmd *cmd,
 	if (opt)
 		link = opt->val;
 
-	return msg_dumpit(nlh, link_stat_show_cb, link);
+	new_json_obj(json);
+	err = msg_dumpit(nlh, link_stat_show_cb, link);
+	delete_json_obj();
+	return err;
 }
 
 static void cmd_link_stat_help(struct cmdl *cmdl)
@@ -439,11 +536,11 @@ static int cmd_link_set_prop(struct nlmsghdr *nlh, const struct cmd *cmd,
 		{ NULL }
 	};
 
-	if (strcmp(cmd->cmd, "priority") == 0)
+	if (strcmp(cmd->cmd, PRIORITY_STR) == 0)
 		prop = TIPC_NLA_PROP_PRIO;
-	else if ((strcmp(cmd->cmd, "tolerance") == 0))
+	else if ((strcmp(cmd->cmd, TOLERANCE_STR) == 0))
 		prop = TIPC_NLA_PROP_TOL;
-	else if ((strcmp(cmd->cmd, "window") == 0))
+	else if ((strcmp(cmd->cmd, WINDOW_STR) == 0))
 		prop = TIPC_NLA_PROP_WIN;
 	else
 		return -EINVAL;
@@ -489,9 +586,9 @@ static int cmd_link_set(struct nlmsghdr *nlh, const struct cmd *cmd,
 			struct cmdl *cmdl, void *data)
 {
 	const struct cmd cmds[] = {
-		{ "priority",	cmd_link_set_prop,	cmd_link_set_help },
-		{ "tolerance",	cmd_link_set_prop,	cmd_link_set_help },
-		{ "window",	cmd_link_set_prop,	cmd_link_set_help },
+		{ PRIORITY_STR,	cmd_link_set_prop,	cmd_link_set_help },
+		{ TOLERANCE_STR,	cmd_link_set_prop,	cmd_link_set_help },
+		{ WINDOW_STR,	cmd_link_set_prop,	cmd_link_set_help },
 		{ NULL }
 	};
 
@@ -537,15 +634,17 @@ static int link_mon_summary_cb(const struct nlmsghdr *nlh, void *data)
 
 	mnl_attr_parse_nested(info[TIPC_NLA_MON], parse_attrs, attrs);
 
-	printf("\nbearer %s\n",
+	open_json_object(NULL);
+	print_string(PRINT_ANY, "bearer", "\nbearer %s\n",
 		mnl_attr_get_str(attrs[TIPC_NLA_MON_BEARER_NAME]));
 
-	printf("    table_generation %u\n",
+	print_uint(PRINT_ANY, "table_generation", "    table_generation %u\n",
 	       mnl_attr_get_u32(attrs[TIPC_NLA_MON_LISTGEN]));
-	printf("    cluster_size %u\n",
+	print_uint(PRINT_ANY, "cluster_size", "    cluster_size %u\n",
 		mnl_attr_get_u32(attrs[TIPC_NLA_MON_PEERCNT]));
-	printf("    algorithm %s\n",
+	print_string(PRINT_ANY, "algorithm", "    algorithm %s\n",
 		attrs[TIPC_NLA_MON_ACTIVE] ? "overlapping-ring" : "full-mesh");
+	close_json_object();
 
 	return MNL_CB_OK;
 }
@@ -554,6 +653,7 @@ static int cmd_link_mon_summary(struct nlmsghdr *nlh, const struct cmd *cmd,
 				struct cmdl *cmdl, void *data)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
+	int err = 0;
 
 	if (help_flag) {
 		fprintf(stderr,	"Usage: %s monitor summary\n", cmdl->argv[0]);
@@ -566,7 +666,11 @@ static int cmd_link_mon_summary(struct nlmsghdr *nlh, const struct cmd *cmd,
 		return -1;
 	}
 
-	return msg_dumpit(nlh, link_mon_summary_cb, NULL);
+	new_json_obj(json);
+	err = msg_dumpit(nlh, link_mon_summary_cb, NULL);
+	delete_json_obj();
+
+	return err;
 }
 
 #define STATUS_WIDTH 7
@@ -587,16 +691,19 @@ static int map_get(uint64_t up_map, int i)
 static void link_mon_print_applied(uint16_t applied, uint64_t up_map)
 {
 	int i;
-	char state;
 
+	open_json_array(PRINT_JSON, "applied_node_status");
 	for (i = 0; i < applied; i++) {
+		char state_str[2] = {0};
+
 		/* print the delimiter for every -n- entry */
 		if (i && !(i % APPL_NODE_STATUS_WIDTH))
-			printf(",");
+			print_string(PRINT_FP, NULL, "%s", ",");
 
-		state = map_get(up_map, i) ? 'U' : 'D';
-		printf("%c", state);
+		sprintf(state_str, "%c", map_get(up_map, i) ? 'U' : 'D');
+		print_string(PRINT_ANY, NULL, "%s", state_str);
 	}
+	close_json_array(PRINT_JSON, "applied_node_status");
 }
 
 /* print the non applied members, since we dont know
@@ -608,19 +715,23 @@ static void link_mon_print_non_applied(uint16_t applied, uint16_t member_cnt,
 	int i;
 	char state;
 
-	printf(" [");
+	open_json_array(PRINT_JSON, "[non_applied_node:status]");
+	print_string(PRINT_FP, NULL, " %s", "[");
 	for (i = applied; i < member_cnt; i++) {
 		char addr_str[16];
+		char full_state[17] = {0};
 
 		/* print the delimiter for every entry */
 		if (i != applied)
-			printf(",");
+			print_string(PRINT_FP, NULL, "%s", ",");
 
 		sprintf(addr_str, "%x:", members[i]);
 		state = map_get(up_map, i) ? 'U' : 'D';
-		printf("%s%c", addr_str, state);
+		sprintf(full_state, "%s%c", addr_str, state);
+		print_string(PRINT_ANY, NULL, "%s", full_state);
 	}
-	printf("]");
+	print_string(PRINT_FP, NULL, "%s", "]");
+	close_json_array(PRINT_JSON, "[non_applied_node:status]");
 }
 
 static void link_mon_print_peer_state(const uint32_t addr, const char *status,
@@ -631,11 +742,17 @@ static void link_mon_print_peer_state(const uint32_t addr, const char *status,
 
 	sprintf(addr_str, "%u.%u.%u", tipc_zone(addr), tipc_cluster(addr),
 		tipc_node(addr));
-
-	printf("%-*s", MAX_NODE_WIDTH, addr_str);
-	printf("%-*s", STATUS_WIDTH, status);
-	printf("%-*s", DIRECTLY_MON_WIDTH, monitored);
-	printf("%-*u", MAX_DOM_GEN_WIDTH, dom_gen);
+	if (is_json_context()) {
+		print_string(PRINT_JSON, "node", NULL, addr_str);
+		print_string(PRINT_JSON, "status", NULL, status);
+		print_string(PRINT_JSON, "monitored", NULL, monitored);
+		print_uint(PRINT_JSON, "generation", NULL, dom_gen);
+	} else {
+		printf("%-*s", MAX_NODE_WIDTH, addr_str);
+		printf("%-*s", STATUS_WIDTH, status);
+		printf("%-*s", DIRECTLY_MON_WIDTH, monitored);
+		printf("%-*u", MAX_DOM_GEN_WIDTH, dom_gen);
+	}
 }
 
 static int link_mon_peer_list_cb(const struct nlmsghdr *nlh, void *data)
@@ -654,6 +771,7 @@ static int link_mon_peer_list_cb(const struct nlmsghdr *nlh, void *data)
 	if (!info[TIPC_NLA_MON_PEER])
 		return MNL_CB_ERROR;
 
+	open_json_object(NULL);
 	mnl_attr_parse_nested(info[TIPC_NLA_MON_PEER], parse_attrs, attrs);
 
 	(attrs[TIPC_NLA_MON_PEER_LOCAL] || attrs[TIPC_NLA_MON_PEER_HEAD]) ?
@@ -688,8 +806,9 @@ static int link_mon_peer_list_cb(const struct nlmsghdr *nlh, void *data)
 				   mnl_attr_get_payload(attrs[TIPC_NLA_MON_PEER_MEMBERS]));
 
 exit:
-	printf("\n");
+	print_string(PRINT_FP, NULL, "\n", "");
 
+	close_json_object();
 	return MNL_CB_OK;
 }
 
@@ -698,6 +817,7 @@ static int link_mon_peer_list(uint32_t mon_ref)
 	struct nlmsghdr *nlh;
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	struct nlattr *nest;
+	int err = 0;
 
 	nlh = msg_init(buf, TIPC_NL_MON_PEER_GET);
 	if (!nlh) {
@@ -709,7 +829,8 @@ static int link_mon_peer_list(uint32_t mon_ref)
 	mnl_attr_put_u32(nlh, TIPC_NLA_MON_REF, mon_ref);
 	mnl_attr_nest_end(nlh, nest);
 
-	return msg_dumpit(nlh, link_mon_peer_list_cb, NULL);
+	err = msg_dumpit(nlh, link_mon_peer_list_cb, NULL);
+	return err;
 }
 
 static int link_mon_list_cb(const struct nlmsghdr *nlh, void *data)
@@ -733,12 +854,16 @@ static int link_mon_list_cb(const struct nlmsghdr *nlh, void *data)
 	if (*req_bearer && (strcmp(req_bearer, bname) != 0))
 		return MNL_CB_OK;
 
-	printf("\nbearer %s\n", bname);
-	printf("%s\n", title);
+	open_json_object(NULL);
+	print_string(PRINT_ANY, "bearer", "\nbearer %s\n", bname);
+	print_string(PRINT_FP, NULL, "%s\n", title);
 
+	open_json_array(PRINT_JSON, bname);
 	if (mnl_attr_get_u32(attrs[TIPC_NLA_MON_PEERCNT]))
 		link_mon_peer_list(mnl_attr_get_u32(attrs[TIPC_NLA_MON_REF]));
+	close_json_array(PRINT_JSON, bname);
 
+	close_json_object();
 	return MNL_CB_OK;
 }
 
@@ -804,7 +929,10 @@ static int cmd_link_mon_list(struct nlmsghdr *nlh, const struct cmd *cmd,
 		return -1;
 	}
 
-	return msg_dumpit(nlh, link_mon_list_cb, bname);
+	new_json_obj(json);
+	err = msg_dumpit(nlh, link_mon_list_cb, bname);
+	delete_json_obj();
+	return err;
 }
 
 static void cmd_link_mon_set_help(struct cmdl *cmdl)
@@ -848,8 +976,10 @@ static int link_mon_get_cb(const struct nlmsghdr *nlh, void *data)
 	if (!attrs[TIPC_NLA_MON_ACTIVATION_THRESHOLD])
 		return MNL_CB_ERROR;
 
-	printf("%u\n",
-	       mnl_attr_get_u32(attrs[TIPC_NLA_MON_ACTIVATION_THRESHOLD]));
+	new_json_obj(json);
+	print_uint(PRINT_ANY, "threshold", "%u\n",
+			   mnl_attr_get_u32(attrs[TIPC_NLA_MON_ACTIVATION_THRESHOLD]));
+	delete_json_obj();
 
 	return MNL_CB_OK;
 }
-- 
2.7.4

^ permalink raw reply related

* [iproute2-next v2 1/2] tipc: JSON support for showing nametable
From: Hoang Le @ 2018-06-11  2:16 UTC (permalink / raw)
  To: netdev, tipc-discussion

Add json output support for nametable show

Example output:
$tipc -j -p nametable show

[ {
        "type": 0,
        "lower": 16781313,
        "upper": 16781313,
        "scope": "zone",
        "port": 0,
        "node": ""
    },{
        "type": 0,
        "lower": 16781416,
        "upper": 16781416,
        "scope": "cluster",
        "port": 0,
        "node": ""
    } ]

v2:
    Replace variable 'json_flag' by 'json' declared in include/utils.h
    Add new parameter '-pretty' to support pretty output

Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
---
 tipc/nametable.c | 31 ++++++++++++++++++++++---------
 tipc/tipc.c      | 20 +++++++++++++++++++-
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/tipc/nametable.c b/tipc/nametable.c
index ae73dfa5f8b9..eb4bd0bda835 100644
--- a/tipc/nametable.c
+++ b/tipc/nametable.c
@@ -21,6 +21,7 @@
 #include "msg.h"
 #include "nametable.h"
 #include "misc.h"
+#include "utils.h"
 
 #define PORTID_STR_LEN 45 /* Four u32 and five delimiter chars */
 
@@ -46,7 +47,7 @@ static int nametable_show_cb(const struct nlmsghdr *nlh, void *data)
 	if (!publ[TIPC_NLA_NAME_TABLE_PUBL])
 		return MNL_CB_ERROR;
 
-	if (!*iteration)
+	if (!*iteration && !is_json_context())
 		printf("%-10s %-10s %-10s %-8s %-10s %-33s\n",
 		       "Type", "Lower", "Upper", "Scope", "Port",
 		       "Node");
@@ -54,13 +55,20 @@ static int nametable_show_cb(const struct nlmsghdr *nlh, void *data)
 
 	hash2nodestr(mnl_attr_get_u32(publ[TIPC_NLA_PUBL_NODE]), str);
 
-	printf("%-10u %-10u %-10u %-8s %-10u %s\n",
-	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_TYPE]),
-	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_LOWER]),
-	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_UPPER]),
-	       scope[mnl_attr_get_u32(publ[TIPC_NLA_PUBL_SCOPE])],
-	       mnl_attr_get_u32(publ[TIPC_NLA_PUBL_REF]),
-	       str);
+	open_json_object(NULL);
+	print_uint(PRINT_ANY, "type", "%-10u",
+			   mnl_attr_get_u32(publ[TIPC_NLA_PUBL_TYPE]));
+	print_uint(PRINT_ANY, "lower", "%-10u",
+			   mnl_attr_get_u32(publ[TIPC_NLA_PUBL_LOWER]));
+	print_uint(PRINT_ANY, "upper", "%-10u",
+			   mnl_attr_get_u32(publ[TIPC_NLA_PUBL_UPPER]));
+	print_string(PRINT_ANY, "scope", "%-8s",
+			     scope[mnl_attr_get_u32(publ[TIPC_NLA_PUBL_SCOPE])]);
+	print_uint(PRINT_ANY, "port", "%-10u",
+			   mnl_attr_get_u32(publ[TIPC_NLA_PUBL_REF]));
+	print_string(PRINT_ANY, "node", "%s", str);
+	print_string(PRINT_FP, NULL, "\n", "");
+	close_json_object();
 
 	return MNL_CB_OK;
 }
@@ -70,6 +78,7 @@ static int cmd_nametable_show(struct nlmsghdr *nlh, const struct cmd *cmd,
 {
 	int iteration = 0;
 	char buf[MNL_SOCKET_BUFFER_SIZE];
+	int rc = 0;
 
 	if (help_flag) {
 		fprintf(stderr, "Usage: %s nametable show\n", cmdl->argv[0]);
@@ -81,7 +90,11 @@ static int cmd_nametable_show(struct nlmsghdr *nlh, const struct cmd *cmd,
 		return -1;
 	}
 
-	return msg_dumpit(nlh, nametable_show_cb, &iteration);
+	new_json_obj(json);
+	rc = msg_dumpit(nlh, nametable_show_cb, &iteration);
+	delete_json_obj();
+
+	return rc;
 }
 
 void cmd_nametable_help(struct cmdl *cmdl)
diff --git a/tipc/tipc.c b/tipc/tipc.c
index 600d5e2a160f..f85ddee0e278 100644
--- a/tipc/tipc.c
+++ b/tipc/tipc.c
@@ -24,6 +24,8 @@
 #include "cmdl.h"
 
 int help_flag;
+int json;
+int pretty;
 
 static void about(struct cmdl *cmdl)
 {
@@ -33,6 +35,8 @@ static void about(struct cmdl *cmdl)
 		"\n"
 		"Options:\n"
 		" -h, --help \t\tPrint help for last given command\n"
+		" -j, --json \t\tJson format printouts\n"
+		" -p, --pretty \t\tpretty print\n"
 		"\n"
 		"Commands:\n"
 		" bearer                - Show or modify bearers\n"
@@ -53,6 +57,8 @@ int main(int argc, char *argv[])
 	const struct cmd cmd = {"tipc", NULL, about};
 	struct option long_options[] = {
 		{"help", no_argument, 0, 'h'},
+		{"json", no_argument, 0, 'j'},
+		{"pretty", no_argument, 0, 'p'},
 		{0, 0, 0, 0}
 	};
 	const struct cmd cmds[] = {
@@ -69,7 +75,7 @@ int main(int argc, char *argv[])
 	do {
 		int option_index = 0;
 
-		i = getopt_long(argc, argv, "h", long_options, &option_index);
+		i = getopt_long(argc, argv, "hjp", long_options, &option_index);
 
 		switch (i) {
 		case 'h':
@@ -79,6 +85,18 @@ int main(int argc, char *argv[])
 			 */
 			help_flag = 1;
 			break;
+		case 'j':
+			/*
+			 * Enable json format printouts
+			 */
+			json = 1;
+			break;
+		case 'p':
+			/*
+			 * Enable json pretty output
+			 */
+			pretty = 1;
+			break;
 		case -1:
 			/* End of options */
 			break;
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support
From: Jason Wang @ 2018-06-11  2:29 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20180611050741-mutt-send-email-mst@kernel.org>



On 2018年06月11日 10:12, Michael S. Tsirkin wrote:
> On Fri, Jun 08, 2018 at 01:07:09PM +0800, Jason Wang wrote:
>>
>> On 2018年06月08日 12:46, Michael S. Tsirkin wrote:
>>> On Fri, Jun 08, 2018 at 11:50:42AM +0800, Jason Wang wrote:
>>>> This feature bit is duplicated with VIRTIO_F_ANY_LAYOUT, this means if
>>>> a userpsace want to enable VRITIO_F_ANY_LAYOUT,
>>>> VHOST_NET_F_VIRTIO_NET_HDR will be implied too. This is wrong and will
>>>> break networking.
>>> What breaks networking exactly? VHOST_NET supported ANY_LAYOUT
>>> from day one. For this reason it does not need to know about
>>> VRITIO_F_ANY_LAYOUT and we reused the bit for other purposes.
>> It's the knowledge of vhost_net code it self but not userspace. For
>> userspace, it should depends on the value of returned by VHOST_GET_FEATURES.
>> So when userspace can set_features with ANY_LAYOUT, vhost may think it wants
>> VHOST_NET_F_VIRTIO_NET_HDR.
> Yes but that's the admittedly ugly API that we have now.
> userspace is supposed to know VRITIO_F_ANY_LAYOUT does
> not make sense for vhost.

Ok.

>
>
>
>>>
>>>
>>>> Fixing this by safely removing
>>>> VHOST_NET_F_VIRTIO_NET_HDR support. There should be very few or even
>>>> no userspace can use this.
>>> Quite possibly, but it is hard to be sure. It seems safer to
>>> maintain it unless there's an actual reason something's broken.
>> I think not since the feature is negotiated not mandatory?
> That doesn't mean much.
>
>>>> Further cleanups could be done for
>>>> -net-next for safety.
>>>>
>>>> In the future, we need a vhost dedicated feature set/get ioctl()
>>>> instead of reusing virtio ones.
>>> Not just in the future, we might want to switch iommu
>>> to a sane structure without the 64 bit padding bug
>>> right now.
>> Yes, I hit this bug when introducing V2 of msg IOTLB message.
> Sounds good, so if you like, reserve a bit for
> VHOST_NET_F_VIRTIO_NET_HDR in the new ioctl mask and
> do not enable it there.

Ok, and maybe VHOST_F_LOG_ALL.

>
>>>> Fixes: 4e9fa50c6ccbe ("vhost: move features to core")
>>> This tag makes no sense here IMHO. Looks like people are using some tool
>>> that just looks at the earliest version where patch won't apply. The
>>> commit in question just moved some code around.
>> Looks not, before this commit, vhost_net won't return ANY_LAYOUT.
>>
>> Thanks
> Well ANY_LAYOUT just happens to be same as VHOST_NET_F_VIRTIO_NET_HDR
> and that has been set since forever.

So do you still want this patch? If not we need to document that 
ANY_LAYOUT could not be passed through SET_FEATURES somewhere.

Thanks

>
>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>> ---
>>>>    drivers/vhost/net.c | 15 +++++----------
>>>>    1 file changed, 5 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>>>> index 986058a..83eef52 100644
>>>> --- a/drivers/vhost/net.c
>>>> +++ b/drivers/vhost/net.c
>>>> @@ -69,7 +69,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
>>>>    enum {
>>>>    	VHOST_NET_FEATURES = VHOST_FEATURES |
>>>> -			 (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
>>>>    			 (1ULL << VIRTIO_NET_F_MRG_RXBUF) |
>>>>    			 (1ULL << VIRTIO_F_IOMMU_PLATFORM)
>>>>    };
>>>> @@ -1255,15 +1254,11 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
>>>>    			       (1ULL << VIRTIO_F_VERSION_1))) ?
>>>>    			sizeof(struct virtio_net_hdr_mrg_rxbuf) :
>>>>    			sizeof(struct virtio_net_hdr);
>>>> -	if (features & (1 << VHOST_NET_F_VIRTIO_NET_HDR)) {
>>>> -		/* vhost provides vnet_hdr */
>>>> -		vhost_hlen = hdr_len;
>>>> -		sock_hlen = 0;
>>>> -	} else {
>>>> -		/* socket provides vnet_hdr */
>>>> -		vhost_hlen = 0;
>>>> -		sock_hlen = hdr_len;
>>>> -	}
>>>> +
>>>> +        /* socket provides vnet_hdr */
>>>> +	vhost_hlen = 0;
>>>> +	sock_hlen = hdr_len;
>>>> +
>>>>    	mutex_lock(&n->dev.mutex);
>>>>    	if ((features & (1 << VHOST_F_LOG_ALL)) &&
>>>>    	    !vhost_log_access_ok(&n->dev))
>>>> -- 
>>>> 2.7.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support
From: Michael S. Tsirkin @ 2018-06-11  3:08 UTC (permalink / raw)
  To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <90e9b3ea-e9d2-d60d-8355-42d447c67452@redhat.com>

On Mon, Jun 11, 2018 at 10:29:36AM +0800, Jason Wang wrote:
> 
> 
> On 2018年06月11日 10:12, Michael S. Tsirkin wrote:
> > On Fri, Jun 08, 2018 at 01:07:09PM +0800, Jason Wang wrote:
> > > 
> > > On 2018年06月08日 12:46, Michael S. Tsirkin wrote:
> > > > On Fri, Jun 08, 2018 at 11:50:42AM +0800, Jason Wang wrote:
> > > > > This feature bit is duplicated with VIRTIO_F_ANY_LAYOUT, this means if
> > > > > a userpsace want to enable VRITIO_F_ANY_LAYOUT,
> > > > > VHOST_NET_F_VIRTIO_NET_HDR will be implied too. This is wrong and will
> > > > > break networking.
> > > > What breaks networking exactly? VHOST_NET supported ANY_LAYOUT
> > > > from day one. For this reason it does not need to know about
> > > > VRITIO_F_ANY_LAYOUT and we reused the bit for other purposes.
> > > It's the knowledge of vhost_net code it self but not userspace. For
> > > userspace, it should depends on the value of returned by VHOST_GET_FEATURES.
> > > So when userspace can set_features with ANY_LAYOUT, vhost may think it wants
> > > VHOST_NET_F_VIRTIO_NET_HDR.
> > Yes but that's the admittedly ugly API that we have now.
> > userspace is supposed to know VRITIO_F_ANY_LAYOUT does
> > not make sense for vhost.
> 
> Ok.
> 
> > 
> > 
> > 
> > > > 
> > > > 
> > > > > Fixing this by safely removing
> > > > > VHOST_NET_F_VIRTIO_NET_HDR support. There should be very few or even
> > > > > no userspace can use this.
> > > > Quite possibly, but it is hard to be sure. It seems safer to
> > > > maintain it unless there's an actual reason something's broken.
> > > I think not since the feature is negotiated not mandatory?
> > That doesn't mean much.
> > 
> > > > > Further cleanups could be done for
> > > > > -net-next for safety.
> > > > > 
> > > > > In the future, we need a vhost dedicated feature set/get ioctl()
> > > > > instead of reusing virtio ones.
> > > > Not just in the future, we might want to switch iommu
> > > > to a sane structure without the 64 bit padding bug
> > > > right now.
> > > Yes, I hit this bug when introducing V2 of msg IOTLB message.
> > Sounds good, so if you like, reserve a bit for
> > VHOST_NET_F_VIRTIO_NET_HDR in the new ioctl mask and
> > do not enable it there.
> 
> Ok, and maybe VHOST_F_LOG_ALL.
> 
> > 
> > > > > Fixes: 4e9fa50c6ccbe ("vhost: move features to core")
> > > > This tag makes no sense here IMHO. Looks like people are using some tool
> > > > that just looks at the earliest version where patch won't apply. The
> > > > commit in question just moved some code around.
> > > Looks not, before this commit, vhost_net won't return ANY_LAYOUT.
> > > 
> > > Thanks
> > Well ANY_LAYOUT just happens to be same as VHOST_NET_F_VIRTIO_NET_HDR
> > and that has been set since forever.
> 
> So do you still want this patch? If not we need to document that ANY_LAYOUT
> could not be passed through SET_FEATURES somewhere.
> 
> Thanks

Let's document this, yes. I don't think we should drop
VHOST_NET_F_VIRTIO_NET_HDR now.

> > 
> > > > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > > > ---
> > > > >    drivers/vhost/net.c | 15 +++++----------
> > > > >    1 file changed, 5 insertions(+), 10 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > > > index 986058a..83eef52 100644
> > > > > --- a/drivers/vhost/net.c
> > > > > +++ b/drivers/vhost/net.c
> > > > > @@ -69,7 +69,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
> > > > >    enum {
> > > > >    	VHOST_NET_FEATURES = VHOST_FEATURES |
> > > > > -			 (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
> > > > >    			 (1ULL << VIRTIO_NET_F_MRG_RXBUF) |
> > > > >    			 (1ULL << VIRTIO_F_IOMMU_PLATFORM)
> > > > >    };
> > > > > @@ -1255,15 +1254,11 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
> > > > >    			       (1ULL << VIRTIO_F_VERSION_1))) ?
> > > > >    			sizeof(struct virtio_net_hdr_mrg_rxbuf) :
> > > > >    			sizeof(struct virtio_net_hdr);
> > > > > -	if (features & (1 << VHOST_NET_F_VIRTIO_NET_HDR)) {
> > > > > -		/* vhost provides vnet_hdr */
> > > > > -		vhost_hlen = hdr_len;
> > > > > -		sock_hlen = 0;
> > > > > -	} else {
> > > > > -		/* socket provides vnet_hdr */
> > > > > -		vhost_hlen = 0;
> > > > > -		sock_hlen = hdr_len;
> > > > > -	}
> > > > > +
> > > > > +        /* socket provides vnet_hdr */
> > > > > +	vhost_hlen = 0;
> > > > > +	sock_hlen = hdr_len;
> > > > > +
> > > > >    	mutex_lock(&n->dev.mutex);
> > > > >    	if ((features & (1 << VHOST_F_LOG_ALL)) &&
> > > > >    	    !vhost_log_access_ok(&n->dev))
> > > > > -- 
> > > > > 2.7.4
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH 0/5] can: enable multi-queue for SocketCAN devices
From: Oleksij Rempel @ 2018-06-11  4:41 UTC (permalink / raw)
  To: Jonas Mark (BT-FIR/ENG1), Andy Shevchenko
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, linux-can@vger.kernel.org,
	netdev, Linux Kernel Mailing List, Heiko Schocher,
	ZHU Yi (BT-FIR/ENG1-Zhu)
In-Reply-To: <b7f078ad43104de9bf2e7dbbe6a2a463@de.bosch.com>


[-- Attachment #1.1: Type: text/plain, Size: 2007 bytes --]

Hi,


On 07.06.2018 17:14, Jonas Mark (BT-FIR/ENG1) wrote:
> Hi Andy,
> 
>>> The functionality bases on an external peripheral chip named Companion.
>>> It offers two CAN interfaces, each has 8 prioritized transmit FIFOs as
>>> well as one receive FIFO. Besides CAN, undisclosed additional functions
>>> can be accessed through the char device.
>>>
>>> A standard SPI interface with two additional lines for flow control is
>>> used. The Companion chip is the SPI slave.
>>
>> Can remoteproc API be utilized here?
> 
> So far I wasn't aware of the remoteproc API. It appears to me that is
> limited to power on/off and loading firmware in an AMP scenario. Here,
> the Companion has a fixed firmware in it. It must already be running
> quickly after power-up, even before the boot loader.

yes, remoteproc is not quite suitable for this task.

> Does remoteproc also contain a communication framework?

it is using VirtIO

> Do you mean rpmsg? Here, I do not see how we could benefit from it.

using same message format instead of inventing new one will be really
good step:
https://github.com/OpenAMP/open-amp/wiki/RPMsg-Messaging-Protocol
(less code duplicating same functionality)

Looks like every company trying to solve the same problem over and over
again. We have point to point link between two systems. Each system has
multiple functionalities/applications so we should be able to address
this functionality. So we end to some thing with source address and
destination address. In all protocols used for inter processor/chip
communication, the difference is only the layout of 3 common fields:
source, destination and size. In many cases the ISO/OSI layer model is
badly broken and

> Can you point me to an example where rpmsg is used over SPI?

RPMsg is just transport layer, 5 or 6 wire SPI is in this case Physical
layer with flow control support. Currently i'm not sure if VirtIO with
queue support do make sense here.

> Greetings,
> Mark


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* [PATCH] net: dsa: add error handling for pskb_trim_rcsum
From: Zhouyang Jia @ 2018-06-11  5:26 UTC (permalink / raw)
  Cc: Zhouyang Jia, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	David S. Miller, netdev, linux-kernel

When pskb_trim_rcsum fails, the lack of error-handling code may
cause unexpected results.

This patch adds error-handling code after calling pskb_trim_rcsum.

Signed-off-by: Zhouyang Jia <jiazhouyang09@gmail.com>
---
 net/dsa/tag_trailer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/dsa/tag_trailer.c b/net/dsa/tag_trailer.c
index 7d20e1f..56197f0 100644
--- a/net/dsa/tag_trailer.c
+++ b/net/dsa/tag_trailer.c
@@ -75,7 +75,8 @@ static struct sk_buff *trailer_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (!skb->dev)
 		return NULL;
 
-	pskb_trim_rcsum(skb, skb->len - 4);
+	if (pskb_trim_rcsum(skb, skb->len - 4))
+		return NULL;
 
 	return skb;
 }
-- 
2.7.4

^ permalink raw reply related

* Re: WARNING: kmalloc bug in xdp_umem_create
From: Dmitry Vyukov @ 2018-06-11  5:49 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Tetsuo Handa, syzbot+4abadc5d69117b346506, Björn Töpel,
	Karlsson, Magnus, David Miller, LKML, Netdev, syzkaller-bugs
In-Reply-To: <CAJ+HfNh9pRGcd9EO7BEfPPEdCmP5EDdu_rNgLR7r4oDrcLgvQQ@mail.gmail.com>

On Sun, Jun 10, 2018 at 3:03 PM, Björn Töpel <bjorn.topel@gmail.com> wrote:
>> On 2018/06/10 20:52, Dmitry Vyukov wrote:
>> > On Sun, Jun 10, 2018 at 11:31 AM, Björn Töpel <bjorn.topel@gmail.com> wrote:
>> >> Den sön 10 juni 2018 kl 04:53 skrev Tetsuo Handa
>> >> <penguin-kernel@i-love.sakura.ne.jp>:
>> >>>
>> >>> On 2018/06/10 7:47, syzbot wrote:
>> >>>> Hello,
>> >>>>
>> >>>> syzbot found the following crash on:
>> >>>>
>> >>>> HEAD commit:    7d3bf613e99a Merge tag 'libnvdimm-for-4.18' of git://git.k..
>> >>>> git tree:       upstream
>> >>>> console output: https://syzkaller.appspot.com/x/log.txt?x=1073f68f800000
>> >>>> kernel config:  https://syzkaller.appspot.com/x/.config?x=f04d8d0a2afb789a
>> >>>> dashboard link: https://syzkaller.appspot.com/bug?extid=4abadc5d69117b346506
>> >>>> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>> >>>> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=13c9756f800000
>> >>>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16366f9f800000
>> >>>>
>> >>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> >>>> Reported-by: syzbot+4abadc5d69117b346506@syzkaller.appspotmail.com
>> >>>>
>> >>>> random: sshd: uninitialized urandom read (32 bytes read)
>> >>>> random: sshd: uninitialized urandom read (32 bytes read)
>> >>>> random: sshd: uninitialized urandom read (32 bytes read)
>> >>>> random: sshd: uninitialized urandom read (32 bytes read)
>> >>>> random: sshd: uninitialized urandom read (32 bytes read)
>> >>>> WARNING: CPU: 1 PID: 4537 at mm/slab_common.c:996 kmalloc_slab+0x56/0x70 mm/slab_common.c:996
>> >>>> Kernel panic - not syncing: panic_on_warn set ...
>> >>>
>> >>> syzbot gave up upon kmalloc(), but actually error handling path has
>> >>> NULL pointer dereference bug.
>> >>>
>> >>
>> >> Thanks Tetsuo! This crash has been fixed by Daniel Borkmann in commit
>> >> c09290c56376 ("bpf, xdp: fix crash in xdp_umem_unaccount_pages").
>> >
>> > Let's tell syzbot about this:
>> >
>> > #syz fix: bpf, xdp: fix crash in xdp_umem_unaccount_pages
>> >
>> >
>> Excuse me, but that patch fixes NULL pointer dereference which occurs after kmalloc()'s
>> "WARNING: CPU: 1 PID: 4537 at mm/slab_common.c:996 kmalloc_slab+0x56/0x70 mm/slab_common.c:996"
>> message. That is, "Too large memory allocation" itself is not yet fixed.
>
> The code relies on that the sl{u,a,o}b layer says no, and the
> setsockopt bails out. The warning could be opted out using
> __GFP_NOWARN. Is there another preferred way? Two get_user_pages
> calls, where the first call would set pages to NULL just to fault the
> region? Walk the process' VMAs? Something else?

Hi Björn,

Yes, either __GFP_NOWARN for allocations with user-controllable size
or stricter custom limit (if we don't want current sla/u/ob
implementation details to be part of public kernel interface).

^ permalink raw reply

* [PATCH] xen/netfront: raise max number of slots in xennet_get_responses()
From: Juergen Gross @ 2018-06-11  7:57 UTC (permalink / raw)
  To: linux-kernel, xen-devel, netdev; +Cc: boris.ostrovsky, davem, Juergen Gross

The max number of slots used in xennet_get_responses() is set to
MAX_SKB_FRAGS + (rx->status <= RX_COPY_THRESHOLD).

In old kernel-xen MAX_SKB_FRAGS was 18, while nowadays it is 17. This
difference is resulting in frequent messages "too many slots" and a
reduced network throughput for some workloads (factor 10 below that of
a kernel-xen based guest).

Replacing MAX_SKB_FRAGS by XEN_NETIF_NR_SLOTS_MIN for calculation of
the max number of slots to use solves that problem (tests showed no
more messages "too many slots" and throughput was as high as with the
kernel-xen based guest system).

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/net/xen-netfront.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 679da1abd73c..ba411005d829 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -790,7 +790,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
 	RING_IDX cons = queue->rx.rsp_cons;
 	struct sk_buff *skb = xennet_get_rx_skb(queue, cons);
 	grant_ref_t ref = xennet_get_rx_ref(queue, cons);
-	int max = MAX_SKB_FRAGS + (rx->status <= RX_COPY_THRESHOLD);
+	int max = XEN_NETIF_NR_SLOTS_MIN + (rx->status <= RX_COPY_THRESHOLD);
 	int slots = 1;
 	int err = 0;
 	unsigned long ret;
-- 
2.13.7

^ permalink raw reply related

* [PATCH] net: cxgb3: add error handling for some functions
From: Zhouyang Jia @ 2018-06-11  8:42 UTC (permalink / raw)
  Cc: Zhouyang Jia, Santosh Raspatur, David S. Miller, netdev,
	linux-kernel

When sysfs_create_group or alloc_skb fails, the lack of error-handling
code may cause unexpected results.

This patch adds error-handling code after the functions.

Signed-off-by: Zhouyang Jia <jiazhouyang09@gmail.com>
---
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
index 2edfdbd..bb51b7e 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
@@ -545,6 +545,8 @@ static int init_tp_parity(struct adapter *adap)
 	if (skb == adap->nofail_skb) {
 		i = await_mgmt_replies(adap, cnt, 16 + 2048 + 2048 + 1);
 		adap->nofail_skb = alloc_skb(sizeof(*greq), GFP_KERNEL);
+		if (!adap->nofail_skb)
+			goto alloc_skb_fail;
 	}
 
 	t3_tp_set_offload_mode(adap, 0);
@@ -3362,6 +3364,10 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	err = sysfs_create_group(&adapter->port[0]->dev.kobj,
 				 &cxgb3_attr_group);
+	if (err) {
+		dev_err(&pdev->dev, "cannot create sysfs group\n");
+		goto out_free_dev;
+	}
 
 	print_port_info(adapter, ai);
 	return 0;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/3] Legacy clock drivers: Normalize clk API
From: Geert Uytterhoeven @ 2018-06-11  8:44 UTC (permalink / raw)
  To: Greg Ungerer, Ralf Baechle, James Hogan, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Corentin Labbe, David S . Miller
  Cc: Arnd Bergmann, linux-m68k, linux-mips, netdev, linux-kernel,
	Geert Uytterhoeven

	Hi all,

When seeing commit bde4975310eb1982 ("net: stmmac: fix build failure due
to missing COMMON_CLK dependency"), I wondered why this dependency is
needed, as all implementations of the clock API should implement all
required functionality, or provide dummies.

It turns out there were still two implementations that lacked the
clk_set_rate() function: Coldfire and AR7.

This series contains three patches:
  - The first two patches add dummies for clk_set_rate(),
    clk_set_rate(), clk_set_parent(), and clk_get_parent() to the
    Coldfire and AR7, like Arnd has done for other legacy clock
    implementations a while ago.
  - The second patch removes the COMMON_CLK dependency from the stmmac
    network drivers again, as it is no longer needed.
    Obviously this patch has a hard dependency on the first two patches.

Thanks!

Geert Uytterhoeven (3):
  m68k: coldfire: Normalize clk API
  MIPS: AR7: Normalize clk API
  [RFC] Revert "net: stmmac: fix build failure due to missing COMMON_CLK
    dependency"

 arch/m68k/coldfire/clk.c                    | 29 +++++++++++++++++++++++++++++
 arch/mips/ar7/clock.c                       | 29 +++++++++++++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/Kconfig | 10 +++++-----
 3 files changed, 63 insertions(+), 5 deletions(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply

* [PATCH 1/3] m68k: coldfire: Normalize clk API
From: Geert Uytterhoeven @ 2018-06-11  8:44 UTC (permalink / raw)
  To: Greg Ungerer, Ralf Baechle, James Hogan, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Corentin Labbe, David S . Miller
  Cc: Arnd Bergmann, linux-m68k, linux-mips, netdev, linux-kernel,
	Geert Uytterhoeven
In-Reply-To: <1528706663-20670-1-git-send-email-geert@linux-m68k.org>

Coldfire still provides its own variant of the clk API rather than using
the generic COMMON_CLK API.  This generally works, but it causes some
link errors with drivers using the clk_round_rate(), clk_set_rate(),
clk_set_parent(), or clk_get_parent() functions when a platform lacks
those interfaces.

This adds empty stub implementations for each of them, and I don't even
try to do something useful here but instead just print a WARN() message
to make it obvious what is going on if they ever end up being called.

The drivers that call these won't be used on these platforms (otherwise
we'd get a link error today), so the added code is harmless bloat and
will warn about accidental use.

Based on commit bd7fefe1f06ca6cc ("ARM: w90x900: normalize clk API").

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/m68k/coldfire/clk.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/m68k/coldfire/clk.c b/arch/m68k/coldfire/clk.c
index 849cd208e2ed99e6..7bc666e482ebe82f 100644
--- a/arch/m68k/coldfire/clk.c
+++ b/arch/m68k/coldfire/clk.c
@@ -129,4 +129,33 @@ unsigned long clk_get_rate(struct clk *clk)
 }
 EXPORT_SYMBOL(clk_get_rate);
 
+/* dummy functions, should not be called */
+long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+	WARN_ON(clk);
+	return 0;
+}
+EXPORT_SYMBOL(clk_round_rate);
+
+int clk_set_rate(struct clk *clk, unsigned long rate)
+{
+	WARN_ON(clk);
+	return 0;
+}
+EXPORT_SYMBOL(clk_set_rate);
+
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	WARN_ON(clk);
+	return 0;
+}
+EXPORT_SYMBOL(clk_set_parent);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+	WARN_ON(clk);
+	return NULL;
+}
+EXPORT_SYMBOL(clk_get_parent);
+
 /***************************************************************************/
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/3] MIPS: AR7: Normalize clk API
From: Geert Uytterhoeven @ 2018-06-11  8:44 UTC (permalink / raw)
  To: Greg Ungerer, Ralf Baechle, James Hogan, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Corentin Labbe, David S . Miller
  Cc: Arnd Bergmann, linux-m68k, linux-mips, netdev, linux-kernel,
	Geert Uytterhoeven
In-Reply-To: <1528706663-20670-1-git-send-email-geert@linux-m68k.org>

Coldfire still provides its own variant of the clk API rather than using
the generic COMMON_CLK API.  This generally works, but it causes some
link errors with drivers using the clk_round_rate(), clk_set_rate(),
clk_set_parent(), or clk_get_parent() functions when a platform lacks
those interfaces.

This adds empty stub implementations for each of them, and I don't even
try to do something useful here but instead just print a WARN() message
to make it obvious what is going on if they ever end up being called.

The drivers that call these won't be used on these platforms (otherwise
we'd get a link error today), so the added code is harmless bloat and
will warn about accidental use.

Based on commit bd7fefe1f06ca6cc ("ARM: w90x900: normalize clk API").

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 arch/mips/ar7/clock.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/mips/ar7/clock.c b/arch/mips/ar7/clock.c
index 0137656107a9c5b5..6b64fd96dba8fb26 100644
--- a/arch/mips/ar7/clock.c
+++ b/arch/mips/ar7/clock.c
@@ -476,3 +476,32 @@ void __init ar7_init_clocks(void)
 	/* adjust vbus clock rate */
 	vbus_clk.rate = bus_clk.rate / 2;
 }
+
+/* dummy functions, should not be called */
+long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+	WARN_ON(clk);
+	return 0;
+}
+EXPORT_SYMBOL(clk_round_rate);
+
+int clk_set_rate(struct clk *clk, unsigned long rate)
+{
+	WARN_ON(clk);
+	return 0;
+}
+EXPORT_SYMBOL(clk_set_rate);
+
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	WARN_ON(clk);
+	return 0;
+}
+EXPORT_SYMBOL(clk_set_parent);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+	WARN_ON(clk);
+	return NULL;
+}
+EXPORT_SYMBOL(clk_get_parent);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/3 RFC] Revert "net: stmmac: fix build failure due to missing COMMON_CLK dependency"
From: Geert Uytterhoeven @ 2018-06-11  8:44 UTC (permalink / raw)
  To: Greg Ungerer, Ralf Baechle, James Hogan, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Corentin Labbe, David S . Miller
  Cc: Arnd Bergmann, linux-m68k, linux-mips, netdev, linux-kernel,
	Geert Uytterhoeven
In-Reply-To: <1528706663-20670-1-git-send-email-geert@linux-m68k.org>

This reverts commit bde4975310eb1982bd0bbff673989052d92fd481.

All legacy clock implementations now implement clk_set_rate() (Some
implementations may be dummies, though).

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Marked "RFC", as this depends on "m68k: coldfire: Normalize clk API" and
"MIPS: AR7: Normalize clk API".
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index cb5b0f58c395c2bd..e28c0d2c58e911ed 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -33,7 +33,7 @@ config DWMAC_DWC_QOS_ETH
 	select PHYLIB
 	select CRC32
 	select MII
-	depends on OF && COMMON_CLK && HAS_DMA
+	depends on OF && HAS_DMA
 	help
 	  Support for chips using the snps,dwc-qos-ethernet.txt DT binding.
 
@@ -57,7 +57,7 @@ config DWMAC_ANARION
 config DWMAC_IPQ806X
 	tristate "QCA IPQ806x DWMAC support"
 	default ARCH_QCOM
-	depends on OF && COMMON_CLK && (ARCH_QCOM || COMPILE_TEST)
+	depends on OF && (ARCH_QCOM || COMPILE_TEST)
 	select MFD_SYSCON
 	help
 	  Support for QCA IPQ806X DWMAC Ethernet.
@@ -100,7 +100,7 @@ config DWMAC_OXNAS
 config DWMAC_ROCKCHIP
 	tristate "Rockchip dwmac support"
 	default ARCH_ROCKCHIP
-	depends on OF && COMMON_CLK && (ARCH_ROCKCHIP || COMPILE_TEST)
+	depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
 	select MFD_SYSCON
 	help
 	  Support for Ethernet controller on Rockchip RK3288 SoC.
@@ -123,7 +123,7 @@ config DWMAC_SOCFPGA
 config DWMAC_STI
 	tristate "STi GMAC support"
 	default ARCH_STI
-	depends on OF && COMMON_CLK && (ARCH_STI || COMPILE_TEST)
+	depends on OF && (ARCH_STI || COMPILE_TEST)
 	select MFD_SYSCON
 	---help---
 	  Support for ethernet controller on STi SOCs.
@@ -147,7 +147,7 @@ config DWMAC_STM32
 config DWMAC_SUNXI
 	tristate "Allwinner GMAC support"
 	default ARCH_SUNXI
-	depends on OF && COMMON_CLK && (ARCH_SUNXI || COMPILE_TEST)
+	depends on OF && (ARCH_SUNXI || COMPILE_TEST)
 	---help---
 	  Support for Allwinner A20/A31 GMAC ethernet controllers.
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 3/3 RFC] Revert "net: stmmac: fix build failure due to missing COMMON_CLK dependency"
From: Arnd Bergmann @ 2018-06-11  8:59 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Ungerer, Ralf Baechle, James Hogan, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Corentin Labbe, David S . Miller,
	linux-m68k, open list:RALINK MIPS ARCHITECTURE, Networking,
	Linux Kernel Mailing List
In-Reply-To: <1528706663-20670-4-git-send-email-geert@linux-m68k.org>

On Mon, Jun 11, 2018 at 10:44 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> This reverts commit bde4975310eb1982bd0bbff673989052d92fd481.
>
> All legacy clock implementations now implement clk_set_rate() (Some
> implementations may be dummies, though).
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Marked "RFC", as this depends on "m68k: coldfire: Normalize clk API" and
> "MIPS: AR7: Normalize clk API".

This seems reasonable. It's possible that it will cause regressions because the
COMMON_CLK dependency hides another dependency on something else
that not everything implements, but we should fix that properly if that happens.

       Arnd

^ permalink raw reply

* Re: [PATCH 0/3] Legacy clock drivers: Normalize clk API
From: Arnd Bergmann @ 2018-06-11  9:02 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg Ungerer, Ralf Baechle, James Hogan, Giuseppe Cavallaro,
	Alexandre Torgue, Jose Abreu, Corentin Labbe, David S . Miller,
	linux-m68k, open list:RALINK MIPS ARCHITECTURE, Networking,
	Linux Kernel Mailing List
In-Reply-To: <1528706663-20670-1-git-send-email-geert@linux-m68k.org>

On Mon, Jun 11, 2018 at 10:44 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>         Hi all,
>
> When seeing commit bde4975310eb1982 ("net: stmmac: fix build failure due
> to missing COMMON_CLK dependency"), I wondered why this dependency is
> needed, as all implementations of the clock API should implement all
> required functionality, or provide dummies.
>
> It turns out there were still two implementations that lacked the
> clk_set_rate() function: Coldfire and AR7.
>
> This series contains three patches:
>   - The first two patches add dummies for clk_set_rate(),
>     clk_set_rate(), clk_set_parent(), and clk_get_parent() to the
>     Coldfire and AR7, like Arnd has done for other legacy clock
>     implementations a while ago.
>   - The second patch removes the COMMON_CLK dependency from the stmmac
>     network drivers again, as it is no longer needed.
>     Obviously this patch has a hard dependency on the first two patches.

Yes, good idea.

Acked-by: Arnd Bergmann <arnd@arnd.de>

One question: what happens on machines that don't support any CLK
interface, i.e.
that don't have any of COMMON_CLK/HAVE_CLK/CLKDEV_LOOKUP?

I guess those are already hopelessly broken for many drivers, right?

         Arnd

^ permalink raw reply


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