Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v3 3/6] net: qualcomm: rmnet: Allow only one rmnet dev per muxid per real dev
From: Subash Abhinov Kasiviswanathan @ 2017-12-12  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1513038615-16407-1-git-send-email-subashab@codeaurora.org>

Upon de-multiplexing data from one real dev, the packets can be sent
to an unique rmnet device for a given mux id.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 9caa5e3..5bb29f4 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -185,6 +185,9 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
 	if (ep->egress_dev)
 		return -EINVAL;
 
+	if (rmnet_get_endpoint(port, id))
+		return -EBUSY;
+
 	rc = register_netdevice(rmnet_dev);
 	if (!rc) {
 		ep->egress_dev = rmnet_dev;
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 4/6] net: qualcomm: rmnet: Process packets over ethernet
From: Subash Abhinov Kasiviswanathan @ 2017-12-12  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1513038615-16407-1-git-send-email-subashab@codeaurora.org>

Add support to send and receive packets over ethernet.
An example of usage is testing the data path on UML. This can be
achieved by setting up two UML instances in multicast mode and
associating rmnet over the UML ethernet device.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index a46053c..0553932 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -15,6 +15,7 @@
 
 #include <linux/netdevice.h>
 #include <linux/netdev_features.h>
+#include <linux/if_arp.h>
 #include "rmnet_private.h"
 #include "rmnet_config.h"
 #include "rmnet_vnd.h"
@@ -104,6 +105,15 @@ static void rmnet_set_skb_proto(struct sk_buff *skb)
 {
 	struct sk_buff *skbn;
 
+	if (skb->dev->type == ARPHRD_ETHER) {
+		if (pskb_expand_head(skb, ETH_HLEN, 0, GFP_KERNEL)) {
+			kfree_skb(skb);
+			return;
+		}
+
+		skb_push(skb, ETH_HLEN);
+	}
+
 	if (port->ingress_data_format & RMNET_INGRESS_FORMAT_DEAGGREGATION) {
 		while ((skbn = rmnet_map_deaggregate(skb)) != NULL)
 			__rmnet_map_ingress_handler(skbn, port);
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 5/6] net: qualcomm: rmnet: Allow to configure flags for new devices
From: Subash Abhinov Kasiviswanathan @ 2017-12-12  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1513038615-16407-1-git-send-email-subashab@codeaurora.org>

Add an option to configure the rmnet aggregation and command features
on device creation. This is achieved by using the vlan flags option.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 46bb228..7a4c26e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -177,11 +177,20 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
 	if (err)
 		goto err2;
 
-	netdev_dbg(dev, "data format [ingress 0x%08X]\n", ingress_format);
-	port->ingress_data_format = ingress_format;
 	port->rmnet_mode = mode;
 
 	hlist_add_head_rcu(&ep->hlnode, &port->muxed_ep[mux_id]);
+
+	if (data[IFLA_VLAN_FLAGS]) {
+		struct ifla_vlan_flags *flags;
+
+		flags = nla_data(data[IFLA_VLAN_FLAGS]);
+		ingress_format = flags->flags & flags->mask;
+	}
+
+	netdev_dbg(dev, "data format [ingress 0x%08X]\n", ingress_format);
+	port->ingress_data_format = ingress_format;
+
 	return 0;
 
 err2:
@@ -313,7 +322,8 @@ static int rmnet_rtnl_validate(struct nlattr *tb[], struct nlattr *data[],
 
 static size_t rmnet_get_size(const struct net_device *dev)
 {
-	return nla_total_size(2); /* IFLA_VLAN_ID */
+	return nla_total_size(2) /* IFLA_VLAN_ID */ +
+	       nla_total_size(sizeof(struct ifla_vlan_flags)); /* IFLA_VLAN_FLAGS */
 }
 
 struct rtnl_link_ops rmnet_link_ops __read_mostly = {
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v3 6/6] net: qualcomm: rmnet: Allow to configure flags for existing devices
From: Subash Abhinov Kasiviswanathan @ 2017-12-12  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1513038615-16407-1-git-send-email-subashab@codeaurora.org>

Add an option to configure the mux id, aggregation and commad feature
for existing rmnet devices. Implement the changelink netlink
operation for this.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 7a4c26e..cedacdd 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -320,6 +320,45 @@ static int rmnet_rtnl_validate(struct nlattr *tb[], struct nlattr *data[],
 	return 0;
 }
 
+static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
+			    struct nlattr *data[],
+			    struct netlink_ext_ack *extack)
+{
+	struct rmnet_priv *priv = netdev_priv(dev);
+	struct net_device *real_dev;
+	struct rmnet_endpoint *ep;
+	struct rmnet_port *port;
+	u16 mux_id;
+
+	real_dev = __dev_get_by_index(dev_net(dev),
+				      nla_get_u32(tb[IFLA_LINK]));
+
+	if (!real_dev || !dev || !rmnet_is_real_dev_registered(real_dev))
+		return -ENODEV;
+
+	port = rmnet_get_port_rtnl(real_dev);
+
+	if (data[IFLA_VLAN_ID]) {
+		mux_id = nla_get_u16(data[IFLA_VLAN_ID]);
+		ep = rmnet_get_endpoint(port, priv->mux_id);
+
+		hlist_del_init_rcu(&ep->hlnode);
+		hlist_add_head_rcu(&ep->hlnode, &port->muxed_ep[mux_id]);
+
+		ep->mux_id = mux_id;
+		priv->mux_id = mux_id;
+	}
+
+	if (data[IFLA_VLAN_FLAGS]) {
+		struct ifla_vlan_flags *flags;
+
+		flags = nla_data(data[IFLA_VLAN_FLAGS]);
+		port->ingress_data_format = flags->flags & flags->mask;
+	}
+
+	return 0;
+}
+
 static size_t rmnet_get_size(const struct net_device *dev)
 {
 	return nla_total_size(2) /* IFLA_VLAN_ID */ +
@@ -335,6 +374,7 @@ struct rtnl_link_ops rmnet_link_ops __read_mostly = {
 	.newlink	= rmnet_newlink,
 	.dellink	= rmnet_dellink,
 	.get_size	= rmnet_get_size,
+	.changelink     = rmnet_changelink,
 };
 
 /* Needs either rcu_read_lock() or rtnl lock */
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] igb: Free IRQs when device is hotplugged
From: Stephen Hemminger @ 2017-12-12  0:34 UTC (permalink / raw)
  To: Lyude Paul
  Cc: intel-wired-lan, Todd Fujinaka, stable, Jeff Kirsher, netdev,
	linux-kernel
In-Reply-To: <20171211234502.27445-1-lyude@redhat.com>

On Mon, 11 Dec 2017 18:45:02 -0500
Lyude Paul <lyude@redhat.com> wrote:

> Recently I got a Caldigit TS3 Thunderbolt 3 dock, and noticed that upon
> hotplugging my kernel would immediately crash due to igb:
> 
> [  680.825801] kernel BUG at drivers/pci/msi.c:352!
> [  680.828388] invalid opcode: 0000 [#1] SMP
> [  680.829194] Modules linked in: igb(O) thunderbolt i2c_algo_bit joydev vfat fat btusb btrtl btbcm btintel bluetooth ecdh_generic hp_wmi sparse_keymap rfkill wmi_bmof iTCO_wdt intel_rapl x86_pkg_temp_thermal coretemp crc32_pclmul snd_pcm rtsx_pci_ms mei_me snd_timer memstick snd pcspkr mei soundcore i2c_i801 tpm_tis psmouse shpchp wmi tpm_tis_core tpm video hp_wireless acpi_pad rtsx_pci_sdmmc mmc_core crc32c_intel serio_raw rtsx_pci mfd_core xhci_pci xhci_hcd i2c_hid i2c_core [last unloaded: igb]
> [  680.831085] CPU: 1 PID: 78 Comm: kworker/u16:1 Tainted: G           O     4.15.0-rc3Lyude-Test+ #6
> [  680.831596] Hardware name: HP HP ZBook Studio G4/826B, BIOS P71 Ver. 01.03 06/09/2017
> [  680.832168] Workqueue: kacpi_hotplug acpi_hotplug_work_fn
> [  680.832687] RIP: 0010:free_msi_irqs+0x180/0x1b0
> [  680.833271] RSP: 0018:ffffc9000030fbf0 EFLAGS: 00010286
> [  680.833761] RAX: ffff8803405f9c00 RBX: ffff88033e3d2e40 RCX: 000000000000002c
> [  680.834278] RDX: 0000000000000000 RSI: 00000000000000ac RDI: ffff880340be2178
> [  680.834832] RBP: 0000000000000000 R08: ffff880340be1ff0 R09: ffff8803405f9c00
> [  680.835342] R10: 0000000000000000 R11: 0000000000000040 R12: ffff88033d63a298
> [  680.835822] R13: ffff88033d63a000 R14: 0000000000000060 R15: ffff880341959000
> [  680.836332] FS:  0000000000000000(0000) GS:ffff88034f440000(0000) knlGS:0000000000000000
> [  680.836817] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  680.837360] CR2: 000055e64044afdf CR3: 0000000001c09002 CR4: 00000000003606e0
> [  680.837954] Call Trace:
> [  680.838853]  pci_disable_msix+0xce/0xf0
> [  680.839616]  igb_reset_interrupt_capability+0x5d/0x60 [igb]
> [  680.840278]  igb_remove+0x9d/0x110 [igb]
> [  680.840764]  pci_device_remove+0x36/0xb0
> [  680.841279]  device_release_driver_internal+0x157/0x220
> [  680.841739]  pci_stop_bus_device+0x7d/0xa0
> [  680.842255]  pci_stop_bus_device+0x2b/0xa0
> [  680.842722]  pci_stop_bus_device+0x3d/0xa0
> [  680.843189]  pci_stop_and_remove_bus_device+0xe/0x20
> [  680.843627]  trim_stale_devices+0xf3/0x140
> [  680.844086]  trim_stale_devices+0x94/0x140
> [  680.844532]  trim_stale_devices+0xa6/0x140
> [  680.845031]  ? get_slot_status+0x90/0xc0
> [  680.845536]  acpiphp_check_bridge.part.5+0xfe/0x140
> [  680.846021]  acpiphp_hotplug_notify+0x175/0x200
> [  680.846581]  ? free_bridge+0x100/0x100
> [  680.847113]  acpi_device_hotplug+0x8a/0x490
> [  680.847535]  acpi_hotplug_work_fn+0x1a/0x30
> [  680.848076]  process_one_work+0x182/0x3a0
> [  680.848543]  worker_thread+0x2e/0x380
> [  680.848963]  ? process_one_work+0x3a0/0x3a0
> [  680.849373]  kthread+0x111/0x130
> [  680.849776]  ? kthread_create_worker_on_cpu+0x50/0x50
> [  680.850188]  ret_from_fork+0x1f/0x30
> [  680.850601] Code: 43 14 85 c0 0f 84 d5 fe ff ff 31 ed eb 0f 83 c5 01 39 6b 14 0f 86 c5 fe ff ff 8b 7b 10 01 ef e8 b7 e4 d2 ff 48 83 78 70 00 74 e3 <0f> 0b 49 8d b5 a0 00 00 00 e8 62 6f d3 ff e9 c7 fe ff ff 48 8b
> [  680.851497] RIP: free_msi_irqs+0x180/0x1b0 RSP: ffffc9000030fbf0
> 
> As it turns out, normally the freeing of IRQs that would fix this is called
> inside of the scope of __igb_close(). However, since the device is
> already gone by the point we try to unregister the netdevice from the
> driver due to a hotplug we end up seeing that the netif isn't present
> and thus, forget to free any of the device IRQs.
> 
> So: after unregistering the netdev in igb_remove() check whether the PCI
> device is stale and if so, free it's IRQs and tx/rx resources.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Fixes: 9474933caf21 ("igb: close/suspend race in netif_device_detach")
> Cc: Todd Fujinaka <todd.fujinaka@intel.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index c208753ff5b7..e650348b4bd7 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -3325,6 +3325,16 @@ static void igb_remove(struct pci_dev *pdev)
>  
>  	unregister_netdev(netdev);
>  
> +	/* If the PCI device has already been physically removed (e.g. user
> +	 * unplugged a thunderbolt dock containing our hw) then the netif will
> +	 * already be down, so unregistering the netdev won't free the IRQs
> +	 */
> +	if (!pci_device_is_present(pdev)) {
> +		igb_free_irq(adapter);
> +		igb_free_all_tx_resources(adapter);
> +		igb_free_all_rx_resources(adapter);
> +	}
> +
>  	igb_clear_interrupt_scheme(adapter);
>  
>  	pci_iounmap(pdev, adapter->io_addr);

This looks like you are making a special case out of something that should
be fixed in igb_close instead.

Most likely something is wrong with earlier commit.

commit 9474933caf21a4cb5147223dca1551f527aaac36
Author: Todd Fujinaka <todd.fujinaka@intel.com>
Date:   Tue Nov 15 08:54:26 2016 -0800

    igb: close/suspend race in netif_device_detach
    
    Similar to ixgbe, when an interface is part of a namespace it is
    possible that igb_close() may be called while __igb_shutdown() is
    running which ends up in a double free WARN and/or a BUG in
    free_msi_irqs().
    
    Extend the rtnl_lock() to protect the call to netif_device_detach() and
    igb_clear_interrupt_scheme() in __igb_shutdown() and check for
    netif_device_present() to avoid calling igb_clear_interrupt_scheme() a
    second time in igb_close().
    
    Also extend the rtnl lock in igb_resume() to netif_device_attach().
    
    Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
    Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
    Tested-by: Aaron Brown <aaron.f.brown@intel.com>
    Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>


Layers on layers of workarounds does not lead to stability.

^ permalink raw reply

* RE: [PATCH] Fix handling of verdicts after NF_QUEUE
From: Banerjee, Debabrata @ 2017-12-12  0:36 UTC (permalink / raw)
  To: 'Pablo Neira Ayuso'
  Cc: David S . Miller, netfilter-devel@vger.kernel.org,
	coreteam@netfilter.org, netdev@vger.kernel.org,
	stable@vger.kernel.org
In-Reply-To: <20171212002318.GA7681@salvia>

> From: Pablo Neira Ayuso [mailto:pablo@netfilter.org]
> On Mon, Dec 11, 2017 at 06:30:24PM -0500, Debabrata Banerjee wrote:
> > +	} else {
> > +		/* Implicit handling for NF_STOLEN, as well as any other
> > +		 * non conventional verdicts.
> > +		 */
> > +		ret = 0;
> 
> Another possibility (more simple?) would be this:
> 
> int nf_hook_slow(struct sk_buff *skb, struct nf_hook_state *state) {
>         struct nf_hook_entry *entry;
>         unsigned int verdict;
> -       int ret = 0;
> +       int ret;
> 
>         entry = rcu_dereference(state->hook_entries);
> next_hook:
> +       ret = 0;
> 
> Basically, make sure ret is set to zero when jumping to the next_hook label.

Many ways to fix it, but I thought including the comment was appropriate.
Happy to change it if we want simpler instead.

-Deb

^ permalink raw reply

* Re: [PATCH] igb: Free IRQs when device is hotplugged
From: Lyude Paul @ 2017-12-12  0:37 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: intel-wired-lan, Todd Fujinaka, stable, Jeff Kirsher, netdev,
	linux-kernel
In-Reply-To: <20171211163422.07d538fb@xeon-e3>

On Mon, 2017-12-11 at 16:34 -0800, Stephen Hemminger wrote:
> On Mon, 11 Dec 2017 18:45:02 -0500
> Lyude Paul <lyude@redhat.com> wrote:
> 
> > Recently I got a Caldigit TS3 Thunderbolt 3 dock, and noticed that upon
> > hotplugging my kernel would immediately crash due to igb:
> > 
> > [  680.825801] kernel BUG at drivers/pci/msi.c:352!
> > [  680.828388] invalid opcode: 0000 [#1] SMP
> > [  680.829194] Modules linked in: igb(O) thunderbolt i2c_algo_bit joydev
> > vfat fat btusb btrtl btbcm btintel bluetooth ecdh_generic hp_wmi
> > sparse_keymap rfkill wmi_bmof iTCO_wdt intel_rapl x86_pkg_temp_thermal
> > coretemp crc32_pclmul snd_pcm rtsx_pci_ms mei_me snd_timer memstick snd
> > pcspkr mei soundcore i2c_i801 tpm_tis psmouse shpchp wmi tpm_tis_core tpm
> > video hp_wireless acpi_pad rtsx_pci_sdmmc mmc_core crc32c_intel serio_raw
> > rtsx_pci mfd_core xhci_pci xhci_hcd i2c_hid i2c_core [last unloaded: igb]
> > [  680.831085] CPU: 1 PID: 78 Comm: kworker/u16:1 Tainted:
> > G           O     4.15.0-rc3Lyude-Test+ #6
> > [  680.831596] Hardware name: HP HP ZBook Studio G4/826B, BIOS P71 Ver.
> > 01.03 06/09/2017
> > [  680.832168] Workqueue: kacpi_hotplug acpi_hotplug_work_fn
> > [  680.832687] RIP: 0010:free_msi_irqs+0x180/0x1b0
> > [  680.833271] RSP: 0018:ffffc9000030fbf0 EFLAGS: 00010286
> > [  680.833761] RAX: ffff8803405f9c00 RBX: ffff88033e3d2e40 RCX:
> > 000000000000002c
> > [  680.834278] RDX: 0000000000000000 RSI: 00000000000000ac RDI:
> > ffff880340be2178
> > [  680.834832] RBP: 0000000000000000 R08: ffff880340be1ff0 R09:
> > ffff8803405f9c00
> > [  680.835342] R10: 0000000000000000 R11: 0000000000000040 R12:
> > ffff88033d63a298
> > [  680.835822] R13: ffff88033d63a000 R14: 0000000000000060 R15:
> > ffff880341959000
> > [  680.836332] FS:  0000000000000000(0000) GS:ffff88034f440000(0000)
> > knlGS:0000000000000000
> > [  680.836817] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  680.837360] CR2: 000055e64044afdf CR3: 0000000001c09002 CR4:
> > 00000000003606e0
> > [  680.837954] Call Trace:
> > [  680.838853]  pci_disable_msix+0xce/0xf0
> > [  680.839616]  igb_reset_interrupt_capability+0x5d/0x60 [igb]
> > [  680.840278]  igb_remove+0x9d/0x110 [igb]
> > [  680.840764]  pci_device_remove+0x36/0xb0
> > [  680.841279]  device_release_driver_internal+0x157/0x220
> > [  680.841739]  pci_stop_bus_device+0x7d/0xa0
> > [  680.842255]  pci_stop_bus_device+0x2b/0xa0
> > [  680.842722]  pci_stop_bus_device+0x3d/0xa0
> > [  680.843189]  pci_stop_and_remove_bus_device+0xe/0x20
> > [  680.843627]  trim_stale_devices+0xf3/0x140
> > [  680.844086]  trim_stale_devices+0x94/0x140
> > [  680.844532]  trim_stale_devices+0xa6/0x140
> > [  680.845031]  ? get_slot_status+0x90/0xc0
> > [  680.845536]  acpiphp_check_bridge.part.5+0xfe/0x140
> > [  680.846021]  acpiphp_hotplug_notify+0x175/0x200
> > [  680.846581]  ? free_bridge+0x100/0x100
> > [  680.847113]  acpi_device_hotplug+0x8a/0x490
> > [  680.847535]  acpi_hotplug_work_fn+0x1a/0x30
> > [  680.848076]  process_one_work+0x182/0x3a0
> > [  680.848543]  worker_thread+0x2e/0x380
> > [  680.848963]  ? process_one_work+0x3a0/0x3a0
> > [  680.849373]  kthread+0x111/0x130
> > [  680.849776]  ? kthread_create_worker_on_cpu+0x50/0x50
> > [  680.850188]  ret_from_fork+0x1f/0x30
> > [  680.850601] Code: 43 14 85 c0 0f 84 d5 fe ff ff 31 ed eb 0f 83 c5 01 39
> > 6b 14 0f 86 c5 fe ff ff 8b 7b 10 01 ef e8 b7 e4 d2 ff 48 83 78 70 00 74 e3
> > <0f> 0b 49 8d b5 a0 00 00 00 e8 62 6f d3 ff e9 c7 fe ff ff 48 8b
> > [  680.851497] RIP: free_msi_irqs+0x180/0x1b0 RSP: ffffc9000030fbf0
> > 
> > As it turns out, normally the freeing of IRQs that would fix this is called
> > inside of the scope of __igb_close(). However, since the device is
> > already gone by the point we try to unregister the netdevice from the
> > driver due to a hotplug we end up seeing that the netif isn't present
> > and thus, forget to free any of the device IRQs.
> > 
> > So: after unregistering the netdev in igb_remove() check whether the PCI
> > device is stale and if so, free it's IRQs and tx/rx resources.
> > 
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > Fixes: 9474933caf21 ("igb: close/suspend race in netif_device_detach")
> > Cc: Todd Fujinaka <todd.fujinaka@intel.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/net/ethernet/intel/igb/igb_main.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> > b/drivers/net/ethernet/intel/igb/igb_main.c
> > index c208753ff5b7..e650348b4bd7 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_main.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> > @@ -3325,6 +3325,16 @@ static void igb_remove(struct pci_dev *pdev)
> >  
> >  	unregister_netdev(netdev);
> >  
> > +	/* If the PCI device has already been physically removed (e.g. user
> > +	 * unplugged a thunderbolt dock containing our hw) then the netif
> > will
> > +	 * already be down, so unregistering the netdev won't free the IRQs
> > +	 */
> > +	if (!pci_device_is_present(pdev)) {
> > +		igb_free_irq(adapter);
> > +		igb_free_all_tx_resources(adapter);
> > +		igb_free_all_rx_resources(adapter);
> > +	}
> > +
> >  	igb_clear_interrupt_scheme(adapter);
> >  
> >  	pci_iounmap(pdev, adapter->io_addr);
> 
> This looks like you are making a special case out of something that should
> be fixed in igb_close instead.
> 
> Most likely something is wrong with earlier commit.
> 
> commit 9474933caf21a4cb5147223dca1551f527aaac36
> Author: Todd Fujinaka <todd.fujinaka@intel.com>
> Date:   Tue Nov 15 08:54:26 2016 -0800
> 
>     igb: close/suspend race in netif_device_detach
>     
>     Similar to ixgbe, when an interface is part of a namespace it is
>     possible that igb_close() may be called while __igb_shutdown() is
>     running which ends up in a double free WARN and/or a BUG in
>     free_msi_irqs().
>     
>     Extend the rtnl_lock() to protect the call to netif_device_detach() and
>     igb_clear_interrupt_scheme() in __igb_shutdown() and check for
>     netif_device_present() to avoid calling igb_clear_interrupt_scheme() a
>     second time in igb_close().
>     
>     Also extend the rtnl lock in igb_resume() to netif_device_attach().
>     
>     Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
>     Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
>     Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>     Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> 
> 
> Layers on layers of workarounds does not lead to stability.'

Agreed, although it should be noted though I mentioned that exact commit in the
patch I sent :P. Will rework patch and send out a new version.

> 

^ permalink raw reply

* [PATCH iproute2 net-next v2 0/4] Abstract columns, properly space and wrap fields
From: Stefano Brivio @ 2017-12-12  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca

Currently, 'ss' simply subdivides the whole available screen width
between available columns, starting from a set of hardcoded amount
of spacing and growing column widths.

This makes the output unreadable in several cases, as it doesn't take
into account the actual content width.

Fix this by introducing a simple abstraction for columns, buffering
the output, measuring the width of the fields, grouping fields into
lines as they fit, equally distributing any remaining whitespace, and
finally rendering the result. Some examples are reported below [1].

This implementation doesn't seem to cause any significant performance
issues, as reported in 3/4.

Patch 1/4 replaces all relevant printf() calls by the out() helper,
which simply consists of the usual printf() implementation.

Patch 2/4 implements column abstraction, with configurable column
width and delimiters, and 3/4 splits buffering and rendering phases,
employing a simple buffering mechanism with chunked allocation and
introducing a rendering function.

Up to this point, the output is still unchanged.

Finally, 4/4 introduces field width calculation based on content
length measured while buffering, in order to split fields onto
multiple lines and equally space them within the single lines.

Now that column behaviour is well-defined and more easily
configurable, it should be easier to further improve the output by
splitting logically separable information (e.g. TCP details) into
additional columns. However, this patchset keeps the full "extended"
information into a single column, for the moment being.


v2: rebase after conflict with 00ac78d39c29 ("ss: print tcpi_rcv_ssthresh")


[1]

- 80 columns terminal, ss -Z -f netlink
  * before:
Recv-Q Send-Q Local Address:Port                 Peer Address:Port

0      0            rtnl:evolution-calen/2075           *                     pr
oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0      0            rtnl:abrt-applet/32700              *                     pr
oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0      0            rtnl:firefox/21619                  *                     pr
oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0      0            rtnl:evolution-calen/32639           *                     p
roc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
[...]

  * after:
Recv-Q   Send-Q     Local Address:Port                      Peer Address:Port
0        0                   rtnl:evolution-calen/2075                  *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0        0                   rtnl:abrt-applet/32700                     *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0        0                   rtnl:firefox/21619                         *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0        0                   rtnl:evolution-calen/32639                 *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
[...]

- 80 columns terminal, ss -tunpl
  * before:
Netid  State      Recv-Q Send-Q Local Address:Port               Peer Address:Port
udp    UNCONN     0      0         *:37732                 *:*
udp    UNCONN     0      0         *:5353                  *:*
udp    UNCONN     0      0      192.168.122.1:53                    *:*
udp    UNCONN     0      0      *%virbr0:67                    *:*
[...]

  * after:
Netid   State    Recv-Q   Send-Q     Local Address:Port      Peer Address:Port
udp     UNCONN   0        0                      *:37732                *:*
udp     UNCONN   0        0                      *:5353                 *:*
udp     UNCONN   0        0          192.168.122.1:53                   *:*
udp     UNCONN   0        0               *%virbr0:67                   *:*
[...]

 - 66 columns terminal, ss -tunpl
  * before:
Netid  State      Recv-Q Send-Q Local Address:Port               P
eer Address:Port
udp    UNCONN     0      0       *:37732               *:*

udp    UNCONN     0      0       *:5353                *:*

udp    UNCONN     0      0      192.168.122.1:53
*:*
udp    UNCONN     0      0      *%virbr0:67                  *:*
[...]

  * after:
Netid State  Recv-Q Send-Q Local Address:Port   Peer Address:Port
udp   UNCONN 0      0                  *:37732             *:*
udp   UNCONN 0      0                  *:5353              *:*
udp   UNCONN 0      0      192.168.122.1:53                *:*
udp   UNCONN 0      0           *%virbr0:67                *:*
[...]


Stefano Brivio (4):
  ss: Replace printf() calls for "main" output by calls to helper
  ss: Introduce columns lightweight abstraction
  ss: Buffer raw fields first, then render them as a table
  ss: Implement automatic column width calculation

 misc/ss.c | 895 +++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 621 insertions(+), 274 deletions(-)

-- 
2.9.4

^ permalink raw reply

* [PATCH iproute2 net-next v2 1/4] ss: Replace printf() calls for "main" output by calls to helper
From: Stefano Brivio @ 2017-12-12  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca
In-Reply-To: <cover.1513039237.git.sbrivio@redhat.com>

This is preparation work for output buffering, which will allow
us to use optimal spacing and alignment of logical "columns".

The new out() function is just a re-implementation of a typical
libc's printf(), except that the return value of vfprintf() is
ignored as no callers use it. This implementation will be
replaced in the next patches to provide column width adjustment
and adequate spacing.

All printf() calls that output parts of the socket list are now
replaced by calls to out(). Output of summary and version is
excluded from this.

No functional differences here, output not affected.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: rebase after conflict with 00ac78d39c29 ("ss: print tcpi_rcv_ssthresh")

 misc/ss.c | 399 ++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 205 insertions(+), 194 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index da52d5edeb7e..a7d3b89e1478 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -26,6 +26,7 @@
 #include <getopt.h>
 #include <stdbool.h>
 #include <limits.h>
+#include <stdarg.h>
 
 #include "utils.h"
 #include "rt_names.h"
@@ -823,6 +824,15 @@ static const char *vsock_netid_name(int type)
 	}
 }
 
+static void out(const char *fmt, ...)
+{
+	va_list args;
+
+	va_start(args, fmt);
+	vfprintf(stdout, fmt, args);
+	va_end(args);
+}
+
 static void sock_state_print(struct sockstat *s)
 {
 	const char *sock_name;
@@ -863,39 +873,39 @@ static void sock_state_print(struct sockstat *s)
 	}
 
 	if (netid_width)
-		printf("%-*s ", netid_width,
-		       is_sctp_assoc(s, sock_name) ? "" : sock_name);
+		out("%-*s ", netid_width,
+		    is_sctp_assoc(s, sock_name) ? "" : sock_name);
 	if (state_width) {
 		if (is_sctp_assoc(s, sock_name))
-			printf("`- %-*s ", state_width - 3,
-			       sctp_sstate_name[s->state]);
+			out("`- %-*s ", state_width - 3,
+			    sctp_sstate_name[s->state]);
 		else
-			printf("%-*s ", state_width, sstate_name[s->state]);
+			out("%-*s ", state_width, sstate_name[s->state]);
 	}
 
-	printf("%-6d %-6d %s", s->rq, s->wq, odd_width_pad);
+	out("%-6d %-6d %s", s->rq, s->wq, odd_width_pad);
 }
 
 static void sock_details_print(struct sockstat *s)
 {
 	if (s->uid)
-		printf(" uid:%u", s->uid);
+		out(" uid:%u", s->uid);
 
-	printf(" ino:%u", s->ino);
-	printf(" sk:%llx", s->sk);
+	out(" ino:%u", s->ino);
+	out(" sk:%llx", s->sk);
 
 	if (s->mark)
-		printf(" fwmark:0x%x", s->mark);
+		out(" fwmark:0x%x", s->mark);
 }
 
 static void sock_addr_print_width(int addr_len, const char *addr, char *delim,
 		int port_len, const char *port, const char *ifname)
 {
 	if (ifname) {
-		printf("%*s%%%s%s%-*s ", addr_len, addr, ifname, delim,
-				port_len, port);
+		out("%*s%%%s%s%-*s ", addr_len, addr, ifname, delim,
+		    port_len, port);
 	} else {
-		printf("%*s%s%-*s ", addr_len, addr, delim, port_len, port);
+		out("%*s%s%-*s ", addr_len, addr, delim, port_len, port);
 	}
 }
 
@@ -1793,12 +1803,12 @@ static void proc_ctx_print(struct sockstat *s)
 		if (find_entry(s->ino, &buf,
 				(show_proc_ctx & show_sock_ctx) ?
 				PROC_SOCK_CTX : PROC_CTX) > 0) {
-			printf(" users:(%s)", buf);
+			out(" users:(%s)", buf);
 			free(buf);
 		}
 	} else if (show_users) {
 		if (find_entry(s->ino, &buf, USERS) > 0) {
-			printf(" users:(%s)", buf);
+			out(" users:(%s)", buf);
 			free(buf);
 		}
 	}
@@ -1878,51 +1888,51 @@ static char *sprint_bw(char *buf, double bw)
 static void sctp_stats_print(struct sctp_info *s)
 {
 	if (s->sctpi_tag)
-		printf(" tag:%x", s->sctpi_tag);
+		out(" tag:%x", s->sctpi_tag);
 	if (s->sctpi_state)
-		printf(" state:%s", sctp_sstate_name[s->sctpi_state]);
+		out(" state:%s", sctp_sstate_name[s->sctpi_state]);
 	if (s->sctpi_rwnd)
-		printf(" rwnd:%d", s->sctpi_rwnd);
+		out(" rwnd:%d", s->sctpi_rwnd);
 	if (s->sctpi_unackdata)
-		printf(" unackdata:%d", s->sctpi_unackdata);
+		out(" unackdata:%d", s->sctpi_unackdata);
 	if (s->sctpi_penddata)
-		printf(" penddata:%d", s->sctpi_penddata);
+		out(" penddata:%d", s->sctpi_penddata);
 	if (s->sctpi_instrms)
-		printf(" instrms:%d", s->sctpi_instrms);
+		out(" instrms:%d", s->sctpi_instrms);
 	if (s->sctpi_outstrms)
-		printf(" outstrms:%d", s->sctpi_outstrms);
+		out(" outstrms:%d", s->sctpi_outstrms);
 	if (s->sctpi_inqueue)
-		printf(" inqueue:%d", s->sctpi_inqueue);
+		out(" inqueue:%d", s->sctpi_inqueue);
 	if (s->sctpi_outqueue)
-		printf(" outqueue:%d", s->sctpi_outqueue);
+		out(" outqueue:%d", s->sctpi_outqueue);
 	if (s->sctpi_overall_error)
-		printf(" overerr:%d", s->sctpi_overall_error);
+		out(" overerr:%d", s->sctpi_overall_error);
 	if (s->sctpi_max_burst)
-		printf(" maxburst:%d", s->sctpi_max_burst);
+		out(" maxburst:%d", s->sctpi_max_burst);
 	if (s->sctpi_maxseg)
-		printf(" maxseg:%d", s->sctpi_maxseg);
+		out(" maxseg:%d", s->sctpi_maxseg);
 	if (s->sctpi_peer_rwnd)
-		printf(" prwnd:%d", s->sctpi_peer_rwnd);
+		out(" prwnd:%d", s->sctpi_peer_rwnd);
 	if (s->sctpi_peer_tag)
-		printf(" ptag:%x", s->sctpi_peer_tag);
+		out(" ptag:%x", s->sctpi_peer_tag);
 	if (s->sctpi_peer_capable)
-		printf(" pcapable:%d", s->sctpi_peer_capable);
+		out(" pcapable:%d", s->sctpi_peer_capable);
 	if (s->sctpi_peer_sack)
-		printf(" psack:%d", s->sctpi_peer_sack);
+		out(" psack:%d", s->sctpi_peer_sack);
 	if (s->sctpi_s_autoclose)
-		printf(" autoclose:%d", s->sctpi_s_autoclose);
+		out(" autoclose:%d", s->sctpi_s_autoclose);
 	if (s->sctpi_s_adaptation_ind)
-		printf(" adapind:%d", s->sctpi_s_adaptation_ind);
+		out(" adapind:%d", s->sctpi_s_adaptation_ind);
 	if (s->sctpi_s_pd_point)
-		printf(" pdpoint:%d", s->sctpi_s_pd_point);
+		out(" pdpoint:%d", s->sctpi_s_pd_point);
 	if (s->sctpi_s_nodelay)
-		printf(" nodealy:%d", s->sctpi_s_nodelay);
+		out(" nodealy:%d", s->sctpi_s_nodelay);
 	if (s->sctpi_s_disable_fragments)
-		printf(" nofrag:%d", s->sctpi_s_disable_fragments);
+		out(" nofrag:%d", s->sctpi_s_disable_fragments);
 	if (s->sctpi_s_v4mapped)
-		printf(" v4mapped:%d", s->sctpi_s_v4mapped);
+		out(" v4mapped:%d", s->sctpi_s_v4mapped);
 	if (s->sctpi_s_frag_interleave)
-		printf(" fraginl:%d", s->sctpi_s_frag_interleave);
+		out(" fraginl:%d", s->sctpi_s_frag_interleave);
 }
 
 static void tcp_stats_print(struct tcpstat *s)
@@ -1930,65 +1940,65 @@ static void tcp_stats_print(struct tcpstat *s)
 	char b1[64];
 
 	if (s->has_ts_opt)
-		printf(" ts");
+		out(" ts");
 	if (s->has_sack_opt)
-		printf(" sack");
+		out(" sack");
 	if (s->has_ecn_opt)
-		printf(" ecn");
+		out(" ecn");
 	if (s->has_ecnseen_opt)
-		printf(" ecnseen");
+		out(" ecnseen");
 	if (s->has_fastopen_opt)
-		printf(" fastopen");
+		out(" fastopen");
 	if (s->cong_alg[0])
-		printf(" %s", s->cong_alg);
+		out(" %s", s->cong_alg);
 	if (s->has_wscale_opt)
-		printf(" wscale:%d,%d", s->snd_wscale, s->rcv_wscale);
+		out(" wscale:%d,%d", s->snd_wscale, s->rcv_wscale);
 	if (s->rto)
-		printf(" rto:%g", s->rto);
+		out(" rto:%g", s->rto);
 	if (s->backoff)
-		printf(" backoff:%u", s->backoff);
+		out(" backoff:%u", s->backoff);
 	if (s->rtt)
-		printf(" rtt:%g/%g", s->rtt, s->rttvar);
+		out(" rtt:%g/%g", s->rtt, s->rttvar);
 	if (s->ato)
-		printf(" ato:%g", s->ato);
+		out(" ato:%g", s->ato);
 
 	if (s->qack)
-		printf(" qack:%d", s->qack);
+		out(" qack:%d", s->qack);
 	if (s->qack & 1)
-		printf(" bidir");
+		out(" bidir");
 
 	if (s->mss)
-		printf(" mss:%d", s->mss);
+		out(" mss:%d", s->mss);
 	if (s->rcv_mss)
-		printf(" rcvmss:%d", s->rcv_mss);
+		out(" rcvmss:%d", s->rcv_mss);
 	if (s->advmss)
-		printf(" advmss:%d", s->advmss);
+		out(" advmss:%d", s->advmss);
 	if (s->cwnd)
-		printf(" cwnd:%u", s->cwnd);
+		out(" cwnd:%u", s->cwnd);
 	if (s->ssthresh)
-		printf(" ssthresh:%d", s->ssthresh);
+		out(" ssthresh:%d", s->ssthresh);
 
 	if (s->bytes_acked)
-		printf(" bytes_acked:%llu", s->bytes_acked);
+		out(" bytes_acked:%llu", s->bytes_acked);
 	if (s->bytes_received)
-		printf(" bytes_received:%llu", s->bytes_received);
+		out(" bytes_received:%llu", s->bytes_received);
 	if (s->segs_out)
-		printf(" segs_out:%u", s->segs_out);
+		out(" segs_out:%u", s->segs_out);
 	if (s->segs_in)
-		printf(" segs_in:%u", s->segs_in);
+		out(" segs_in:%u", s->segs_in);
 	if (s->data_segs_out)
-		printf(" data_segs_out:%u", s->data_segs_out);
+		out(" data_segs_out:%u", s->data_segs_out);
 	if (s->data_segs_in)
-		printf(" data_segs_in:%u", s->data_segs_in);
+		out(" data_segs_in:%u", s->data_segs_in);
 
 	if (s->dctcp && s->dctcp->enabled) {
 		struct dctcpstat *dctcp = s->dctcp;
 
-		printf(" dctcp:(ce_state:%u,alpha:%u,ab_ecn:%u,ab_tot:%u)",
-				dctcp->ce_state, dctcp->alpha, dctcp->ab_ecn,
-				dctcp->ab_tot);
+		out(" dctcp:(ce_state:%u,alpha:%u,ab_ecn:%u,ab_tot:%u)",
+			     dctcp->ce_state, dctcp->alpha, dctcp->ab_ecn,
+			     dctcp->ab_tot);
 	} else if (s->dctcp) {
-		printf(" dctcp:fallback_mode");
+		out(" dctcp:fallback_mode");
 	}
 
 	if (s->bbr_info) {
@@ -1998,73 +2008,72 @@ static void tcp_stats_print(struct tcpstat *s)
 		bw <<= 32;
 		bw |= s->bbr_info->bbr_bw_lo;
 
-		printf(" bbr:(bw:%sbps,mrtt:%g",
-		       sprint_bw(b1, bw * 8.0),
-		       (double)s->bbr_info->bbr_min_rtt / 1000.0);
+		out(" bbr:(bw:%sbps,mrtt:%g",
+		    sprint_bw(b1, bw * 8.0),
+		    (double)s->bbr_info->bbr_min_rtt / 1000.0);
 		if (s->bbr_info->bbr_pacing_gain)
-			printf(",pacing_gain:%g",
-			       (double)s->bbr_info->bbr_pacing_gain / 256.0);
+			out(",pacing_gain:%g",
+			    (double)s->bbr_info->bbr_pacing_gain / 256.0);
 		if (s->bbr_info->bbr_cwnd_gain)
-			printf(",cwnd_gain:%g",
-			       (double)s->bbr_info->bbr_cwnd_gain / 256.0);
-		printf(")");
+			out(",cwnd_gain:%g",
+			    (double)s->bbr_info->bbr_cwnd_gain / 256.0);
+		out(")");
 	}
 
 	if (s->send_bps)
-		printf(" send %sbps", sprint_bw(b1, s->send_bps));
+		out(" send %sbps", sprint_bw(b1, s->send_bps));
 	if (s->lastsnd)
-		printf(" lastsnd:%u", s->lastsnd);
+		out(" lastsnd:%u", s->lastsnd);
 	if (s->lastrcv)
-		printf(" lastrcv:%u", s->lastrcv);
+		out(" lastrcv:%u", s->lastrcv);
 	if (s->lastack)
-		printf(" lastack:%u", s->lastack);
+		out(" lastack:%u", s->lastack);
 
 	if (s->pacing_rate) {
-		printf(" pacing_rate %sbps", sprint_bw(b1, s->pacing_rate));
+		out(" pacing_rate %sbps", sprint_bw(b1, s->pacing_rate));
 		if (s->pacing_rate_max)
-				printf("/%sbps", sprint_bw(b1,
-							s->pacing_rate_max));
+			out("/%sbps", sprint_bw(b1, s->pacing_rate_max));
 	}
 
 	if (s->delivery_rate)
-		printf(" delivery_rate %sbps", sprint_bw(b1, s->delivery_rate));
+		out(" delivery_rate %sbps", sprint_bw(b1, s->delivery_rate));
 	if (s->app_limited)
-		printf(" app_limited");
+		out(" app_limited");
 
 	if (s->busy_time) {
-		printf(" busy:%llums", s->busy_time / 1000);
+		out(" busy:%llums", s->busy_time / 1000);
 		if (s->rwnd_limited)
-			printf(" rwnd_limited:%llums(%.1f%%)",
-			       s->rwnd_limited / 1000,
-			       100.0 * s->rwnd_limited / s->busy_time);
+			out(" rwnd_limited:%llums(%.1f%%)",
+			    s->rwnd_limited / 1000,
+			    100.0 * s->rwnd_limited / s->busy_time);
 		if (s->sndbuf_limited)
-			printf(" sndbuf_limited:%llums(%.1f%%)",
-			       s->sndbuf_limited / 1000,
-			       100.0 * s->sndbuf_limited / s->busy_time);
+			out(" sndbuf_limited:%llums(%.1f%%)",
+			    s->sndbuf_limited / 1000,
+			    100.0 * s->sndbuf_limited / s->busy_time);
 	}
 
 	if (s->unacked)
-		printf(" unacked:%u", s->unacked);
+		out(" unacked:%u", s->unacked);
 	if (s->retrans || s->retrans_total)
-		printf(" retrans:%u/%u", s->retrans, s->retrans_total);
+		out(" retrans:%u/%u", s->retrans, s->retrans_total);
 	if (s->lost)
-		printf(" lost:%u", s->lost);
+		out(" lost:%u", s->lost);
 	if (s->sacked && s->ss.state != SS_LISTEN)
-		printf(" sacked:%u", s->sacked);
+		out(" sacked:%u", s->sacked);
 	if (s->fackets)
-		printf(" fackets:%u", s->fackets);
+		out(" fackets:%u", s->fackets);
 	if (s->reordering != 3)
-		printf(" reordering:%d", s->reordering);
+		out(" reordering:%d", s->reordering);
 	if (s->rcv_rtt)
-		printf(" rcv_rtt:%g", s->rcv_rtt);
+		out(" rcv_rtt:%g", s->rcv_rtt);
 	if (s->rcv_space)
-		printf(" rcv_space:%d", s->rcv_space);
+		out(" rcv_space:%d", s->rcv_space);
 	if (s->rcv_ssthresh)
-		printf(" rcv_ssthresh:%u", s->rcv_ssthresh);
+		out(" rcv_ssthresh:%u", s->rcv_ssthresh);
 	if (s->not_sent)
-		printf(" notsent:%u", s->not_sent);
+		out(" notsent:%u", s->not_sent);
 	if (s->min_rtt)
-		printf(" minrtt:%g", s->min_rtt);
+		out(" minrtt:%g", s->min_rtt);
 }
 
 static void tcp_timer_print(struct tcpstat *s)
@@ -2081,18 +2090,18 @@ static void tcp_timer_print(struct tcpstat *s)
 	if (s->timer) {
 		if (s->timer > 4)
 			s->timer = 5;
-		printf(" timer:(%s,%s,%d)",
-				tmr_name[s->timer],
-				print_ms_timer(s->timeout),
-				s->retrans);
+		out(" timer:(%s,%s,%d)",
+			     tmr_name[s->timer],
+			     print_ms_timer(s->timeout),
+			     s->retrans);
 	}
 }
 
 static void sctp_timer_print(struct tcpstat *s)
 {
 	if (s->timer)
-		printf(" timer:(T3_RTX,%s,%d)",
-		       print_ms_timer(s->timeout), s->retrans);
+		out(" timer:(T3_RTX,%s,%d)",
+		    print_ms_timer(s->timeout), s->retrans);
 }
 
 static int tcp_show_line(char *line, const struct filter *f, int family)
@@ -2151,13 +2160,13 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 	if (show_details) {
 		sock_details_print(&s.ss);
 		if (opt[0])
-			printf(" opt:\"%s\"", opt);
+			out(" opt:\"%s\"", opt);
 	}
 
 	if (show_tcpinfo)
 		tcp_stats_print(&s);
 
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -2200,44 +2209,44 @@ static void print_skmeminfo(struct rtattr *tb[], int attrtype)
 			const struct inet_diag_meminfo *minfo =
 				RTA_DATA(tb[INET_DIAG_MEMINFO]);
 
-			printf(" mem:(r%u,w%u,f%u,t%u)",
-					minfo->idiag_rmem,
-					minfo->idiag_wmem,
-					minfo->idiag_fmem,
-					minfo->idiag_tmem);
+			out(" mem:(r%u,w%u,f%u,t%u)",
+				   minfo->idiag_rmem,
+				   minfo->idiag_wmem,
+				   minfo->idiag_fmem,
+				   minfo->idiag_tmem);
 		}
 		return;
 	}
 
 	skmeminfo = RTA_DATA(tb[attrtype]);
 
-	printf(" skmem:(r%u,rb%u,t%u,tb%u,f%u,w%u,o%u",
-	       skmeminfo[SK_MEMINFO_RMEM_ALLOC],
-	       skmeminfo[SK_MEMINFO_RCVBUF],
-	       skmeminfo[SK_MEMINFO_WMEM_ALLOC],
-	       skmeminfo[SK_MEMINFO_SNDBUF],
-	       skmeminfo[SK_MEMINFO_FWD_ALLOC],
-	       skmeminfo[SK_MEMINFO_WMEM_QUEUED],
-	       skmeminfo[SK_MEMINFO_OPTMEM]);
+	out(" skmem:(r%u,rb%u,t%u,tb%u,f%u,w%u,o%u",
+		     skmeminfo[SK_MEMINFO_RMEM_ALLOC],
+		     skmeminfo[SK_MEMINFO_RCVBUF],
+		     skmeminfo[SK_MEMINFO_WMEM_ALLOC],
+		     skmeminfo[SK_MEMINFO_SNDBUF],
+		     skmeminfo[SK_MEMINFO_FWD_ALLOC],
+		     skmeminfo[SK_MEMINFO_WMEM_QUEUED],
+		     skmeminfo[SK_MEMINFO_OPTMEM]);
 
 	if (RTA_PAYLOAD(tb[attrtype]) >=
 		(SK_MEMINFO_BACKLOG + 1) * sizeof(__u32))
-		printf(",bl%u", skmeminfo[SK_MEMINFO_BACKLOG]);
+		out(",bl%u", skmeminfo[SK_MEMINFO_BACKLOG]);
 
 	if (RTA_PAYLOAD(tb[attrtype]) >=
 		(SK_MEMINFO_DROPS + 1) * sizeof(__u32))
-		printf(",d%u", skmeminfo[SK_MEMINFO_DROPS]);
+		out(",d%u", skmeminfo[SK_MEMINFO_DROPS]);
 
-	printf(")");
+	out(")");
 }
 
 static void print_md5sig(struct tcp_diag_md5sig *sig)
 {
-	printf("%s/%d=",
-	       format_host(sig->tcpm_family,
-			   sig->tcpm_family == AF_INET6 ? 16 : 4,
-			   &sig->tcpm_addr),
-	       sig->tcpm_prefixlen);
+	out("%s/%d=",
+	    format_host(sig->tcpm_family,
+			sig->tcpm_family == AF_INET6 ? 16 : 4,
+			&sig->tcpm_addr),
+	    sig->tcpm_prefixlen);
 	print_escape_buf(sig->tcpm_key, sig->tcpm_keylen, " ,");
 }
 
@@ -2382,10 +2391,10 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 		struct tcp_diag_md5sig *sig = RTA_DATA(tb[INET_DIAG_MD5SIG]);
 		int len = RTA_PAYLOAD(tb[INET_DIAG_MD5SIG]);
 
-		printf(" md5keys:");
+		out(" md5keys:");
 		print_md5sig(sig++);
 		for (len -= sizeof(*sig); len > 0; len -= sizeof(*sig)) {
-			printf(",");
+			out(",");
 			print_md5sig(sig++);
 		}
 	}
@@ -2420,18 +2429,18 @@ static void sctp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r,
 		len = RTA_PAYLOAD(tb[INET_DIAG_LOCALS]);
 		sa = RTA_DATA(tb[INET_DIAG_LOCALS]);
 
-		printf("locals:%s", format_host_sa(sa));
+		out("locals:%s", format_host_sa(sa));
 		for (sa++, len -= sizeof(*sa); len > 0; sa++, len -= sizeof(*sa))
-			printf(",%s", format_host_sa(sa));
+			out(",%s", format_host_sa(sa));
 
 	}
 	if (tb[INET_DIAG_PEERS]) {
 		len = RTA_PAYLOAD(tb[INET_DIAG_PEERS]);
 		sa = RTA_DATA(tb[INET_DIAG_PEERS]);
 
-		printf(" peers:%s", format_host_sa(sa));
+		out(" peers:%s", format_host_sa(sa));
 		for (sa++, len -= sizeof(*sa); len > 0; sa++, len -= sizeof(*sa))
-			printf(",%s", format_host_sa(sa));
+			out(",%s", format_host_sa(sa));
 	}
 	if (tb[INET_DIAG_INFO]) {
 		struct sctp_info *info;
@@ -2518,18 +2527,19 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 	if (show_details) {
 		sock_details_print(s);
 		if (s->local.family == AF_INET6 && tb[INET_DIAG_SKV6ONLY])
-			printf(" v6only:%u", v6only);
+			out(" v6only:%u", v6only);
 
 		if (tb[INET_DIAG_SHUTDOWN]) {
 			unsigned char mask;
 
 			mask = rta_getattr_u8(tb[INET_DIAG_SHUTDOWN]);
-			printf(" %c-%c", mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
+			out(" %c-%c",
+			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
 
 	if (show_mem || (show_tcpinfo && s->type != IPPROTO_UDP)) {
-		printf("\n\t");
+		out("\n\t");
 		if (s->type == IPPROTO_SCTP)
 			sctp_show_info(nlh, r, tb);
 		else
@@ -2537,7 +2547,7 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 	}
 	sctp_ino = s->ino;
 
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -2993,9 +3003,9 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 	inet_stats_print(&s, false);
 
 	if (show_details && opt[0])
-		printf(" opt:\"%s\"", opt);
+		out(" opt:\"%s\"", opt);
 
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -3170,10 +3180,11 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 			unsigned char mask;
 
 			mask = rta_getattr_u8(tb[UNIX_DIAG_SHUTDOWN]);
-			printf(" %c-%c", mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
+			out(" %c-%c",
+			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
-	printf("\n");
+	out("\n");
 
 	return 0;
 }
@@ -3330,7 +3341,7 @@ static int unix_show(struct filter *f)
 		if (++cnt > MAX_UNIX_REMEMBER) {
 			while (list) {
 				unix_stats_print(list, f);
-				printf("\n");
+				out("\n");
 
 				unix_list_drop_first(&list);
 			}
@@ -3340,7 +3351,7 @@ static int unix_show(struct filter *f)
 	fclose(fp);
 	while (list) {
 		unix_stats_print(list, f);
-		printf("\n");
+		out("\n");
 
 		unix_list_drop_first(&list);
 	}
@@ -3386,12 +3397,12 @@ static int packet_stats_print(struct sockstat *s, const struct filter *f)
 
 static void packet_show_ring(struct packet_diag_ring *ring)
 {
-	printf("blk_size:%d", ring->pdr_block_size);
-	printf(",blk_nr:%d", ring->pdr_block_nr);
-	printf(",frm_size:%d", ring->pdr_frame_size);
-	printf(",frm_nr:%d", ring->pdr_frame_nr);
-	printf(",tmo:%d", ring->pdr_retire_tmo);
-	printf(",features:0x%x", ring->pdr_features);
+	out("blk_size:%d", ring->pdr_block_size);
+	out(",blk_nr:%d", ring->pdr_block_nr);
+	out(",frm_size:%d", ring->pdr_frame_size);
+	out(",frm_nr:%d", ring->pdr_frame_nr);
+	out(",tmo:%d", ring->pdr_retire_tmo);
+	out(",features:0x%x", ring->pdr_features);
 }
 
 static int packet_show_sock(const struct sockaddr_nl *addr,
@@ -3449,56 +3460,56 @@ static int packet_show_sock(const struct sockaddr_nl *addr,
 
 	if (show_details) {
 		if (pinfo) {
-			printf("\n\tver:%d", pinfo->pdi_version);
-			printf(" cpy_thresh:%d", pinfo->pdi_copy_thresh);
-			printf(" flags( ");
+			out("\n\tver:%d", pinfo->pdi_version);
+			out(" cpy_thresh:%d", pinfo->pdi_copy_thresh);
+			out(" flags( ");
 			if (pinfo->pdi_flags & PDI_RUNNING)
-				printf("running");
+				out("running");
 			if (pinfo->pdi_flags & PDI_AUXDATA)
-				printf(" auxdata");
+				out(" auxdata");
 			if (pinfo->pdi_flags & PDI_ORIGDEV)
-				printf(" origdev");
+				out(" origdev");
 			if (pinfo->pdi_flags & PDI_VNETHDR)
-				printf(" vnethdr");
+				out(" vnethdr");
 			if (pinfo->pdi_flags & PDI_LOSS)
-				printf(" loss");
+				out(" loss");
 			if (!pinfo->pdi_flags)
-				printf("0");
-			printf(" )");
+				out("0");
+			out(" )");
 		}
 		if (ring_rx) {
-			printf("\n\tring_rx(");
+			out("\n\tring_rx(");
 			packet_show_ring(ring_rx);
-			printf(")");
+			out(")");
 		}
 		if (ring_tx) {
-			printf("\n\tring_tx(");
+			out("\n\tring_tx(");
 			packet_show_ring(ring_tx);
-			printf(")");
+			out(")");
 		}
 		if (has_fanout) {
 			uint16_t type = (fanout >> 16) & 0xffff;
 
-			printf("\n\tfanout(");
-			printf("id:%d,", fanout & 0xffff);
-			printf("type:");
+			out("\n\tfanout(");
+			out("id:%d,", fanout & 0xffff);
+			out("type:");
 
 			if (type == 0)
-				printf("hash");
+				out("hash");
 			else if (type == 1)
-				printf("lb");
+				out("lb");
 			else if (type == 2)
-				printf("cpu");
+				out("cpu");
 			else if (type == 3)
-				printf("roll");
+				out("roll");
 			else if (type == 4)
-				printf("random");
+				out("random");
 			else if (type == 5)
-				printf("qm");
+				out("qm");
 			else
-				printf("0x%x", type);
+				out("0x%x", type);
 
-			printf(")");
+			out(")");
 		}
 	}
 
@@ -3508,15 +3519,15 @@ static int packet_show_sock(const struct sockaddr_nl *addr,
 		int num = RTA_PAYLOAD(tb[PACKET_DIAG_FILTER]) /
 			  sizeof(struct sock_filter);
 
-		printf("\n\tbpf filter (%d): ", num);
+		out("\n\tbpf filter (%d): ", num);
 		while (num) {
-			printf(" 0x%02x %u %u %u,",
-			      fil->code, fil->jt, fil->jf, fil->k);
+			out(" 0x%02x %u %u %u,",
+			    fil->code, fil->jt, fil->jf, fil->k);
 			num--;
 			fil++;
 		}
 	}
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -3559,7 +3570,7 @@ static int packet_show_line(char *buf, const struct filter *f, int fam)
 	if (packet_stats_print(&stat, f))
 		return 0;
 
-	printf("\n");
+	out("\n");
 	return 0;
 }
 
@@ -3672,14 +3683,14 @@ static int netlink_show_one(struct filter *f,
 		else if (pid > 0)
 			getpidcon(pid, &pid_context);
 
-		printf(" proc_ctx=%s", pid_context ? : "unavailable");
+		out(" proc_ctx=%s", pid_context ? : "unavailable");
 		free(pid_context);
 	}
 
 	if (show_details) {
-		printf(" sk=%llx cb=%llx groups=0x%08x", sk, cb, groups);
+		out(" sk=%llx cb=%llx groups=0x%08x", sk, cb, groups);
 	}
-	printf("\n");
+	out("\n");
 
 	return 0;
 }
@@ -3715,9 +3726,9 @@ static int netlink_show_sock(const struct sockaddr_nl *addr,
 	}
 
 	if (show_mem) {
-		printf("\t");
+		out("\t");
 		print_skmeminfo(tb, NETLINK_DIAG_MEMINFO);
-		printf("\n");
+		out("\n");
 	}
 
 	return 0;
@@ -3808,7 +3819,7 @@ static void vsock_stats_print(struct sockstat *s, struct filter *f)
 
 	proc_ctx_print(s);
 
-	printf("\n");
+	out("\n");
 }
 
 static int vsock_show_sock(const struct sockaddr_nl *addr,
@@ -4573,10 +4584,10 @@ int main(int argc, char *argv[])
 
 	if (show_header) {
 		if (netid_width)
-			printf("%-*s ", netid_width, "Netid");
+			out("%-*s ", netid_width, "Netid");
 		if (state_width)
-			printf("%-*s ", state_width, "State");
-		printf("%-6s %-6s %s", "Recv-Q", "Send-Q", odd_width_pad);
+			out("%-*s ", state_width, "State");
+		out("%-6s %-6s %s", "Recv-Q", "Send-Q", odd_width_pad);
 	}
 
 	/* Make enough space for the local/remote port field */
@@ -4584,9 +4595,9 @@ int main(int argc, char *argv[])
 	serv_width += 13;
 
 	if (show_header) {
-		printf("%*s:%-*s %*s:%-*s\n",
-		       addr_width, "Local Address", serv_width, "Port",
-		       addr_width, "Peer Address", serv_width, "Port");
+		out("%*s:%-*s %*s:%-*s\n",
+		    addr_width, "Local Address", serv_width, "Port",
+		    addr_width, "Peer Address", serv_width, "Port");
 	}
 
 	fflush(stdout);
-- 
2.9.4

^ permalink raw reply related

* [PATCH iproute2 net-next v2 2/4] ss: Introduce columns lightweight abstraction
From: Stefano Brivio @ 2017-12-12  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca
In-Reply-To: <cover.1513039237.git.sbrivio@redhat.com>

Instead of embedding spacing directly while printing contents,
logically declare columns and functions to buffer their content,
to print left and right spacing around fields, to flush them to
screen, and to print headers.

This makes it a bit easier to handle layout changes and prepares
for full output buffering, needed for optimal spacing in field
output layout.

Columns are currently set up to retain exactly the same output
as before. This needs some slight adjustments of the values
previously calculated in main(), as the width value introduced
here already includes the width of left delimiters and spacing
is not explicitly printed anymore whenever a field is printed.
These calculations will go away altogether once automatic width
calculation is implemented.

We can also remove explicit printing of newlines after the final
content for a given line is printed, flushing the last field on
a line will cause field_flush() to print newlines where
appropriate.

No changes in output expected here.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: rebase after conflict with 00ac78d39c29 ("ss: print tcpi_rcv_ssthresh")

 misc/ss.c | 291 ++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 198 insertions(+), 93 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index a7d3b89e1478..42310ba4120d 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -103,11 +103,48 @@ int show_header = 1;
 int follow_events;
 int sctp_ino;
 
-int netid_width;
-int state_width;
-int addr_width;
-int serv_width;
-char *odd_width_pad = "";
+enum col_id {
+	COL_NETID,
+	COL_STATE,
+	COL_RECVQ,
+	COL_SENDQ,
+	COL_ADDR,
+	COL_SERV,
+	COL_RADDR,
+	COL_RSERV,
+	COL_EXT,
+	COL_MAX
+};
+
+enum col_align {
+	ALIGN_LEFT,
+	ALIGN_CENTER,
+	ALIGN_RIGHT
+};
+
+struct column {
+	const enum col_align align;
+	const char *header;
+	const char *ldelim;
+	int width;	/* Including delimiter. -1: fit to content, 0: hide */
+	int stored;	/* Characters buffered */
+	int printed;	/* Characters printed so far */
+};
+
+static struct column columns[] = {
+	{ ALIGN_LEFT,	"Netid",		"",	0,	0,	0 },
+	{ ALIGN_LEFT,	"State",		" ",	0,	0,	0 },
+	{ ALIGN_LEFT,	"Recv-Q",		" ",	7,	0,	0 },
+	{ ALIGN_LEFT,	"Send-Q",		" ",	7,	0,	0 },
+	{ ALIGN_RIGHT,	"Local Address:",	" ",	0,	0,	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0,	0,	0 },
+	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0,	0,	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0,	0,	0 },
+	{ ALIGN_LEFT,	"",			"",	-1,	0,	0 },
+};
+
+static struct column *current_field = columns;
+static char field_buf[BUFSIZ];
 
 static const char *TCP_PROTO = "tcp";
 static const char *SCTP_PROTO = "sctp";
@@ -826,13 +863,113 @@ static const char *vsock_netid_name(int type)
 
 static void out(const char *fmt, ...)
 {
+	struct column *f = current_field;
 	va_list args;
 
 	va_start(args, fmt);
-	vfprintf(stdout, fmt, args);
+	f->stored += vsnprintf(field_buf + f->stored, BUFSIZ - f->stored,
+			       fmt, args);
 	va_end(args);
 }
 
+static int print_left_spacing(struct column *f)
+{
+	int s;
+
+	if (f->width < 0 || f->align == ALIGN_LEFT)
+		return 0;
+
+	s = f->width - f->stored - f->printed;
+	if (f->align == ALIGN_CENTER)
+		/* If count of total spacing is odd, shift right by one */
+		s = (s + 1) / 2;
+
+	if (s > 0)
+		return printf("%*c", s, ' ');
+
+	return 0;
+}
+
+static void print_right_spacing(struct column *f)
+{
+	int s;
+
+	if (f->width < 0 || f->align == ALIGN_RIGHT)
+		return;
+
+	s = f->width - f->printed;
+	if (f->align == ALIGN_CENTER)
+		s /= 2;
+
+	if (s > 0)
+		printf("%*c", s, ' ');
+}
+
+static int field_needs_delimiter(struct column *f)
+{
+	if (!f->stored)
+		return 0;
+
+	/* Was another field already printed on this line? */
+	for (f--; f >= columns; f--)
+		if (f->width)
+			return 1;
+
+	return 0;
+}
+
+/* Flush given field to screen together with delimiter and spacing */
+static void field_flush(struct column *f)
+{
+	if (!f->width)
+		return;
+
+	if (field_needs_delimiter(f))
+		f->printed = printf("%s", f->ldelim);
+
+	f->printed += print_left_spacing(f);
+	f->printed += printf("%s", field_buf);
+	print_right_spacing(f);
+
+	*field_buf = 0;
+	f->printed = 0;
+	f->stored = 0;
+}
+
+static int field_is_last(struct column *f)
+{
+	return f - columns == COL_MAX - 1;
+}
+
+static void field_next(void)
+{
+	field_flush(current_field);
+
+	if (field_is_last(current_field)) {
+		printf("\n");
+		current_field = columns;
+	} else {
+		current_field++;
+	}
+}
+
+/* Walk through fields and flush them until we reach the desired one */
+static void field_set(enum col_id id)
+{
+	while (id != current_field - columns)
+		field_next();
+}
+
+/* Print header for all non-empty columns */
+static void print_header(void)
+{
+	while (!field_is_last(current_field)) {
+		if (current_field->width)
+			out(current_field->header);
+		field_next();
+	}
+}
+
 static void sock_state_print(struct sockstat *s)
 {
 	const char *sock_name;
@@ -872,18 +1009,21 @@ static void sock_state_print(struct sockstat *s)
 		sock_name = "unknown";
 	}
 
-	if (netid_width)
-		out("%-*s ", netid_width,
-		    is_sctp_assoc(s, sock_name) ? "" : sock_name);
-	if (state_width) {
-		if (is_sctp_assoc(s, sock_name))
-			out("`- %-*s ", state_width - 3,
-			    sctp_sstate_name[s->state]);
-		else
-			out("%-*s ", state_width, sstate_name[s->state]);
+	if (is_sctp_assoc(s, sock_name)) {
+		field_set(COL_STATE);		/* Empty Netid field */
+		out("`- %s", sctp_sstate_name[s->state]);
+	} else {
+		field_set(COL_NETID);
+		out("%s", sock_name);
+		field_set(COL_STATE);
+		out("%s", sstate_name[s->state]);
 	}
 
-	out("%-6d %-6d %s", s->rq, s->wq, odd_width_pad);
+	field_set(COL_RECVQ);
+	out("%-6d", s->rq);
+	field_set(COL_SENDQ);
+	out("%-6d", s->wq);
+	field_set(COL_ADDR);
 }
 
 static void sock_details_print(struct sockstat *s)
@@ -898,21 +1038,17 @@ static void sock_details_print(struct sockstat *s)
 		out(" fwmark:0x%x", s->mark);
 }
 
-static void sock_addr_print_width(int addr_len, const char *addr, char *delim,
-		int port_len, const char *port, const char *ifname)
-{
-	if (ifname) {
-		out("%*s%%%s%s%-*s ", addr_len, addr, ifname, delim,
-		    port_len, port);
-	} else {
-		out("%*s%s%-*s ", addr_len, addr, delim, port_len, port);
-	}
-}
-
 static void sock_addr_print(const char *addr, char *delim, const char *port,
 		const char *ifname)
 {
-	sock_addr_print_width(addr_width, addr, delim, serv_width, port, ifname);
+	if (ifname)
+		out("%s" "%%" "%s%s", addr, ifname, delim);
+	else
+		out("%s%s", addr, delim);
+
+	field_next();
+	out("%s", port);
+	field_next();
 }
 
 static const char *print_ms_timer(unsigned int timeout)
@@ -1093,7 +1229,6 @@ static void inet_addr_print(const inet_prefix *a, int port,
 {
 	char buf[1024];
 	const char *ap = buf;
-	int est_len = addr_width;
 	const char *ifname = NULL;
 
 	if (a->family == AF_INET) {
@@ -1112,24 +1247,13 @@ static void inet_addr_print(const inet_prefix *a, int port,
 					 "[%s]", ap);
 				ap = buf;
 			}
-
-			est_len = strlen(ap);
-			if (est_len <= addr_width)
-				est_len = addr_width;
-			else
-				est_len = addr_width + ((est_len-addr_width+3)/4)*4;
 		}
 	}
 
-	if (ifindex) {
-		ifname   = ll_index_to_name(ifindex);
-		est_len -= strlen(ifname) + 1;  /* +1 for percent char */
-		if (est_len < 0)
-			est_len = 0;
-	}
+	if (ifindex)
+		ifname = ll_index_to_name(ifindex);
 
-	sock_addr_print_width(est_len, ap, ":", serv_width, resolve_service(port),
-			ifname);
+	sock_addr_print(ap, ":", resolve_service(port), ifname);
 }
 
 struct aafilter {
@@ -2166,7 +2290,6 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 	if (show_tcpinfo)
 		tcp_stats_print(&s);
 
-	out("\n");
 	return 0;
 }
 
@@ -2547,7 +2670,6 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 	}
 	sctp_ino = s->ino;
 
-	out("\n");
 	return 0;
 }
 
@@ -3005,7 +3127,6 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 	if (show_details && opt[0])
 		out(" opt:\"%s\"", opt);
 
-	out("\n");
 	return 0;
 }
 
@@ -3184,7 +3305,6 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 			    mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
-	out("\n");
 
 	return 0;
 }
@@ -3341,8 +3461,6 @@ static int unix_show(struct filter *f)
 		if (++cnt > MAX_UNIX_REMEMBER) {
 			while (list) {
 				unix_stats_print(list, f);
-				out("\n");
-
 				unix_list_drop_first(&list);
 			}
 			cnt = 0;
@@ -3351,8 +3469,6 @@ static int unix_show(struct filter *f)
 	fclose(fp);
 	while (list) {
 		unix_stats_print(list, f);
-		out("\n");
-
 		unix_list_drop_first(&list);
 	}
 
@@ -3527,7 +3643,6 @@ static int packet_show_sock(const struct sockaddr_nl *addr,
 			fil++;
 		}
 	}
-	out("\n");
 	return 0;
 }
 
@@ -3570,7 +3685,6 @@ static int packet_show_line(char *buf, const struct filter *f, int fam)
 	if (packet_stats_print(&stat, f))
 		return 0;
 
-	out("\n");
 	return 0;
 }
 
@@ -3690,7 +3804,6 @@ static int netlink_show_one(struct filter *f,
 	if (show_details) {
 		out(" sk=%llx cb=%llx groups=0x%08x", sk, cb, groups);
 	}
-	out("\n");
 
 	return 0;
 }
@@ -3728,7 +3841,6 @@ static int netlink_show_sock(const struct sockaddr_nl *addr,
 	if (show_mem) {
 		out("\t");
 		print_skmeminfo(tb, NETLINK_DIAG_MEMINFO);
-		out("\n");
 	}
 
 	return 0;
@@ -3818,8 +3930,6 @@ static void vsock_stats_print(struct sockstat *s, struct filter *f)
 	vsock_addr_print(&s->remote, s->rport);
 
 	proc_ctx_print(s);
-
-	out("\n");
 }
 
 static int vsock_show_sock(const struct sockaddr_nl *addr,
@@ -4539,13 +4649,17 @@ int main(int argc, char *argv[])
 	if (ssfilter_parse(&current_filter.f, argc, argv, filter_fp))
 		usage();
 
-	netid_width = 0;
 	if (current_filter.dbs&(current_filter.dbs-1))
-		netid_width = 5;
+		columns[COL_NETID].width = 6;
 
-	state_width = 0;
 	if (current_filter.states&(current_filter.states-1))
-		state_width = 10;
+		columns[COL_STATE].width = 10;
+
+	/* If Netid or State are hidden, no delimiter before next column */
+	if (!columns[COL_NETID].width)
+		columns[COL_STATE].width--;
+	else if (!columns[COL_STATE].width)
+		columns[COL_RECVQ].width--;
 
 	if (isatty(STDOUT_FILENO)) {
 		struct winsize w;
@@ -4556,49 +4670,38 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	addrp_width = screen_width;
-	if (netid_width)
-		addrp_width -= netid_width + 1;
-	if (state_width)
-		addrp_width -= state_width + 1;
-	addrp_width -= 14;
+	addrp_width = screen_width -
+		      columns[COL_NETID].width -
+		      columns[COL_STATE].width -
+		      columns[COL_RECVQ].width -
+		      columns[COL_SENDQ].width;
 
 	if (addrp_width&1) {
-		if (netid_width)
-			netid_width++;
-		else if (state_width)
-			state_width++;
+		if (columns[COL_NETID].width)
+			columns[COL_NETID].width++;
+		else if (columns[COL_STATE].width)
+			columns[COL_STATE].width++;
 		else
-			odd_width_pad = " ";
+			columns[COL_SENDQ].width++;
 	}
 
 	addrp_width /= 2;
-	addrp_width--;
 
-	serv_width = resolve_services ? 7 : 5;
+	columns[COL_SERV].width = resolve_services ? 8 : 6;
+	if (addrp_width < 15 + columns[COL_SERV].width)
+		addrp_width = 15 + columns[COL_SERV].width;
 
-	if (addrp_width < 15+serv_width+1)
-		addrp_width = 15+serv_width+1;
-
-	addr_width = addrp_width - serv_width - 1;
-
-	if (show_header) {
-		if (netid_width)
-			out("%-*s ", netid_width, "Netid");
-		if (state_width)
-			out("%-*s ", state_width, "State");
-		out("%-6s %-6s %s", "Recv-Q", "Send-Q", odd_width_pad);
-	}
+	columns[COL_ADDR].width = addrp_width - columns[COL_SERV].width;
 
 	/* Make enough space for the local/remote port field */
-	addr_width -= 13;
-	serv_width += 13;
+	columns[COL_ADDR].width -= 13;
+	columns[COL_SERV].width += 13;
 
-	if (show_header) {
-		out("%*s:%-*s %*s:%-*s\n",
-		    addr_width, "Local Address", serv_width, "Port",
-		    addr_width, "Peer Address", serv_width, "Port");
-	}
+	columns[COL_RADDR].width = columns[COL_ADDR].width;
+	columns[COL_RSERV].width = columns[COL_SERV].width;
+
+	if (show_header)
+		print_header();
 
 	fflush(stdout);
 
@@ -4627,5 +4730,7 @@ int main(int argc, char *argv[])
 	if (show_users || show_proc_ctx || show_sock_ctx)
 		user_ent_destroy();
 
+	field_next();
+
 	return 0;
 }
-- 
2.9.4

^ permalink raw reply related

* [PATCH iproute2 net-next v2 3/4] ss: Buffer raw fields first, then render them as a table
From: Stefano Brivio @ 2017-12-12  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca
In-Reply-To: <cover.1513039237.git.sbrivio@redhat.com>

This allows us to measure the maximum field length for each
column before printing fields and will permit us to apply
optimal field spacing and distribution. Structure of the output
buffer with chunked allocation is described in comments.

Output is still unchanged, original spacing is used.

Running over one million sockets with -tul options by simply
modifying main() to loop 50,000 times over the *_show()
functions, buffering the whole output and rendering it at the
end, with 10 UDP sockets, 10 TCP sockets, while throwing
output away, doesn't show significant changes in execution time
on my laptop with an Intel i7-6600U CPU:

- before this patch:
$ time ./ss -tul > /dev/null
real	0m29.899s
user	0m2.017s
sys	0m27.801s

- after this patch:
$ time ./ss -tul > /dev/null
real	0m29.827s
user	0m1.942s
sys	0m27.812s

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: rebase after conflict with 00ac78d39c29 ("ss: print tcpi_rcv_ssthresh")

 misc/ss.c | 271 +++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 225 insertions(+), 46 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 42310ba4120d..166267974c36 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -47,6 +47,8 @@
 #include <linux/vm_sockets_diag.h>
 
 #define MAGIC_SEQ 123456
+#define BUF_CHUNK (1024 * 1024)
+#define LEN_ALIGN(x) (((x) + 1) & ~1)
 
 #define DIAG_REQUEST(_req, _r)						    \
 	struct {							    \
@@ -127,24 +129,45 @@ struct column {
 	const char *header;
 	const char *ldelim;
 	int width;	/* Including delimiter. -1: fit to content, 0: hide */
-	int stored;	/* Characters buffered */
-	int printed;	/* Characters printed so far */
 };
 
 static struct column columns[] = {
-	{ ALIGN_LEFT,	"Netid",		"",	0,	0,	0 },
-	{ ALIGN_LEFT,	"State",		" ",	0,	0,	0 },
-	{ ALIGN_LEFT,	"Recv-Q",		" ",	7,	0,	0 },
-	{ ALIGN_LEFT,	"Send-Q",		" ",	7,	0,	0 },
-	{ ALIGN_RIGHT,	"Local Address:",	" ",	0,	0,	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0,	0,	0 },
-	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0,	0,	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0,	0,	0 },
-	{ ALIGN_LEFT,	"",			"",	-1,	0,	0 },
+	{ ALIGN_LEFT,	"Netid",		"",	0 },
+	{ ALIGN_LEFT,	"State",		" ",	0 },
+	{ ALIGN_LEFT,	"Recv-Q",		" ",	7 },
+	{ ALIGN_LEFT,	"Send-Q",		" ",	7 },
+	{ ALIGN_RIGHT,	"Local Address:",	" ",	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0 },
+	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0 },
+	{ ALIGN_LEFT,	"Port",			"",	0 },
+	{ ALIGN_LEFT,	"",			"",	-1 },
 };
 
 static struct column *current_field = columns;
-static char field_buf[BUFSIZ];
+
+/* Output buffer: chained chunks of BUF_CHUNK bytes. Each field is written to
+ * the buffer as a variable size token. A token consists of a 16 bits length
+ * field, followed by a string which is not NULL-terminated.
+ *
+ * A new chunk is allocated and linked when the current chunk doesn't have
+ * enough room to store the current token as a whole.
+ */
+struct buf_chunk {
+	struct buf_chunk *next;	/* Next chained chunk */
+	char *end;		/* Current end of content */
+	char data[0];
+};
+
+struct buf_token {
+	uint16_t len;		/* Data length, excluding length descriptor */
+	char data[0];
+};
+
+static struct {
+	struct buf_token *cur;	/* Position of current token in chunk */
+	struct buf_chunk *head;	/* First chunk */
+	struct buf_chunk *tail;	/* Current chunk */
+} buffer;
 
 static const char *TCP_PROTO = "tcp";
 static const char *SCTP_PROTO = "sctp";
@@ -861,25 +884,109 @@ static const char *vsock_netid_name(int type)
 	}
 }
 
+/* Allocate and initialize a new buffer chunk */
+static struct buf_chunk *buf_chunk_new(void)
+{
+	struct buf_chunk *new = malloc(BUF_CHUNK);
+
+	if (!new)
+		abort();
+
+	new->next = NULL;
+
+	/* This is also the last block */
+	buffer.tail = new;
+
+	/* Next token will be stored at the beginning of chunk data area, and
+	 * its initial length is zero.
+	 */
+	buffer.cur = (struct buf_token *)new->data;
+	buffer.cur->len = 0;
+
+	new->end = buffer.cur->data;
+
+	return new;
+}
+
+/* Return available tail room in given chunk */
+static int buf_chunk_avail(struct buf_chunk *chunk)
+{
+	return BUF_CHUNK - offsetof(struct buf_chunk, data) -
+	       (chunk->end - chunk->data);
+}
+
+/* Update end pointer and token length, link new chunk if we hit the end of the
+ * current one. Return -EAGAIN if we got a new chunk, caller has to print again.
+ */
+static int buf_update(int len)
+{
+	struct buf_chunk *chunk = buffer.tail;
+	struct buf_token *t = buffer.cur;
+
+	/* Claim success if new content fits in the current chunk, and anyway
+	 * if this is the first token in the chunk: in the latter case,
+	 * allocating a new chunk won't help, so we'll just cut the output.
+	 */
+	if ((len < buf_chunk_avail(chunk) && len != -1 /* glibc < 2.0.6 */) ||
+	    t == (struct buf_token *)chunk->data) {
+		len = min(len, buf_chunk_avail(chunk));
+
+		/* Total field length can't exceed 2^16 bytes, cut as needed */
+		len = min(len, USHRT_MAX - t->len);
+
+		chunk->end += len;
+		t->len += len;
+		return 0;
+	}
+
+	/* Content truncated, time to allocate more */
+	chunk->next = buf_chunk_new();
+
+	/* Copy current token over to new chunk, including length descriptor */
+	memcpy(chunk->next->data, t, sizeof(t->len) + t->len);
+	chunk->next->end += t->len;
+
+	/* Discard partially written field in old chunk */
+	chunk->end -= t->len + sizeof(t->len);
+
+	return -EAGAIN;
+}
+
+/* Append content to buffer as part of the current field */
 static void out(const char *fmt, ...)
 {
 	struct column *f = current_field;
 	va_list args;
+	char *pos;
+	int len;
+
+	if (!f->width)
+		return;
+
+	if (!buffer.head)
+		buffer.head = buf_chunk_new();
+
+again:	/* Append to buffer: if we have a new chunk, print again */
 
+	pos = buffer.cur->data + buffer.cur->len;
 	va_start(args, fmt);
-	f->stored += vsnprintf(field_buf + f->stored, BUFSIZ - f->stored,
-			       fmt, args);
+
+	/* Limit to tail room. If we hit the limit, buf_update() will tell us */
+	len = vsnprintf(pos, buf_chunk_avail(buffer.tail), fmt, args);
 	va_end(args);
+
+	if (buf_update(len))
+		goto again;
 }
 
-static int print_left_spacing(struct column *f)
+static int print_left_spacing(struct column *f, int stored, int printed)
 {
 	int s;
 
 	if (f->width < 0 || f->align == ALIGN_LEFT)
 		return 0;
 
-	s = f->width - f->stored - f->printed;
+	s = f->width - stored - printed;
 	if (f->align == ALIGN_CENTER)
 		/* If count of total spacing is odd, shift right by one */
 		s = (s + 1) / 2;
@@ -890,14 +997,14 @@ static int print_left_spacing(struct column *f)
 	return 0;
 }
 
-static void print_right_spacing(struct column *f)
+static void print_right_spacing(struct column *f, int printed)
 {
 	int s;
 
 	if (f->width < 0 || f->align == ALIGN_RIGHT)
 		return;
 
-	s = f->width - f->printed;
+	s = f->width - printed;
 	if (f->align == ALIGN_CENTER)
 		s /= 2;
 
@@ -905,35 +1012,29 @@ static void print_right_spacing(struct column *f)
 		printf("%*c", s, ' ');
 }
 
-static int field_needs_delimiter(struct column *f)
-{
-	if (!f->stored)
-		return 0;
-
-	/* Was another field already printed on this line? */
-	for (f--; f >= columns; f--)
-		if (f->width)
-			return 1;
-
-	return 0;
-}
-
-/* Flush given field to screen together with delimiter and spacing */
+/* Done with field: update buffer pointer, start new token after current one */
 static void field_flush(struct column *f)
 {
+	struct buf_chunk *chunk = buffer.tail;
+	unsigned int pad = buffer.cur->len % 2;
+
 	if (!f->width)
 		return;
 
-	if (field_needs_delimiter(f))
-		f->printed = printf("%s", f->ldelim);
-
-	f->printed += print_left_spacing(f);
-	f->printed += printf("%s", field_buf);
-	print_right_spacing(f);
+	/* We need a new chunk if we can't store the next length descriptor.
+	 * Mind the gap between end of previous token and next aligned position
+	 * for length descriptor.
+	 */
+	if (buf_chunk_avail(chunk) - pad < sizeof(buffer.cur->len)) {
+		chunk->end += pad;
+		chunk->next = buf_chunk_new();
+		return;
+	}
 
-	*field_buf = 0;
-	f->printed = 0;
-	f->stored = 0;
+	buffer.cur = (struct buf_token *)(buffer.cur->data +
+					  LEN_ALIGN(buffer.cur->len));
+	buffer.cur->len = 0;
+	buffer.tail->end = buffer.cur->data;
 }
 
 static int field_is_last(struct column *f)
@@ -945,12 +1046,10 @@ static void field_next(void)
 {
 	field_flush(current_field);
 
-	if (field_is_last(current_field)) {
-		printf("\n");
+	if (field_is_last(current_field))
 		current_field = columns;
-	} else {
+	else
 		current_field++;
-	}
 }
 
 /* Walk through fields and flush them until we reach the desired one */
@@ -970,6 +1069,86 @@ static void print_header(void)
 	}
 }
 
+/* Get the next available token in the buffer starting from the current token */
+static struct buf_token *buf_token_next(struct buf_token *cur)
+{
+	struct buf_chunk *chunk = buffer.tail;
+
+	/* If we reached the end of chunk contents, get token from next chunk */
+	if (cur->data + LEN_ALIGN(cur->len) == chunk->end) {
+		buffer.tail = chunk = chunk->next;
+		return chunk ? (struct buf_token *)chunk->data : NULL;
+	}
+
+	return (struct buf_token *)(cur->data + LEN_ALIGN(cur->len));
+}
+
+/* Free up all allocated buffer chunks */
+static void buf_free_all(void)
+{
+	struct buf_chunk *tmp;
+
+	for (buffer.tail = buffer.head; buffer.tail; ) {
+		tmp = buffer.tail;
+		buffer.tail = buffer.tail->next;
+		free(tmp);
+	}
+	buffer.head = NULL;
+}
+
+/* Render buffered output with spacing and delimiters, then free up buffers */
+static void render(void)
+{
+	struct buf_token *token = (struct buf_token *)buffer.head->data;
+	int printed, line_started = 0, need_newline = 0;
+	struct column *f;
+
+	/* Ensure end alignment of last token, it wasn't necessarily flushed */
+	buffer.tail->end += buffer.cur->len % 2;
+
+	/* Rewind and replay */
+	buffer.tail = buffer.head;
+
+	f = columns;
+	while (!f->width)
+		f++;
+
+	while (token) {
+		/* Print left delimiter only if we already started a line */
+		if (line_started++)
+			printed = printf("%s", current_field->ldelim);
+		else
+			printed = 0;
+
+		/* Print field content from token data with spacing */
+		printed += print_left_spacing(f, token->len, printed);
+		printed += fwrite(token->data, 1, token->len, stdout);
+		print_right_spacing(f, printed);
+
+		/* Variable field size or overflow, won't align to screen */
+		if (printed > f->width)
+			need_newline = 1;
+
+		/* Go to next non-empty field, deal with end-of-line */
+		do {
+			if (field_is_last(f)) {
+				if (need_newline) {
+					printf("\n");
+					need_newline = 0;
+				}
+				f = columns;
+				line_started = 0;
+			} else {
+				f++;
+			}
+		} while (!f->width);
+
+		token = buf_token_next(token);
+	}
+
+	buf_free_all();
+}
+
 static void sock_state_print(struct sockstat *s)
 {
 	const char *sock_name;
@@ -4730,7 +4909,7 @@ int main(int argc, char *argv[])
 	if (show_users || show_proc_ctx || show_sock_ctx)
 		user_ent_destroy();
 
-	field_next();
+	render();
 
 	return 0;
 }
-- 
2.9.4

^ permalink raw reply related

* [PATCH iproute2 net-next v2 4/4] ss: Implement automatic column width calculation
From: Stefano Brivio @ 2017-12-12  0:46 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Sabrina Dubroca
In-Reply-To: <cover.1513039237.git.sbrivio@redhat.com>

Group fitting fields into lines and space them equally using the
remaining screen width for each line. If columns don't fit on
one line, break them into the least possible amount of lines and
keep them aligned across lines.

This is done by:
 - recording the length of the longest item in each column during
   formatting and buffering (which was added in the previous patch)
 - fitting as many fields as possible on each line of output
 - distributing the remaining padding space equally between the
   columns

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: rebase after conflict with 00ac78d39c29 ("ss: print tcpi_rcv_ssthresh")

 misc/ss.c | 188 +++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 120 insertions(+), 68 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 166267974c36..9d21ed7a0705 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -128,19 +128,21 @@ struct column {
 	const enum col_align align;
 	const char *header;
 	const char *ldelim;
-	int width;	/* Including delimiter. -1: fit to content, 0: hide */
+	int disabled;
+	int width;	/* Calculated, including additional layout spacing */
+	int max_len;	/* Measured maximum field length in this column */
 };
 
 static struct column columns[] = {
-	{ ALIGN_LEFT,	"Netid",		"",	0 },
-	{ ALIGN_LEFT,	"State",		" ",	0 },
-	{ ALIGN_LEFT,	"Recv-Q",		" ",	7 },
-	{ ALIGN_LEFT,	"Send-Q",		" ",	7 },
-	{ ALIGN_RIGHT,	"Local Address:",	" ",	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0 },
-	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0 },
-	{ ALIGN_LEFT,	"Port",			"",	0 },
-	{ ALIGN_LEFT,	"",			"",	-1 },
+	{ ALIGN_LEFT,	"Netid",		"",	0, 0, 0 },
+	{ ALIGN_LEFT,	"State",		" ",	0, 0, 0 },
+	{ ALIGN_LEFT,	"Recv-Q",		" ",	0, 0, 0 },
+	{ ALIGN_LEFT,	"Send-Q",		" ",	0, 0, 0 },
+	{ ALIGN_RIGHT,	"Local Address:",	" ",	0, 0, 0 },
+	{ ALIGN_LEFT,	"Port",			"",	0, 0, 0 },
+	{ ALIGN_RIGHT,	"Peer Address:",	" ",	0, 0, 0 },
+	{ ALIGN_LEFT,	"Port",			"",	0, 0, 0 },
+	{ ALIGN_LEFT,	"",			"",	0, 0, 0 },
 };
 
 static struct column *current_field = columns;
@@ -960,7 +962,7 @@ static void out(const char *fmt, ...)
 	char *pos;
 	int len;
 
-	if (!f->width)
+	if (f->disabled)
 		return;
 
 	if (!buffer.head)
@@ -983,7 +985,7 @@ static int print_left_spacing(struct column *f, int stored, int printed)
 {
 	int s;
 
-	if (f->width < 0 || f->align == ALIGN_LEFT)
+	if (!f->width || f->align == ALIGN_LEFT)
 		return 0;
 
 	s = f->width - stored - printed;
@@ -1001,7 +1003,7 @@ static void print_right_spacing(struct column *f, int printed)
 {
 	int s;
 
-	if (f->width < 0 || f->align == ALIGN_RIGHT)
+	if (!f->width || f->align == ALIGN_RIGHT)
 		return;
 
 	s = f->width - printed;
@@ -1018,9 +1020,12 @@ static void field_flush(struct column *f)
 	struct buf_chunk *chunk = buffer.tail;
 	unsigned int pad = buffer.cur->len % 2;
 
-	if (!f->width)
+	if (f->disabled)
 		return;
 
+	if (buffer.cur->len > f->max_len)
+		f->max_len = buffer.cur->len;
+
 	/* We need a new chunk if we can't store the next length descriptor.
 	 * Mind the gap between end of previous token and next aligned position
 	 * for length descriptor.
@@ -1063,7 +1068,7 @@ static void field_set(enum col_id id)
 static void print_header(void)
 {
 	while (!field_is_last(current_field)) {
-		if (current_field->width)
+		if (!current_field->disabled)
 			out(current_field->header);
 		field_next();
 	}
@@ -1096,16 +1101,106 @@ static void buf_free_all(void)
 	buffer.head = NULL;
 }
 
+/* Calculate column width from contents length. If columns don't fit on one
+ * line, break them into the least possible amount of lines and keep them
+ * aligned across lines. Available screen space is equally spread between fields
+ * as additional spacing.
+ */
+static void render_calc_width(int screen_width)
+{
+	int first, len = 0, linecols = 0;
+	struct column *c, *eol = columns - 1;
+
+	/* First pass: set width for each column to measured content length */
+	for (first = 1, c = columns; c - columns < COL_MAX; c++) {
+		if (c->disabled)
+			continue;
+
+		if (!first && c->max_len)
+			c->width = c->max_len + strlen(c->ldelim);
+		else
+			c->width = c->max_len;
+
+		/* But don't exceed screen size. If we exceed the screen size
+		 * for even a single field, it will just start on a line of its
+		 * own and then naturally wrap.
+		 */
+		c->width = min(c->width, screen_width);
+
+		if (c->width)
+			first = 0;
+	}
+
+	/* Second pass: find out newlines and distribute available spacing */
+	for (c = columns; c - columns < COL_MAX; c++) {
+		int pad, spacing, rem, last;
+		struct column *tmp;
+
+		if (!c->width)
+			continue;
+
+		linecols++;
+		len += c->width;
+
+		for (last = 1, tmp = c + 1; tmp - columns < COL_MAX; tmp++) {
+			if (tmp->width) {
+				last = 0;
+				break;
+			}
+		}
+
+		if (!last && len < screen_width) {
+			/* Columns fit on screen so far, nothing to do yet */
+			continue;
+		}
+
+		if (len == screen_width) {
+			/* Exact fit, just start with new line */
+			goto newline;
+		}
+
+		if (len > screen_width) {
+			/* Screen width exceeded: go back one column */
+			len -= c->width;
+			c--;
+			linecols--;
+		}
+
+		/* Distribute remaining space to columns on this line */
+		pad = screen_width - len;
+		spacing = pad / linecols;
+		rem = pad % linecols;
+		for (tmp = c; tmp > eol; tmp--) {
+			if (!tmp->width)
+				continue;
+
+			tmp->width += spacing;
+			if (rem) {
+				tmp->width++;
+				rem--;
+			}
+		}
+
+newline:
+		/* Line break: reset line counters, mark end-of-line */
+		eol = c;
+		len = 0;
+		linecols = 0;
+	}
+}
+
 /* Render buffered output with spacing and delimiters, then free up buffers */
-static void render(void)
+static void render(int screen_width)
 {
 	struct buf_token *token = (struct buf_token *)buffer.head->data;
-	int printed, line_started = 0, need_newline = 0;
+	int printed, line_started = 0;
 	struct column *f;
 
 	/* Ensure end alignment of last token, it wasn't necessarily flushed */
 	buffer.tail->end += buffer.cur->len % 2;
 
+	render_calc_width(screen_width);
+
 	/* Rewind and replay */
 	buffer.tail = buffer.head;
 
@@ -1125,23 +1220,16 @@ static void render(void)
 		printed += fwrite(token->data, 1, token->len, stdout);
 		print_right_spacing(f, printed);
 
-		/* Variable field size or overflow, won't align to screen */
-		if (printed > f->width)
-			need_newline = 1;
-
 		/* Go to next non-empty field, deal with end-of-line */
 		do {
 			if (field_is_last(f)) {
-				if (need_newline) {
-					printf("\n");
-					need_newline = 0;
-				}
+				printf("\n");
 				f = columns;
 				line_started = 0;
 			} else {
 				f++;
 			}
-		} while (!f->width);
+		} while (f->disabled);
 
 		token = buf_token_next(token);
 	}
@@ -4532,7 +4620,7 @@ int main(int argc, char *argv[])
 	FILE *filter_fp = NULL;
 	int ch;
 	int state_filter = 0;
-	int addrp_width, screen_width = 80;
+	int screen_width = 80;
 
 	while ((ch = getopt_long(argc, argv,
 				 "dhaletuwxnro460spbEf:miA:D:F:vVzZN:KHS",
@@ -4828,17 +4916,11 @@ int main(int argc, char *argv[])
 	if (ssfilter_parse(&current_filter.f, argc, argv, filter_fp))
 		usage();
 
-	if (current_filter.dbs&(current_filter.dbs-1))
-		columns[COL_NETID].width = 6;
-
-	if (current_filter.states&(current_filter.states-1))
-		columns[COL_STATE].width = 10;
+	if (!(current_filter.dbs & (current_filter.dbs - 1)))
+		columns[COL_NETID].disabled = 1;
 
-	/* If Netid or State are hidden, no delimiter before next column */
-	if (!columns[COL_NETID].width)
-		columns[COL_STATE].width--;
-	else if (!columns[COL_STATE].width)
-		columns[COL_RECVQ].width--;
+	if (!(current_filter.states & (current_filter.states - 1)))
+		columns[COL_STATE].disabled = 1;
 
 	if (isatty(STDOUT_FILENO)) {
 		struct winsize w;
@@ -4849,36 +4931,6 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	addrp_width = screen_width -
-		      columns[COL_NETID].width -
-		      columns[COL_STATE].width -
-		      columns[COL_RECVQ].width -
-		      columns[COL_SENDQ].width;
-
-	if (addrp_width&1) {
-		if (columns[COL_NETID].width)
-			columns[COL_NETID].width++;
-		else if (columns[COL_STATE].width)
-			columns[COL_STATE].width++;
-		else
-			columns[COL_SENDQ].width++;
-	}
-
-	addrp_width /= 2;
-
-	columns[COL_SERV].width = resolve_services ? 8 : 6;
-	if (addrp_width < 15 + columns[COL_SERV].width)
-		addrp_width = 15 + columns[COL_SERV].width;
-
-	columns[COL_ADDR].width = addrp_width - columns[COL_SERV].width;
-
-	/* Make enough space for the local/remote port field */
-	columns[COL_ADDR].width -= 13;
-	columns[COL_SERV].width += 13;
-
-	columns[COL_RADDR].width = columns[COL_ADDR].width;
-	columns[COL_RSERV].width = columns[COL_SERV].width;
-
 	if (show_header)
 		print_header();
 
@@ -4909,7 +4961,7 @@ int main(int argc, char *argv[])
 	if (show_users || show_proc_ctx || show_sock_ctx)
 		user_ent_destroy();
 
-	render();
+	render(screen_width);
 
 	return 0;
 }
-- 
2.9.4

^ permalink raw reply related

* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2017-12-12  1:07 UTC (permalink / raw)
  To: David Miller, Networking
  Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Jerome Brunet,
	Heiner Kallweit

Hi all,

Today's linux-next merge of the net-next tree got a conflict in:

  drivers/net/phy/meson-gxl.c

between commit:

  f1e2400a80ff ("net: phy: meson-gxl: detect LPA corruption")

from the net tree and commit:

  80274abafc60 ("net: phy: remove generic settings for callbacks config_aneg and read_status from drivers")

from the net-next tree.

I fixed it up (I just used the former) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/net/phy/meson-gxl.c
index 700007dd4be5,401e3234be58..000000000000
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@@ -130,9 -58,7 +130,8 @@@ static struct phy_driver meson_gxl_phy[
  		.features	= PHY_BASIC_FEATURES,
  		.flags		= PHY_IS_INTERNAL,
  		.config_init	= meson_gxl_config_init,
- 		.config_aneg	= genphy_config_aneg,
  		.aneg_done      = genphy_aneg_done,
 +		.read_status	= meson_gxl_read_status,
  		.suspend        = genphy_suspend,
  		.resume         = genphy_resume,
  	},

^ permalink raw reply

* [PATCH bpf 0/3] Misc BPF fixes
From: Daniel Borkmann @ 2017-12-12  1:25 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann

Couple of outstanding fixes for BPF tree: 1) fixes a perf RB
corruption, 2) and 3) fixes a few build issues from the recent
bpf_perf_event.h uapi corrections. Thanks!

Daniel Borkmann (3):
  bpf: fix corruption on concurrent perf_event_output calls
  bpf: fix build issues on um due to mising bpf_perf_event.h
  bpf: fix broken BPF selftest build

 arch/um/include/asm/Kbuild              |  1 +
 kernel/trace/bpf_trace.c                | 19 ++++++++++++-------
 tools/include/uapi/asm/bpf_perf_event.h |  7 +++++++
 tools/testing/selftests/bpf/Makefile    | 13 +------------
 4 files changed, 21 insertions(+), 19 deletions(-)
 create mode 100644 tools/include/uapi/asm/bpf_perf_event.h

-- 
2.9.5

^ permalink raw reply

* [PATCH bpf 1/3] bpf: fix corruption on concurrent perf_event_output calls
From: Daniel Borkmann @ 2017-12-12  1:25 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20171212012532.30268-1-daniel@iogearbox.net>

When tracing and networking programs are both attached in the
system and both use event-output helpers that eventually call
into perf_event_output(), then we could end up in a situation
where the tracing attached program runs in user context while
a cls_bpf program is triggered on that same CPU out of softirq
context.

Since both rely on the same per-cpu perf_sample_data, we could
potentially corrupt it. This can only ever happen in a combination
of the two types; all tracing programs use a bpf_prog_active
counter to bail out in case a program is already running on
that CPU out of a different context. XDP and cls_bpf programs
by themselves don't have this issue as they run in the same
context only. Therefore, split both perf_sample_data so they
cannot be accessed from each other.

Fixes: 20b9d7ac4852 ("bpf: avoid excessive stack usage for perf_sample_data")
Reported-by: Alexei Starovoitov <ast@fb.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Tested-by: Song Liu <songliubraving@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/trace/bpf_trace.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 0ce99c3..40207c2 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -343,14 +343,13 @@ static const struct bpf_func_proto bpf_perf_event_read_value_proto = {
 	.arg4_type	= ARG_CONST_SIZE,
 };
 
-static DEFINE_PER_CPU(struct perf_sample_data, bpf_sd);
+static DEFINE_PER_CPU(struct perf_sample_data, bpf_trace_sd);
 
 static __always_inline u64
 __bpf_perf_event_output(struct pt_regs *regs, struct bpf_map *map,
-			u64 flags, struct perf_raw_record *raw)
+			u64 flags, struct perf_sample_data *sd)
 {
 	struct bpf_array *array = container_of(map, struct bpf_array, map);
-	struct perf_sample_data *sd = this_cpu_ptr(&bpf_sd);
 	unsigned int cpu = smp_processor_id();
 	u64 index = flags & BPF_F_INDEX_MASK;
 	struct bpf_event_entry *ee;
@@ -373,8 +372,6 @@ __bpf_perf_event_output(struct pt_regs *regs, struct bpf_map *map,
 	if (unlikely(event->oncpu != cpu))
 		return -EOPNOTSUPP;
 
-	perf_sample_data_init(sd, 0, 0);
-	sd->raw = raw;
 	perf_event_output(event, sd, regs);
 	return 0;
 }
@@ -382,6 +379,7 @@ __bpf_perf_event_output(struct pt_regs *regs, struct bpf_map *map,
 BPF_CALL_5(bpf_perf_event_output, struct pt_regs *, regs, struct bpf_map *, map,
 	   u64, flags, void *, data, u64, size)
 {
+	struct perf_sample_data *sd = this_cpu_ptr(&bpf_trace_sd);
 	struct perf_raw_record raw = {
 		.frag = {
 			.size = size,
@@ -392,7 +390,10 @@ BPF_CALL_5(bpf_perf_event_output, struct pt_regs *, regs, struct bpf_map *, map,
 	if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
 		return -EINVAL;
 
-	return __bpf_perf_event_output(regs, map, flags, &raw);
+	perf_sample_data_init(sd, 0, 0);
+	sd->raw = &raw;
+
+	return __bpf_perf_event_output(regs, map, flags, sd);
 }
 
 static const struct bpf_func_proto bpf_perf_event_output_proto = {
@@ -407,10 +408,12 @@ static const struct bpf_func_proto bpf_perf_event_output_proto = {
 };
 
 static DEFINE_PER_CPU(struct pt_regs, bpf_pt_regs);
+static DEFINE_PER_CPU(struct perf_sample_data, bpf_misc_sd);
 
 u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
 		     void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy)
 {
+	struct perf_sample_data *sd = this_cpu_ptr(&bpf_misc_sd);
 	struct pt_regs *regs = this_cpu_ptr(&bpf_pt_regs);
 	struct perf_raw_frag frag = {
 		.copy		= ctx_copy,
@@ -428,8 +431,10 @@ u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
 	};
 
 	perf_fetch_caller_regs(regs);
+	perf_sample_data_init(sd, 0, 0);
+	sd->raw = &raw;
 
-	return __bpf_perf_event_output(regs, map, flags, &raw);
+	return __bpf_perf_event_output(regs, map, flags, sd);
 }
 
 BPF_CALL_0(bpf_get_current_task)
-- 
2.9.5

^ permalink raw reply related

* [PATCH bpf 3/3] bpf: fix broken BPF selftest build
From: Daniel Borkmann @ 2017-12-12  1:25 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann, Hendrik Brueckner,
	Arnaldo Carvalho de Melo
In-Reply-To: <20171212012532.30268-1-daniel@iogearbox.net>

At least on x86_64, the kernel's BPF selftests seemed to have stopped
to build due to 618e165b2a8e ("selftests/bpf: sync kernel headers and
introduce arch support in Makefile"):

  [...]
  In file included from test_verifier.c:29:0:
  ../../../include/uapi/linux/bpf_perf_event.h:11:32:
     fatal error: asm/bpf_perf_event.h: No such file or directory
   #include <asm/bpf_perf_event.h>
                                ^
  compilation terminated.
  [...]

While pulling in tools/arch/*/include/uapi/asm/bpf_perf_event.h seems
to work fine, there's no automated fall-back logic right now that would
do the same out of tools/include/uapi/asm-generic/bpf_perf_event.h. The
usual convention today is to add a include/[uapi/]asm/ equivalent that
would pull in the correct arch header or generic one as fall-back, all
ifdef'ed based on compiler target definition. It's similarly done also
in other cases such as tools/include/asm/barrier.h, thus adapt the same
here.

Fixes: 618e165b2a8e ("selftests/bpf: sync kernel headers and introduce arch support in Makefile")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/include/uapi/asm/bpf_perf_event.h |  7 +++++++
 tools/testing/selftests/bpf/Makefile    | 13 +------------
 2 files changed, 8 insertions(+), 12 deletions(-)
 create mode 100644 tools/include/uapi/asm/bpf_perf_event.h

diff --git a/tools/include/uapi/asm/bpf_perf_event.h b/tools/include/uapi/asm/bpf_perf_event.h
new file mode 100644
index 0000000..13a5853
--- /dev/null
+++ b/tools/include/uapi/asm/bpf_perf_event.h
@@ -0,0 +1,7 @@
+#if defined(__aarch64__)
+#include "../../arch/arm64/include/uapi/asm/bpf_perf_event.h"
+#elif defined(__s390__)
+#include "../../arch/s390/include/uapi/asm/bpf_perf_event.h"
+#else
+#include <uapi/asm-generic/bpf_perf_event.h>
+#endif
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 21a2d76..792af7c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,19 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
-ifeq ($(srctree),)
-srctree := $(patsubst %/,%,$(dir $(CURDIR)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-srctree := $(patsubst %/,%,$(dir $(srctree)))
-endif
-include $(srctree)/tools/scripts/Makefile.arch
-
-$(call detected_var,SRCARCH)
-
 LIBDIR := ../../../lib
 BPFDIR := $(LIBDIR)/bpf
 APIDIR := ../../../include/uapi
-ASMDIR:= ../../../arch/$(ARCH)/include/uapi
 GENDIR := ../../../../include/generated
 GENHDR := $(GENDIR)/autoconf.h
 
@@ -21,7 +10,7 @@ ifneq ($(wildcard $(GENHDR)),)
   GENFLAGS := -DHAVE_GENHDR
 endif
 
-CFLAGS += -Wall -O2 -I$(APIDIR) -I$(ASMDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
+CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
 LDLIBS += -lcap -lelf
 
 TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
-- 
2.9.5

^ permalink raw reply related

* [PATCH bpf 2/3] bpf: fix build issues on um due to mising bpf_perf_event.h
From: Daniel Borkmann @ 2017-12-12  1:25 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann, Hendrik Brueckner, Richard Weinberger
In-Reply-To: <20171212012532.30268-1-daniel@iogearbox.net>

Since c895f6f703ad ("bpf: correct broken uapi for
BPF_PROG_TYPE_PERF_EVENT program type") um (uml) won't build
on i386 or x86_64:

  [...]
    CC      init/main.o
  In file included from ../include/linux/perf_event.h:18:0,
                   from ../include/linux/trace_events.h:10,
                   from ../include/trace/syscall.h:7,
                   from ../include/linux/syscalls.h:82,
                   from ../init/main.c:20:
  ../include/uapi/linux/bpf_perf_event.h:11:32: fatal error:
  asm/bpf_perf_event.h: No such file or directory #include
  <asm/bpf_perf_event.h>
  [...]

Lets add missing bpf_perf_event.h also to um arch. This seems
to be the only one still missing.

Fixes: c895f6f703ad ("bpf: correct broken uapi for BPF_PROG_TYPE_PERF_EVENT program type")
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Suggested-by: Richard Weinberger <richard@sigma-star.at>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>
Cc: Richard Weinberger <richard@sigma-star.at>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 arch/um/include/asm/Kbuild | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/um/include/asm/Kbuild b/arch/um/include/asm/Kbuild
index 50a32c3..73c57f6 100644
--- a/arch/um/include/asm/Kbuild
+++ b/arch/um/include/asm/Kbuild
@@ -1,4 +1,5 @@
 generic-y += barrier.h
+generic-y += bpf_perf_event.h
 generic-y += bug.h
 generic-y += clkdev.h
 generic-y += current.h
-- 
2.9.5

^ permalink raw reply related

* Re: [PATCH] selftests: bpf: Adding config fragment CONFIG_CGROUP_BPF=y
From: Daniel Borkmann @ 2017-12-12  1:34 UTC (permalink / raw)
  To: Naresh Kamboju, netdev; +Cc: guro, shuahkh, shuah, linux-kselftest
In-Reply-To: <1513020323-29591-1-git-send-email-naresh.kamboju@linaro.org>

On 12/11/2017 08:25 PM, Naresh Kamboju wrote:
> CONFIG_CGROUP_BPF=y is required for test_dev_cgroup test case.
> 
> Signed-off-by: Naresh Kamboju <naresh.kamboju@linaro.org>

Applied to bpf-next, thanks Naresh!

^ permalink raw reply

* [PATCH v2] igb: Free IRQs when device is hotplugged
From: Lyude Paul @ 2017-12-12  1:34 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Todd Fujinaka, Stephen Hemminger, stable, Jeff Kirsher, netdev,
	linux-kernel

Recently I got a Caldigit TS3 Thunderbolt 3 dock, and noticed that upon
hotplugging my kernel would immediately crash due to igb:

[  680.825801] kernel BUG at drivers/pci/msi.c:352!
[  680.828388] invalid opcode: 0000 [#1] SMP
[  680.829194] Modules linked in: igb(O) thunderbolt i2c_algo_bit joydev vfat fat btusb btrtl btbcm btintel bluetooth ecdh_generic hp_wmi sparse_keymap rfkill wmi_bmof iTCO_wdt intel_rapl x86_pkg_temp_thermal coretemp crc32_pclmul snd_pcm rtsx_pci_ms mei_me snd_timer memstick snd pcspkr mei soundcore i2c_i801 tpm_tis psmouse shpchp wmi tpm_tis_core tpm video hp_wireless acpi_pad rtsx_pci_sdmmc mmc_core crc32c_intel serio_raw rtsx_pci mfd_core xhci_pci xhci_hcd i2c_hid i2c_core [last unloaded: igb]
[  680.831085] CPU: 1 PID: 78 Comm: kworker/u16:1 Tainted: G           O     4.15.0-rc3Lyude-Test+ #6
[  680.831596] Hardware name: HP HP ZBook Studio G4/826B, BIOS P71 Ver. 01.03 06/09/2017
[  680.832168] Workqueue: kacpi_hotplug acpi_hotplug_work_fn
[  680.832687] RIP: 0010:free_msi_irqs+0x180/0x1b0
[  680.833271] RSP: 0018:ffffc9000030fbf0 EFLAGS: 00010286
[  680.833761] RAX: ffff8803405f9c00 RBX: ffff88033e3d2e40 RCX: 000000000000002c
[  680.834278] RDX: 0000000000000000 RSI: 00000000000000ac RDI: ffff880340be2178
[  680.834832] RBP: 0000000000000000 R08: ffff880340be1ff0 R09: ffff8803405f9c00
[  680.835342] R10: 0000000000000000 R11: 0000000000000040 R12: ffff88033d63a298
[  680.835822] R13: ffff88033d63a000 R14: 0000000000000060 R15: ffff880341959000
[  680.836332] FS:  0000000000000000(0000) GS:ffff88034f440000(0000) knlGS:0000000000000000
[  680.836817] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  680.837360] CR2: 000055e64044afdf CR3: 0000000001c09002 CR4: 00000000003606e0
[  680.837954] Call Trace:
[  680.838853]  pci_disable_msix+0xce/0xf0
[  680.839616]  igb_reset_interrupt_capability+0x5d/0x60 [igb]
[  680.840278]  igb_remove+0x9d/0x110 [igb]
[  680.840764]  pci_device_remove+0x36/0xb0
[  680.841279]  device_release_driver_internal+0x157/0x220
[  680.841739]  pci_stop_bus_device+0x7d/0xa0
[  680.842255]  pci_stop_bus_device+0x2b/0xa0
[  680.842722]  pci_stop_bus_device+0x3d/0xa0
[  680.843189]  pci_stop_and_remove_bus_device+0xe/0x20
[  680.843627]  trim_stale_devices+0xf3/0x140
[  680.844086]  trim_stale_devices+0x94/0x140
[  680.844532]  trim_stale_devices+0xa6/0x140
[  680.845031]  ? get_slot_status+0x90/0xc0
[  680.845536]  acpiphp_check_bridge.part.5+0xfe/0x140
[  680.846021]  acpiphp_hotplug_notify+0x175/0x200
[  680.846581]  ? free_bridge+0x100/0x100
[  680.847113]  acpi_device_hotplug+0x8a/0x490
[  680.847535]  acpi_hotplug_work_fn+0x1a/0x30
[  680.848076]  process_one_work+0x182/0x3a0
[  680.848543]  worker_thread+0x2e/0x380
[  680.848963]  ? process_one_work+0x3a0/0x3a0
[  680.849373]  kthread+0x111/0x130
[  680.849776]  ? kthread_create_worker_on_cpu+0x50/0x50
[  680.850188]  ret_from_fork+0x1f/0x30
[  680.850601] Code: 43 14 85 c0 0f 84 d5 fe ff ff 31 ed eb 0f 83 c5 01 39 6b 14 0f 86 c5 fe ff ff 8b 7b 10 01 ef e8 b7 e4 d2 ff 48 83 78 70 00 74 e3 <0f> 0b 49 8d b5 a0 00 00 00 e8 62 6f d3 ff e9 c7 fe ff ff 48 8b
[  680.851497] RIP: free_msi_irqs+0x180/0x1b0 RSP: ffffc9000030fbf0

As it turns out, normally the freeing of IRQs that would fix this is called
inside of the scope of __igb_close(). However, since the device is
already gone by the point we try to unregister the netdevice from the
driver due to a hotplug we end up seeing that the netif isn't present
and thus, forget to free any of the device IRQs.

So: make sure that if we're in the process of dismantling the netdev, we
always allow __igb_close() to be called so that IRQs may be freed
normally. Additionally, only allow igb_close() to be called from
__igb_close() if it hasn't already been called for the given adapter.

Signed-off-by: Lyude Paul <lyude@redhat.com>
Fixes: 9474933caf21 ("igb: close/suspend race in netif_device_detach")
Cc: Todd Fujinaka <todd.fujinaka@intel.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: stable@vger.kernel.org
---
Changes since v1:
  - Remove code for freeing IRQs from igb_remove(), unbreak
    __igb_close() instead (re: Stephen Hemminger)

 drivers/net/ethernet/intel/igb/igb_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index c208753ff5b7..a1083fd074dd 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3663,7 +3663,9 @@ static int __igb_close(struct net_device *netdev, bool suspending)
 	if (!suspending)
 		pm_runtime_get_sync(&pdev->dev);
 
-	igb_down(adapter);
+	if (!test_bit(__IGB_DOWN, &adapter->state))
+		igb_down(adapter);
+
 	igb_free_irq(adapter);
 
 	igb_free_all_tx_resources(adapter);
@@ -3676,7 +3678,7 @@ static int __igb_close(struct net_device *netdev, bool suspending)
 
 int igb_close(struct net_device *netdev)
 {
-	if (netif_device_present(netdev))
+	if (netif_device_present(netdev) || netdev->dismantle)
 		return __igb_close(netdev, false);
 	return 0;
 }
-- 
2.14.3

^ permalink raw reply related

* linux-next: build failure after merge of the mac80211-next tree
From: Stephen Rothwell @ 2017-12-12  1:58 UTC (permalink / raw)
  To: Johannes Berg, Kalle Valo, Wireless
  Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Felix Fietkau,
	Lorenzo Bianconi, Toke Høiland-Jørgensen, David Miller,
	Networking

Hi Johannes,

After merging the mac80211-next tree, today's linux-next build (x86_64
allmodconfig) failed like this:

drivers/net/wireless/mediatek/mt76/mt76x2_main.c:539:19: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
  .wake_tx_queue = mt76_wake_tx_queue,
                   ^
drivers/net/wireless/mediatek/mt76/mt76x2_main.c:539:19: note: (near initialization for 'mt76x2_ops.wake_tx_queue')

Caused by commits

  17f1de56df05 ("mt76: add common code shared between multiple chipsets")
  7bc04215a66b ("mt76: add driver code for MT76x2e")

from the wireless-drivers-next tree interacting with commit

  e937b8da5a59 ("mac80211: Add TXQ scheduling API")

from the mac80211-next tree.

I applied the below hack merge fix ... please let me know if something
more/better is required.  Someone needs to remember to tell Dave when
these trees meet in his tree.

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 12 Dec 2017 12:50:40 +1100
Subject: [PATCH] mt76: fix up for "mac80211: Add TXQ scheduling API"

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/wireless/mediatek/mt76/mt76.h |  2 +-
 drivers/net/wireless/mediatek/mt76/tx.c   | 10 +++++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index aa0880bbea7f..e395d3859212 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -338,7 +338,7 @@ void mt76_tx(struct mt76_dev *dev, struct ieee80211_sta *sta,
 	     struct mt76_wcid *wcid, struct sk_buff *skb);
 void mt76_txq_init(struct mt76_dev *dev, struct ieee80211_txq *txq);
 void mt76_txq_remove(struct mt76_dev *dev, struct ieee80211_txq *txq);
-void mt76_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *txq);
+void mt76_wake_tx_queue(struct ieee80211_hw *hw);
 void mt76_stop_tx_queues(struct mt76_dev *dev, struct ieee80211_sta *sta,
 			 bool send_bar);
 void mt76_txq_schedule(struct mt76_dev *dev, struct mt76_queue *hwq);
diff --git a/drivers/net/wireless/mediatek/mt76/tx.c b/drivers/net/wireless/mediatek/mt76/tx.c
index 4eef69bd8a9e..ad414af0750f 100644
--- a/drivers/net/wireless/mediatek/mt76/tx.c
+++ b/drivers/net/wireless/mediatek/mt76/tx.c
@@ -463,12 +463,16 @@ void mt76_stop_tx_queues(struct mt76_dev *dev, struct ieee80211_sta *sta,
 }
 EXPORT_SYMBOL_GPL(mt76_stop_tx_queues);
 
-void mt76_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *txq)
+void mt76_wake_tx_queue(struct ieee80211_hw *hw)
 {
+	struct ieee80211_txq *txq;
 	struct mt76_dev *dev = hw->priv;
-	struct mt76_txq *mtxq = (struct mt76_txq *) txq->drv_priv;
-	struct mt76_queue *hwq = mtxq->hwq;
+	struct mt76_txq *mtxq;
+	struct mt76_queue *hwq;
 
+	txq = ieee80211_next_txq(hw);
+	mtxq = (struct mt76_txq *) txq->drv_priv;
+	hwq = mtxq->hwq;
 	spin_lock_bh(&hwq->lock);
 	if (list_empty(&mtxq->list))
 		list_add_tail(&mtxq->list, &hwq->swq);
-- 
2.15.0

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply related

* Re: [PATCH v3 17/33] nds32: VDSO support
From: Vincent Chen @ 2017-12-12  1:58 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Greentime Hu, Greentime, Linux Kernel Mailing List, Arnd Bergmann,
	linux-arch, Thomas Gleixner, Jason Cooper, Marc Zyngier,
	Rob Herring, netdev, DTML, Al Viro, David Howells, Will Deacon,
	Daniel Lezcano, linux-serial, Geert Uytterhoeven, Linus Walleij,
	Greg KH, Vincent Chen
In-Reply-To: <20171208121405.dy6raczdlbxtbnfo@lakrids.cambridge.arm.com>

2017-12-08 20:14 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
> On Fri, Dec 08, 2017 at 07:54:42PM +0800, Greentime Hu wrote:
>> 2017-12-08 18:21 GMT+08:00 Mark Rutland <mark.rutland@arm.com>:
>> > On Fri, Dec 08, 2017 at 05:12:00PM +0800, Greentime Hu wrote:
>> >> +static int grab_timer_node_info(void)
>> >> +{
>> >> +     struct device_node *timer_node;
>> >> +
>> >> +     timer_node = of_find_node_by_name(NULL, "timer");
>> >
>> > Please use a compatible string, rather than matching the timer by name.
>> >
>> > It's plausible that you have multiple nodes called "timer" in the DT,
>> > under different parent nodes, and this might not be the device you
>> > think it is. I see your dt in patch 24 has two timer nodes.
>> >
>> > It would be best if your clocksource driver exposed some stuct that you
>> > looked at here, so that you're guaranteed to user the same device.
>>
>> We'd like to use "timer" here because there are 2 different timer IPs
>> and we are sure that they won't be in the same SoC.
>> We think this implementation in VDSO should be platform independent to
>> get cycle-count register.
>> Our customer or other SoC provider who can use "timer" and define
>> cycle-count-offset or cycle-count-down then we can get the correct
>> cycle-count.
>
> This is not the right way to do things.
>
> So from a DT perspective, NAK.
>
> You should not add properties to arbitrary DT bindings to handle a Linux
> implementation detail.
>
> Please remove this DT code, and have the drivers for those timer blocks
> export this information to your vdso code somehow.
>

Hi, Mark:
Based on your suggestion, we define a new sturct timer_info to let
timer driver record the value
of cycle-count-offset and cycle-count-down in timer_init function. The
above code in timer driver
is validate only when CONFIG_NDS32 is defined.

>> We sent atcpit100 patch last time along with our arch, however we'd
>> like to send it to its sub system this time and my colleague is still
>> working on it.
>> He may send the timer patch next week.
>
> I think that it would make sense for that patch to be part of the arch
> port, especially given that (AFAICT) there is no dirver for the other
> timer IP that you mention.
>
> [...]
>
>> >> +int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
>> >> +{
>> >
>> >> +     /*Map timer to user space */
>> >> +     vdso_base += PAGE_SIZE;
>> >> +     prot = __pgprot(_PAGE_V | _PAGE_M_UR_KR | _PAGE_D |
>> >> +                     _PAGE_G | _PAGE_C_DEV);
>> >> +     ret = io_remap_pfn_range(vma, vdso_base, timer_res.start >> PAGE_SHIFT,
>> >> +                              PAGE_SIZE, prot);
>> >> +     if (ret)
>> >> +             goto up_fail;
>> >
>> > Maybe this is fine, but it looks a bit suspicious.
>> >
>> > Is it safe to map IO memory to a userspace process like this?
>> >
>> > In general that isn't safe, since userspace could access other registers
>> > (if those exist), perform accesses that change the state of hardware, or
>> > make unsupported access types (e.g. unaligned, atomic) that result in
>> > errors the kernel can't handle.
>> >
>> > Does none of that apply here?
>>
>> We only provide read permission to this page so hareware state won't
>> be chagned. It will trigger exception if we try to write.
>> We will check about the alignment/atomic issue of this region.
>

For alignment issue, we intentionally make an un-alignment read to
access this region and we
got "Segmentation fault" as expected.


Thanks,
Vincent

> Ok, thanks.
>
> This is another reason to only do this for devices/drivers that we have
> drivers for, since we can't know that this is safe in general.
>
> Thanks,
> Mark.

^ permalink raw reply

* Re: [PATCH v3 net-next 0/9] net: Generic network resolver backend and ILA resolver
From: David Miller @ 2017-12-12  2:16 UTC (permalink / raw)
  To: tom; +Cc: netdev, roopa, rohit
In-Reply-To: <CAPDqMer_F=uFZ08LytAbU7hwT4d98iYFGfHHFKKtL1mB4a+6qw@mail.gmail.com>

From: Tom Herbert <tom@quantonium.net>
Date: Mon, 11 Dec 2017 14:16:17 -0800

> How can we build a system that allows an unlimited number of
> resolutions without drop?

IPV4 routing solves this with a prefixed trie, for example.

The fundamental backing datastructure for the switching
or whatever operation must be in-memory, in the kernel,
scalable, and without a fronting "cache".

^ permalink raw reply

* Re: [PATCH net-next v4 0/2] bpf/tracing: allow user space to query prog array on the same tp
From: Daniel Borkmann @ 2017-12-12  2:17 UTC (permalink / raw)
  To: Yonghong Song, peterz, ast, netdev; +Cc: kernel-team
In-Reply-To: <20171211193903.2428317-1-yhs@fb.com>

On 12/11/2017 08:39 PM, Yonghong Song wrote:
> Commit e87c6bc3852b ("bpf: permit multiple bpf attachments
> for a single perf event") added support to attach multiple
> bpf programs to a single perf event. Given a perf event
> (kprobe, uprobe, or kernel tracepoint), the perf ioctl interface
> is used to query bpf programs attached to the same trace event.
> 
> There already exists a BPF_PROG_QUERY command for introspection
> currently used by cgroup+bpf. We did have an implementation for
> querying tracepoint+bpf through the same interface. However, it
> looks cleaner to use ioctl() style of api here, since attaching
> bpf prog to tracepoint/kuprobe is also done via ioctl.
> 
> Patch #1 had the core implementation and patch #2 added
> a test case in tools bpf selftests suite.
> 
> Changelogs:
> v3 -> v4:
>   - Fix a compilation error with newer gcc like 6.3.1 while
>     old gcc 4.8.5 is okay. I was using &uquery->ids to represent
>     the address to the ids array to make it explicit that the
>     address is passed, and this syntax is rightly rejected
>     by gcc 6.3.1.

Series applied to bpf-next, thanks Yonghong.

^ permalink raw reply

* Re: [PATCH net-next] libbpf: add function to setup XDP
From: Daniel Borkmann @ 2017-12-12  2:23 UTC (permalink / raw)
  To: David Ahern, Eric Leblond, Jakub Kicinski; +Cc: netdev, linux-kernel, ast
In-Reply-To: <49b37939-ebd4-fe92-eeab-e5d2713cadb8@gmail.com>

On 12/10/2017 10:07 PM, David Ahern wrote:
> On 12/10/17 1:34 PM, Eric Leblond wrote:
>>> Would it be possible to print out or preferably return to the caller
>>> the ext ack error message?  A couple of drivers are using it for XDP
>>> mis-configuration reporting instead of printks.  We should encourage
>>> other to do the same and support it in all user space since ext ack 
>>> msgs lead to much better user experience.
>>
>> I've seen the kind of messages displayed by reading at kernel log. They
>> are really useful and it looks almost mandatory to be able to display
>> them.
>>
>> Kernel code seems to not have a parser for the ext ack error message.
>> Did I miss something here ?
>>  
>> Looking at tc code, it seems it is using libmnl to parse them and I
>> doubt it is a good idea to use that in libbpf as it is introducing a
>> dependency.
>>
>> Does someone has an existing parsing code or should I write on my own ?
> 
> I had worked on extack for libbpf but seem to have lost the changes.
> 
> Look at the commits here:
>     https://github.com/dsahern/iproute2/commits/ext-ack
> 
> I suggest using this:
> 
> https://github.com/dsahern/iproute2/commit/b61e4c7dd54a5d3ff98640da4b480441cee497b2
> 
> to bring in nlattr from lib/nlattr (as I recall lib/nlattr can not be
> used directly). From there, use this one:
> 
> https://github.com/dsahern/iproute2/commit/261f7251e6704d565b91e310faabbbb7e18d14a1
> 
> to see what is needed for extack support.
> 
> Really not that much code to add.

+1, ext ack support would improve troubleshooting a lot here; please
add and respin. Thanks, Eric!

^ permalink raw reply

* Re: [PATCH net-next v5 2/2] net: ethernet: socionext: add AVE ethernet driver
From: Masami Hiramatsu @ 2017-12-12  2:29 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Kunihiko Hayashi, Mark Rutland, Andrew Lunn, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	Linux kernel mailing list, Masahiro Yamada, Rob Herring,
	Philippe Ombredanne, Jassi Brar, David S. Miller,
	linux-arm-kernel
In-Reply-To: <20171211134627.GU10595-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>

Hi Russell,

2017-12-11 22:46 GMT+09:00 Russell King - ARM Linux <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>:
> On Mon, Dec 11, 2017 at 10:34:17PM +0900, Masami Hiramatsu wrote:
>> IMHO, even if we use SPDX license identifier, I recommend to use
>> C-style comments as many other files do, since it is C code.
>> If SPDX identifier requires C++ style, that is SPDX parser's issue
>> and should be fixed to get it from C-style comment.
>
> See the numerous emails on this subject already.  The issue of C
> vs C++ comments has come up many times by many different people, but
> the result is the same.  That's not going to happen.  Linux kernel
> C files are required to use "//" for the SPDX identifier by order
> of Linus Torvalds.

OK, I got it.

>
> Linus has also revealed in that discussion that he has a preference
> for "//" style commenting for single comments, so it seems that the
> kernel coding style may change - but there is no desire for patches
> to "clean up" single line comments to use "//".

Thank you for making it clear.

Then what I'm considering is copyright notice lines. Those are usually
treat as the header lines, not single line. So

> +// SDPX-License-Identifier: GPL-2.0
> +// sni_ave.c - Socionext UniPhier AVE ethernet driver
> +// Copyright 2014 Panasonic Corporation
> +// Copyright 2015-2017 Socionext Inc.

is acceptable? or should we keep C-style header lines for new drivers?

> +// SDPX-License-Identifier: GPL-2.0
> +/*
> + * sni_ave.c - Socionext UniPhier AVE ethernet driver
> + * Copyright 2014 Panasonic Corporation
> + * Copyright 2015-2017 Socionext Inc.
> + */

I just concern that those lines are not "single". that's all. :)

>
> For further information, and to see the discussion that has already
> happened, the arguments that have been made about style, see the
> threads for the patch series that tglx has been posting wrt documenting
> the SPDX stuff for the kernel.

OK, got it.

https://lkml.org/lkml/2017/11/16/663


Thanks,

>
> Thanks (let's stop rehashing the same arguments.)
>


-- 
Masami Hiramatsu
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ 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