Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
From: Andrew Lunn @ 2018-04-12 14:04 UTC (permalink / raw)
  To: Phil Elwell
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <1523541336-145953-5-git-send-email-phil@raspberrypi.org>

On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote:
> The Microchip LAN78XX family of devices are Ethernet controllers with
> a USB interface. Despite being discoverable devices it can be useful to
> be able to configure them from Device Tree, particularly in low-cost
> applications without an EEPROM or programmed OTP.
> 
> Document the supported properties in a bindings file, adding it to
> MAINTAINERS at the same time.

Hi Phil

How you link an OF node to a USB device is not obvious. Could you
please include either a pointer to some binding documentation, or make
your example show it.

Thanks
	Andrew

^ permalink raw reply

* Re: [RFC PATCH v2 03/14] xsk: add umem fill queue support and mmap
From: Michael S. Tsirkin @ 2018-04-12 14:04 UTC (permalink / raw)
  To: Karlsson, Magnus
  Cc: Björn Töpel, Duyck, Alexander H,
	alexander.duyck@gmail.com, john.fastabend@gmail.com, ast@fb.com,
	brouer@redhat.com, willemdebruijn.kernel@gmail.com,
	daniel@iogearbox.net, netdev@vger.kernel.org,
	michael.lundkvist@ericsson.com, Brandeburg, Jesse,
	Singhai, Anjali, Zhang, Qi Z, ravineet.singh@ericsson.com
In-Reply-To: <AFED4FBCE79F3548A8F74434195ACE39588D1AA4@IRSMSX107.ger.corp.intel.com>

On Thu, Apr 12, 2018 at 07:38:25AM +0000, Karlsson, Magnus wrote:
> I think you are definitely right in that there are ways in which
> we can improve performance here. That said, the current queue
> performs slightly better than the previous one we had that was
> more or less a copy of one of your first virtio 1.1 proposals
> from little over a year ago. It had bidirectional queues and a
> valid flag in the descriptor itself. The reason we abandoned this
> was not poor performance (it was good), but a need to go to
> unidirectional queues. Maybe I should have only changed that
> aspect and kept the valid flag.

Is there a summary about unidirectional queues anywhere?  I'm curious to
know whether there are any lessons here to be learned for virtio
or ptr_ring.

-- 
MST

^ permalink raw reply

* Re: [PATCH 1/4] lan78xx: Read MAC address from DT if present
From: Andrew Lunn @ 2018-04-12 14:06 UTC (permalink / raw)
  To: Phil Elwell
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <1523541336-145953-2-git-send-email-phil@raspberrypi.org>

Hi Phil

> -			ret = lan78xx_write_reg(dev, RX_ADDRL, addr_lo);
> -			ret = lan78xx_write_reg(dev, RX_ADDRH, addr_hi);
> +		mac_addr = of_get_mac_address(dev->udev->dev.of_node);

It might be better to use the higher level eth_platform_get_mac_address().

   Andrew

^ permalink raw reply

* Re: [PATCH net-next] Per interface IPv4 stats (CONFIG_IP_IFSTATS_TABLE)
From: kbuild test robot @ 2018-04-12 14:09 UTC (permalink / raw)
  To: Stephen Suryaputra; +Cc: kbuild-all, netdev, Stephen Suryaputra
In-Reply-To: <1523415335-17154-1-git-send-email-ssuryaextr@gmail.com>

Hi Stephen,

Thank you for the patch! Perhaps something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Stephen-Suryaputra/Per-interface-IPv4-stats-CONFIG_IP_IFSTATS_TABLE/20180412-181719
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   net/ipv4/proc.c:414:28: sparse: Variable length array is used.
>> net/ipv4/proc.c:499:43: sparse: incorrect type in argument 1 (different address spaces) @@    expected void [noderef] <asn:3>*mib @@    got vvoid [noderef] <asn:3>*mib @@
   net/ipv4/proc.c:499:43:    expected void [noderef] <asn:3>*mib
   net/ipv4/proc.c:499:43:    got void [noderef] <asn:3>**pcpumib
>> net/ipv4/proc.c:532:34: sparse: cast removes address space of expression
   net/ipv4/proc.c:534:34: sparse: cast removes address space of expression

vim +499 net/ipv4/proc.c

   411	
   412	static int snmp_seq_show_tcp_udp(struct seq_file *seq, void *v)
   413	{
 > 414		unsigned long buff[TCPUDP_MIB_MAX];
   415		struct net *net = seq->private;
   416		int i;
   417	
   418		memset(buff, 0, TCPUDP_MIB_MAX * sizeof(unsigned long));
   419	
   420		seq_puts(seq, "\nTcp:");
   421		for (i = 0; snmp4_tcp_list[i].name; i++)
   422			seq_printf(seq, " %s", snmp4_tcp_list[i].name);
   423	
   424		seq_puts(seq, "\nTcp:");
   425		snmp_get_cpu_field_batch(buff, snmp4_tcp_list,
   426					 net->mib.tcp_statistics);
   427		for (i = 0; snmp4_tcp_list[i].name; i++) {
   428			/* MaxConn field is signed, RFC 2012 */
   429			if (snmp4_tcp_list[i].entry == TCP_MIB_MAXCONN)
   430				seq_printf(seq, " %ld", buff[i]);
   431			else
   432				seq_printf(seq, " %lu", buff[i]);
   433		}
   434	
   435		memset(buff, 0, TCPUDP_MIB_MAX * sizeof(unsigned long));
   436	
   437		snmp_get_cpu_field_batch(buff, snmp4_udp_list,
   438					 net->mib.udp_statistics);
   439		seq_puts(seq, "\nUdp:");
   440		for (i = 0; snmp4_udp_list[i].name; i++)
   441			seq_printf(seq, " %s", snmp4_udp_list[i].name);
   442		seq_puts(seq, "\nUdp:");
   443		for (i = 0; snmp4_udp_list[i].name; i++)
   444			seq_printf(seq, " %lu", buff[i]);
   445	
   446		memset(buff, 0, TCPUDP_MIB_MAX * sizeof(unsigned long));
   447	
   448		/* the UDP and UDP-Lite MIBs are the same */
   449		seq_puts(seq, "\nUdpLite:");
   450		snmp_get_cpu_field_batch(buff, snmp4_udp_list,
   451					 net->mib.udplite_statistics);
   452		for (i = 0; snmp4_udp_list[i].name; i++)
   453			seq_printf(seq, " %s", snmp4_udp_list[i].name);
   454		seq_puts(seq, "\nUdpLite:");
   455		for (i = 0; snmp4_udp_list[i].name; i++)
   456			seq_printf(seq, " %lu", buff[i]);
   457	
   458		seq_putc(seq, '\n');
   459		return 0;
   460	}
   461	
   462	static int snmp_seq_show(struct seq_file *seq, void *v)
   463	{
   464		snmp_seq_show_ipstats(seq, v);
   465	
   466		icmp_put(seq);	/* RFC 2011 compatibility */
   467		icmpmsg_put(seq);
   468	
   469		snmp_seq_show_tcp_udp(seq, v);
   470	
   471		return 0;
   472	}
   473	
   474	static int snmp_seq_open(struct inode *inode, struct file *file)
   475	{
   476		return single_open_net(inode, file, snmp_seq_show);
   477	}
   478	
   479	static const struct file_operations snmp_seq_fops = {
   480		.open	 = snmp_seq_open,
   481		.read	 = seq_read,
   482		.llseek	 = seq_lseek,
   483		.release = single_release_net,
   484	};
   485	
   486	
   487	#ifdef CONFIG_IP_IFSTATS_TABLE
   488	static void snmp_seq_show_item(struct seq_file *seq, void __percpu **pcpumib,
   489				       atomic_long_t *smib,
   490				       const struct snmp_mib *itemlist,
   491				       char *prefix)
   492	{
   493		char name[32];
   494		int i;
   495		unsigned long val;
   496	
   497		for (i = 0; itemlist[i].name; i++) {
   498			val = pcpumib ?
 > 499				snmp_fold_field64(pcpumib, itemlist[i].entry,
   500						  offsetof(struct ipstats_mib, syncp)) :
   501				atomic_long_read(smib + itemlist[i].entry);
   502			snprintf(name, sizeof(name), "%s%s",
   503				 prefix, itemlist[i].name);
   504			seq_printf(seq, "%-32s\t%lu\n", name, val);
   505		}
   506	}
   507	
   508	static void snmp_seq_show_icmpmsg(struct seq_file *seq, atomic_long_t *smib)
   509	{
   510		char name[32];
   511		int i;
   512		unsigned long val;
   513	
   514		for (i = 0; i < ICMPMSG_MIB_MAX; i++) {
   515			val = atomic_long_read(smib + i);
   516			if (val) {
   517				snprintf(name, sizeof(name), "Icmp%sType%u",
   518					 i & 0x100 ? "Out" : "In", i & 0xff);
   519				seq_printf(seq, "%-32s\t%lu\n", name, val);
   520			}
   521		}
   522	}
   523	
   524	static int snmp_dev_seq_show(struct seq_file *seq, void *v)
   525	{
   526		struct in_device *idev = (struct in_device *)seq->private;
   527	
   528		seq_printf(seq, "%-32s\t%u\n", "ifIndex", idev->dev->ifindex);
   529	
   530		BUILD_BUG_ON(offsetof(struct ipstats_mib, mibs) != 0);
   531	
 > 532		snmp_seq_show_item(seq, (void __percpu **)idev->stats.ip, NULL,
   533				   snmp4_ipstats_list, "Ip");
   534		snmp_seq_show_item(seq, (void __percpu **)idev->stats.ip, NULL,
   535				   snmp4_ipextstats_list, "Ip");
   536		snmp_seq_show_item(seq, NULL, idev->stats.icmpdev->mibs,
   537				   snmp4_icmp_list, "Icmp");
   538		snmp_seq_show_icmpmsg(seq, idev->stats.icmpmsgdev->mibs);
   539		return 0;
   540	}
   541	

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

^ permalink raw reply

* Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
From: Phil Elwell @ 2018-04-12 14:10 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <20180412140409.GK28963@lunn.ch>

Hi Andrew,

On 12/04/2018 15:04, Andrew Lunn wrote:
> On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote:
>> The Microchip LAN78XX family of devices are Ethernet controllers with
>> a USB interface. Despite being discoverable devices it can be useful to
>> be able to configure them from Device Tree, particularly in low-cost
>> applications without an EEPROM or programmed OTP.
>>
>> Document the supported properties in a bindings file, adding it to
>> MAINTAINERS at the same time.
> 
> Hi Phil
> 
> How you link an OF node to a USB device is not obvious. Could you
> please include either a pointer to some binding documentation, or make
> your example show it.

Thanks for the feedback. Would you consider this (lifted from the Pi 3B+ Device Tree)
a sufficient example?

&usb {
	usb1@1 {
		compatible = "usb424,2514";
		reg = <1>;
		#address-cells = <1>;
		#size-cells = <0>;

		usb1_1@1 {
			compatible = "usb424,2514";
			reg = <1>;
			#address-cells = <1>;
			#size-cells = <0>;

			ethernet: usbether@1 {
				compatible = "usb424,7800";
				reg = <1>;
				microchip,eee-enabled;
				microchip,tx-lpi-timer = <600>; /* non-aggressive*/
				/*
				 * led0 = 1:link1000/activity
				 * led1 = 6:link10/100/activity
				 */
				microchip,led-modes = <1 6>;
			};
		};
	};
};

Phil

^ permalink raw reply

* Re: [RFC PATCH v2 00/14] Introducing AF_XDP support
From: Björn Töpel @ 2018-04-12 14:14 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: William Tu, Karlsson, Magnus, Alexander Duyck, Alexander Duyck,
	John Fastabend, Jesper Dangaard Brouer, Willem de Bruijn,
	Daniel Borkmann, Linux Kernel Network Developers,
	Björn Töpel, michael.lundkvist, Brandeburg, Jesse,
	Anjali Singhai Jain, Zhang, Qi Z, ravineet.singh
In-Reply-To: <bcfd90dd-4977-4f97-daec-c7128bcdb67e@fb.com>

2018-04-11 20:43 GMT+02:00 Alexei Starovoitov <ast@fb.com>:
> On 4/11/18 5:17 AM, Björn Töpel wrote:
>>
>>
>> In the current RFC you are required to create both an Rx and Tx
>> queue to bind the socket, which is just weird for your "Rx on one
>> device, Tx to another" scenario. I'll fix that in the next RFC.
>
> I would defer on adding new features until the key functionality
> lands.  imo it's in good shape and I would submit it without RFC tag
> as soon as net-next reopens.

Yes, makes sense. We're doing some ptr_ring-like vs head/tail
measurements, and depending on the result we'll send out a proper
patch when net-next is open again.

What tree should we target -- bpf-next or net-next?


Thanks!
Björn

^ permalink raw reply

* Re: [PATCH 2/4] lan78xx: Read initial EEE setting from Device Tree
From: Andrew Lunn @ 2018-04-12 14:16 UTC (permalink / raw)
  To: Phil Elwell
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <1523541336-145953-3-git-send-email-phil@raspberrypi.org>

On Thu, Apr 12, 2018 at 02:55:34PM +0100, Phil Elwell wrote:
> Add two new Device Tree properties:
> * microchip,eee-enabled  - a boolean to enable EEE
> * microchip,tx-lpi-timer - time in microseconds to wait after TX goes
>                            idle before entering the low power state
>                            (default 600)

Hi Phil

This looks wrong.

What should happen is that the MAC driver calls phy_init_eee() to find
out if the PHY supports EEE. There should be no need to look in device
tree.

	Andrew

^ permalink raw reply

* Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
From: Andrew Lunn @ 2018-04-12 14:17 UTC (permalink / raw)
  To: Phil Elwell
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <0bcc10cd-2a25-311f-d006-ddefe47567dc@raspberrypi.org>

On Thu, Apr 12, 2018 at 03:10:57PM +0100, Phil Elwell wrote:
> Hi Andrew,
> 
> On 12/04/2018 15:04, Andrew Lunn wrote:
> > On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote:
> >> The Microchip LAN78XX family of devices are Ethernet controllers with
> >> a USB interface. Despite being discoverable devices it can be useful to
> >> be able to configure them from Device Tree, particularly in low-cost
> >> applications without an EEPROM or programmed OTP.
> >>
> >> Document the supported properties in a bindings file, adding it to
> >> MAINTAINERS at the same time.
> > 
> > Hi Phil
> > 
> > How you link an OF node to a USB device is not obvious. Could you
> > please include either a pointer to some binding documentation, or make
> > your example show it.
> 
> Thanks for the feedback. Would you consider this (lifted from the Pi 3B+ Device Tree)
> a sufficient example?

Yes, this is good.

Thanks
     Andrew

^ permalink raw reply

* Re: [PATCH 1/4] lan78xx: Read MAC address from DT if present
From: Phil Elwell @ 2018-04-12 14:18 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <20180412140640.GL28963@lunn.ch>

Hi Andrew,

On 12/04/2018 15:06, Andrew Lunn wrote:
> Hi Phil
> 
>> -			ret = lan78xx_write_reg(dev, RX_ADDRL, addr_lo);
>> -			ret = lan78xx_write_reg(dev, RX_ADDRH, addr_hi);
>> +		mac_addr = of_get_mac_address(dev->udev->dev.of_node);
> 
> It might be better to use the higher level eth_platform_get_mac_address().

OK - I'll take your word for it.

Phil

^ permalink raw reply

* [PATCH] net: ethernet: ti: cpsw: fix tx vlan priority mapping
From: Ivan Khoronzhuk @ 2018-04-12 14:25 UTC (permalink / raw)
  To: grygorii.strashko
  Cc: davem, linux-omap, netdev, linux-kernel, Ivan Khoronzhuk

The CPDMA_TX_PRIORITY_MAP in real is vlan pcp field priority mapping
register and basically replaces vlan pcp field for tagged packets.
So, set it to be 1:1 mapping.

Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
Based on net/master

 drivers/net/ethernet/ti/cpsw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 3037127..74f8284 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -129,7 +129,7 @@ do {								\
 
 #define RX_PRIORITY_MAPPING	0x76543210
 #define TX_PRIORITY_MAPPING	0x33221100
-#define CPDMA_TX_PRIORITY_MAP	0x01234567
+#define CPDMA_TX_PRIORITY_MAP	0x76543210
 
 #define CPSW_VLAN_AWARE		BIT(1)
 #define CPSW_RX_VLAN_ENCAP	BIT(2)
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH 3/4] lan78xx: Read LED modes from Device Tree
From: Andrew Lunn @ 2018-04-12 14:26 UTC (permalink / raw)
  To: Phil Elwell
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <1523541336-145953-4-git-send-email-phil@raspberrypi.org>

On Thu, Apr 12, 2018 at 02:55:35PM +0100, Phil Elwell wrote:
> Add support for DT property "microchip,led-modes", a vector of two
> cells (u32s) in the range 0-15, each of which sets the mode for one
> of the two LEDs. Some possible values are:
> 
>     0=link/activity          1=link1000/activity
>     2=link100/activity       3=link10/activity
>     4=link100/1000/activity  5=link10/1000/activity
>     6=link10/100/activity    14=off    15=on
> 
> Also use the presence of the DT property to indicate that the
> LEDs should be enabled - necessary in the event that no valid OTP
> or EEPROM is available.

I'm not a fan of this, but at the moment, we don't have anything
better.

Please follow what mscc does, add a header file for the LED settings.

       Andrew

> 
> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> ---
>  drivers/net/usb/lan78xx.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index d98397b..ffb483d 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -2008,6 +2008,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
>  {
>  	int ret;
>  	u32 mii_adv;
> +	u32 led_modes[2];
>  	struct phy_device *phydev;
>  
>  	phydev = phy_find_first(dev->mdiobus);
> @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
>  		(void)lan78xx_set_eee(dev->net, &edata);
>  	}
>  
> +	if (!of_property_read_u32_array(dev->udev->dev.of_node,
> +					"microchip,led-modes",
> +					led_modes, ARRAY_SIZE(led_modes))) {
> +		u32 reg;
> +		int i;
> +
> +		reg = phy_read(phydev, 0x1d);
> +		for (i = 0; i < ARRAY_SIZE(led_modes); i++) {
> +			reg &= ~(0xf << (i * 4));
> +			reg |= (led_modes[i] & 0xf) << (i * 4);
> +		}

Please add range checks for led_modes[i] and return -EINVAL if the
check fails.

      Andrew

^ permalink raw reply

* Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
From: Andrew Lunn @ 2018-04-12 14:30 UTC (permalink / raw)
  To: Phil Elwell
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <1523541336-145953-5-git-send-email-phil@raspberrypi.org>

On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote:
> The Microchip LAN78XX family of devices are Ethernet controllers with
> a USB interface. Despite being discoverable devices it can be useful to
> be able to configure them from Device Tree, particularly in low-cost
> applications without an EEPROM or programmed OTP.

It would be good to document what happens when there is an EEPROM. Is
OF used in preference to the EEPROM?
   
Andrew

^ permalink raw reply

* Re: [PATCH 3/4] lan78xx: Read LED modes from Device Tree
From: Phil Elwell @ 2018-04-12 14:30 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <20180412142615.GO28963@lunn.ch>

Hi Andrew,

On 12/04/2018 15:26, Andrew Lunn wrote:
> On Thu, Apr 12, 2018 at 02:55:35PM +0100, Phil Elwell wrote:
>> Add support for DT property "microchip,led-modes", a vector of two
>> cells (u32s) in the range 0-15, each of which sets the mode for one
>> of the two LEDs. Some possible values are:
>>
>>     0=link/activity          1=link1000/activity
>>     2=link100/activity       3=link10/activity
>>     4=link100/1000/activity  5=link10/1000/activity
>>     6=link10/100/activity    14=off    15=on
>>
>> Also use the presence of the DT property to indicate that the
>> LEDs should be enabled - necessary in the event that no valid OTP
>> or EEPROM is available.
> 
> I'm not a fan of this, but at the moment, we don't have anything
> better.
> 
> Please follow what mscc does, add a header file for the LED settings.

Good idea.

> 
>>
>> Signed-off-by: Phil Elwell <phil@raspberrypi.org>
>> ---
>>  drivers/net/usb/lan78xx.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
>> index d98397b..ffb483d 100644
>> --- a/drivers/net/usb/lan78xx.c
>> +++ b/drivers/net/usb/lan78xx.c
>> @@ -2008,6 +2008,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
>>  {
>>  	int ret;
>>  	u32 mii_adv;
>> +	u32 led_modes[2];
>>  	struct phy_device *phydev;
>>  
>>  	phydev = phy_find_first(dev->mdiobus);
>> @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
>>  		(void)lan78xx_set_eee(dev->net, &edata);
>>  	}
>>  
>> +	if (!of_property_read_u32_array(dev->udev->dev.of_node,
>> +					"microchip,led-modes",
>> +					led_modes, ARRAY_SIZE(led_modes))) {
>> +		u32 reg;
>> +		int i;
>> +
>> +		reg = phy_read(phydev, 0x1d);
>> +		for (i = 0; i < ARRAY_SIZE(led_modes); i++) {
>> +			reg &= ~(0xf << (i * 4));
>> +			reg |= (led_modes[i] & 0xf) << (i * 4);
>> +		}
> 
> Please add range checks for led_modes[i] and return -EINVAL if the
> check fails.

Will do.

Thanks,

Phil

^ permalink raw reply

* Re: [PATCH 4/4] dt-bindings: Document the DT bindings for lan78xx
From: Phil Elwell @ 2018-04-12 14:33 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <20180412143007.GP28963@lunn.ch>

Hi Andrew,

On 12/04/2018 15:30, Andrew Lunn wrote:
> On Thu, Apr 12, 2018 at 02:55:36PM +0100, Phil Elwell wrote:
>> The Microchip LAN78XX family of devices are Ethernet controllers with
>> a USB interface. Despite being discoverable devices it can be useful to
>> be able to configure them from Device Tree, particularly in low-cost
>> applications without an EEPROM or programmed OTP.
> 
> It would be good to document what happens when there is an EEPROM. Is
> OF used in preference to the EEPROM?

Yes it is. I'll mention it in V2.

Phil

^ permalink raw reply

* Re: [PATCH 3/4] lan78xx: Read LED modes from Device Tree
From: Andrew Lunn @ 2018-04-12 14:36 UTC (permalink / raw)
  To: Phil Elwell
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <1523541336-145953-4-git-send-email-phil@raspberrypi.org>

> @@ -2097,6 +2098,25 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
>  		(void)lan78xx_set_eee(dev->net, &edata);
>  	}
>  
> +	if (!of_property_read_u32_array(dev->udev->dev.of_node,
> +					"microchip,led-modes",
> +					led_modes, ARRAY_SIZE(led_modes))) {
> +		u32 reg;
> +		int i;
> +
> +		reg = phy_read(phydev, 0x1d);
> +		for (i = 0; i < ARRAY_SIZE(led_modes); i++) {
> +			reg &= ~(0xf << (i * 4));
> +			reg |= (led_modes[i] & 0xf) << (i * 4);
> +		}
> +		(void)phy_write(phydev, 0x1d, reg);

Poking PHY registers directly from the MAC driver is not always a good
idea. This MAC driver does that in a few places :-(

What do we know about the PHY? It is built into the device or is it
external? If it is external, how do you know the LED register is at
0x1d?

The safest place to do this is in the PHY driver, and place these OF
properties into the PHY node.

	   Andrew

^ permalink raw reply

* Re: XDP performance regression due to CONFIG_RETPOLINE Spectre V2
From: Christoph Hellwig @ 2018-04-12 14:51 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: xdp-newbies@vger.kernel.org, netdev@vger.kernel.org,
	Christoph Hellwig, David Woodhouse, William Tu,
	Björn Töpel, Karlsson, Magnus, Alexander Duyck,
	Arnaldo Carvalho de Melo
In-Reply-To: <20180412155029.0324fe58@redhat.com>

On Thu, Apr 12, 2018 at 03:50:29PM +0200, Jesper Dangaard Brouer wrote:
> ---------------
> Implement support for keeping the DMA mapping through the XDP return
> call, to remove RX map/unmap calls.  Implement bulking for XDP
> ndo_xdp_xmit and XDP return frame API.  Bulking allows to perform DMA
> bulking via scatter-gatter DMA calls, XDP TX need it for DMA
> map+unmap. The driver RX DMA-sync (to CPU) per packet calls are harder
> to mitigate (via bulk technique). Ask DMA maintainer for a common
> case direct call for swiotlb DMA sync call ;-)

Why do you even end up in swiotlb code?  Once you bounce buffer your
performance is toast anyway..

^ permalink raw reply

* Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device
From: Maxime Ripard @ 2018-04-12 14:56 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Rob Herring, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20180411141641.14675-4-icenowy-h8G6r0blFSE@public.gmane.org>

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

On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote:
> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> 
> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU
> address space; on the A64 SoC this register is in the SRAM controller
> address space, and with a different offset.
> 
> To access the register from another device and hide the internal
> difference between the device, let it register a regmap named
> "emac-clock". We can then get the device from the phandle, and
> retrieve the regmap with dev_get_regmap(); in this situation the
> regmap_field will be set up to access the only register in the regmap.
> 
> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> [Icenowy: change to use regmaps with single register, change commit
>  message]
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48 ++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 1037f6c78bca..b61210c0d415 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = {
>  	.msb = 31,
>  };
>  
> +/* Specially exported regmap which contains only EMAC register */
> +const struct reg_field single_reg_field = {
> +	.reg = 0,
> +	.lsb = 0,
> +	.msb = 31,
> +};
> +

I'm not sure this would be wise. If we ever need some other register
exported through the regmap, will have to change all the calling sites
everywhere in the kernel, which will be a pain and will break
bisectability.

Chen-Yu's (or was it yours?) initial solution with a custom writeable
hook only allowing a single register seemed like a better one.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: XDP performance regression due to CONFIG_RETPOLINE Spectre V2
From: Christoph Hellwig @ 2018-04-12 14:56 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: xdp-newbies@vger.kernel.org, netdev@vger.kernel.org,
	Christoph Hellwig, David Woodhouse, William Tu,
	Björn Töpel, Karlsson, Magnus, Alexander Duyck,
	Arnaldo Carvalho de Melo
In-Reply-To: <20180412145123.GA7048@lst.de>

On Thu, Apr 12, 2018 at 04:51:23PM +0200, Christoph Hellwig wrote:
> On Thu, Apr 12, 2018 at 03:50:29PM +0200, Jesper Dangaard Brouer wrote:
> > ---------------
> > Implement support for keeping the DMA mapping through the XDP return
> > call, to remove RX map/unmap calls.  Implement bulking for XDP
> > ndo_xdp_xmit and XDP return frame API.  Bulking allows to perform DMA
> > bulking via scatter-gatter DMA calls, XDP TX need it for DMA
> > map+unmap. The driver RX DMA-sync (to CPU) per packet calls are harder
> > to mitigate (via bulk technique). Ask DMA maintainer for a common
> > case direct call for swiotlb DMA sync call ;-)
> 
> Why do you even end up in swiotlb code?  Once you bounce buffer your
> performance is toast anyway..

I guess that is because x86 selects it as the default as soon as
we have more than 4G memory. That should be solveable fairly easily
with the per-device dma ops, though.

^ permalink raw reply

* Re: [PATCH 5/5] arm64: allwinner: a64: add SRAM controller device tree node
From: Maxime Ripard @ 2018-04-12 14:58 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Rob Herring, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20180411141641.14675-6-icenowy-h8G6r0blFSE@public.gmane.org>

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

Hi,

On Wed, Apr 11, 2018 at 10:16:41PM +0800, Icenowy Zheng wrote:
> Allwinner A64 has a SRAM controller, and in the device tree currently
> we have a syscon node to enable EMAC driver to access the EMAC clock
> register. As SRAM controller driver can now export regmap for this
> register, replace the syscon node to the SRAM controller device node,
> and let EMAC driver to acquire its EMAC clock regmap.
> 
> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index 1b2ef28c42bd..1c37659d9d41 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -168,10 +168,25 @@
>  		#size-cells = <1>;
>  		ranges;
>  
> -		syscon: syscon@1c00000 {
> -			compatible = "allwinner,sun50i-a64-system-controller",
> -				"syscon";
> +		sram_controller: sram-controller@1c00000 {
> +			compatible = "allwinner,sun50i-a64-sram-controller";
>  			reg = <0x01c00000 0x1000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			sram_c: sram@18000 {
> +				compatible = "mmio-sram";
> +				reg = <0x00018000 0x28000>;
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				ranges = <0 0x00018000 0x28000>;
> +
> +				de2_sram: sram-section@0 {
> +					compatible = "allwinner,sun50i-a64-sram-c";
> +					reg = <0x0000 0x28000>;
> +				};
> +			};

That doesn't look related at all to what's being discussed here, so
you'd rather add it as part of your DE2-enablement serie (or amend
your commit log to say why this is important to do it in this patch).

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [RFC v3 net-next 13/18] net/sched: Introduce the TBS Qdisc
From: Richard Cochran @ 2018-04-12 15:03 UTC (permalink / raw)
  To: Jesus Sanchez-Palencia
  Cc: Thomas Gleixner, netdev, jhs, xiyou.wangcong, jiri,
	vinicius.gomes, anna-maria, henrik, John Stultz, levi.pearson,
	edumazet, willemb, mlichvar
In-Reply-To: <3d1c27a2-7a21-ab80-e92b-47b415b70548@intel.com>

On Wed, Apr 11, 2018 at 04:38:44PM -0700, Jesus Sanchez-Palencia wrote:
> Just breaking this down a bit, yes, TAI is the network time base, and the NICs
> PTP clock use that because PTP is (commonly) based on TAI. After the PHCs have
> been synchronized over the network (e.g. with ptp4l), my understanding is that
> if applications want to use the clockid_t CLOCK_TAI as a network clock reference
> it's required that something (i.e. phc2sys) is synchronizing the PHCs and the
> system clock, and also that something calls adjtime to apply the TAI vs UTC
> offset to CLOCK_TAI.

Yes.  I haven't seen any distro that sets the TAI-UTC offset after
boot, nor are there any user space tools for this.  The kernel is
ready, though.

> I was thinking about the full offload use-cases, thus when no scheduling is
> happening inside the qdiscs. Applications could just read the time from the PHC
> clocks directly without having to rely on any of the above. On this case,
> userspace would use DYNAMIC_CLOCK just to flag that this is the case, but I must
> admit it's not clear to me how common of a use-case that is, or even if it makes
> sense.

1588 allows only two timescales, TAI and ARB-itrary.  Although it
doesn't make too much sense to use ARB, still people will do strange
things.  Probably some people use UTC.  I am not advocating supporting
alternate timescales, just pointing out the possibility.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH 3/5] net: stmmac: dwmac-sun8i: Allow getting syscon regmap from device
From: Icenowy Zheng @ 2018-04-12 15:11 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Rob Herring, Chen-Yu Tsai, Giuseppe Cavallaro, Corentin Labbe,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20180412145628.iaaeue2imiijwx5d@flea>



于 2018年4月12日 GMT+08:00 下午10:56:28, Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> 写到:
>On Wed, Apr 11, 2018 at 10:16:39PM +0800, Icenowy Zheng wrote:
>> From: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> 
>> On the Allwinner R40 SoC, the "GMAC clock" register is in the CCU
>> address space; on the A64 SoC this register is in the SRAM controller
>> address space, and with a different offset.
>> 
>> To access the register from another device and hide the internal
>> difference between the device, let it register a regmap named
>> "emac-clock". We can then get the device from the phandle, and
>> retrieve the regmap with dev_get_regmap(); in this situation the
>> regmap_field will be set up to access the only register in the
>regmap.
>> 
>> Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
>> [Icenowy: change to use regmaps with single register, change commit
>>  message]
>> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 48
>++++++++++++++++++++++-
>>  1 file changed, 46 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> index 1037f6c78bca..b61210c0d415 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> @@ -85,6 +85,13 @@ const struct reg_field old_syscon_reg_field = {
>>  	.msb = 31,
>>  };
>>  
>> +/* Specially exported regmap which contains only EMAC register */
>> +const struct reg_field single_reg_field = {
>> +	.reg = 0,
>> +	.lsb = 0,
>> +	.msb = 31,
>> +};
>> +
>
>I'm not sure this would be wise. If we ever need some other register
>exported through the regmap, will have to change all the calling sites
>everywhere in the kernel, which will be a pain and will break
>bisectability.

In this situation the register can be exported as another
 regmap. Currently the code will access a regmap with name
"emac-clock" for this register.

>
>Chen-Yu's (or was it yours?) initial solution with a custom writeable
>hook only allowing a single register seemed like a better one.

But I remember you mentioned that you want it to hide the
difference inside the device.

>
>Maxime

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH net 2/2] sfc: limit ARFS workitems in flight per channel
From: David Miller @ 2018-04-12 15:11 UTC (permalink / raw)
  To: ecree; +Cc: linux-net-drivers, netdev
In-Reply-To: <d3bd9e31-5247-8032-8f35-c85d3728c32e@solarflare.com>

From: Edward Cree <ecree@solarflare.com>
Date: Thu, 12 Apr 2018 15:02:50 +0100

> A misconfigured system (e.g. with all interrupts affinitised to all CPUs)
>  may produce a storm of ARFS steering events.  With the existing sfc ARFS
>  implementation, that could create a backlog of workitems that grinds the
>  system to a halt.  To prevent this, limit the number of workitems that
>  may be in flight for a given SFC device to 8 (EFX_RPS_MAX_IN_FLIGHT), and
>  return EBUSY from our ndo_rx_flow_steer method if the limit is reached.
> Given this limit, also store the workitems in an array of slots within the
>  struct efx_nic, rather than dynamically allocating for each request.
> 
> Signed-off-by: Edward Cree <ecree@solarflare.com>

I don't think this behavior is all that great.

If you really have to queue up these operations because they take a long
time, I think it is better to enter a synchronous mode and sleep once
you hit this in-flight limit of 8.

Either that or make the expiration work smarter when it has lots of events
to process.

^ permalink raw reply

* Re: [PATCH 2/4] lan78xx: Read initial EEE setting from Device Tree
From: Phil Elwell @ 2018-04-12 15:17 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Woojung Huh, Microchip Linux Driver Support, Rob Herring,
	Mark Rutland, David S. Miller, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Walleij, Andrew Morton, Randy Dunlap,
	netdev, devicetree, linux-kernel, linux-usb
In-Reply-To: <20180412141630.GM28963@lunn.ch>

Andrew,

On 12/04/2018 15:16, Andrew Lunn wrote:
> On Thu, Apr 12, 2018 at 02:55:34PM +0100, Phil Elwell wrote:
>> Add two new Device Tree properties:
>> * microchip,eee-enabled  - a boolean to enable EEE
>> * microchip,tx-lpi-timer - time in microseconds to wait after TX goes
>>                            idle before entering the low power state
>>                            (default 600)
> 
> Hi Phil
> 
> This looks wrong.
> 
> What should happen is that the MAC driver calls phy_init_eee() to find
> out if the PHY supports EEE. There should be no need to look in device
> tree.

If the driver should be calling phy_init_eee to initialise EEE operation then I'm fine
with that (although I notice that the TI cpsw calls phy_ethtool_set_eee but I don't see
it calling phy_init_eee). However, it sounds like I need to keep my DT toggle of the
EEE enablement and parameters downstream.

Phil

^ permalink raw reply

* Re: [RFC v3 net-next 13/18] net/sched: Introduce the TBS Qdisc
From: Miroslav Lichvar @ 2018-04-12 15:19 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Jesus Sanchez-Palencia, Thomas Gleixner, netdev, jhs,
	xiyou.wangcong, jiri, vinicius.gomes, anna-maria, henrik,
	John Stultz, levi.pearson, edumazet, willemb
In-Reply-To: <20180412150349.3dgkgqx4ged6t4ng@localhost>

On Thu, Apr 12, 2018 at 08:03:49AM -0700, Richard Cochran wrote:
> On Wed, Apr 11, 2018 at 04:38:44PM -0700, Jesus Sanchez-Palencia wrote:
> > Just breaking this down a bit, yes, TAI is the network time base, and the NICs
> > PTP clock use that because PTP is (commonly) based on TAI. After the PHCs have
> > been synchronized over the network (e.g. with ptp4l), my understanding is that
> > if applications want to use the clockid_t CLOCK_TAI as a network clock reference
> > it's required that something (i.e. phc2sys) is synchronizing the PHCs and the
> > system clock, and also that something calls adjtime to apply the TAI vs UTC
> > offset to CLOCK_TAI.
> 
> Yes.  I haven't seen any distro that sets the TAI-UTC offset after
> boot, nor are there any user space tools for this.  The kernel is
> ready, though.

FWIW, the default NTP configuration in Fedora sets the kernel TAI-UTC
offset.

> > I was thinking about the full offload use-cases, thus when no scheduling is
> > happening inside the qdiscs. Applications could just read the time from the PHC
> > clocks directly without having to rely on any of the above. On this case,
> > userspace would use DYNAMIC_CLOCK just to flag that this is the case, but I must
> > admit it's not clear to me how common of a use-case that is, or even if it makes
> > sense.
> 
> 1588 allows only two timescales, TAI and ARB-itrary.  Although it
> doesn't make too much sense to use ARB, still people will do strange
> things.  Probably some people use UTC.  I am not advocating supporting
> alternate timescales, just pointing out the possibility.

There is also the possibility that the NIC clock is not synchronized
to anything. For synchronization of the system clock it's easier to
leave it free running and only track its phase/frequency offset to
allow conversion between the PHC and system time.

-- 
Miroslav Lichvar

^ permalink raw reply

* RE: [RFC PATCH v2 03/14] xsk: add umem fill queue support and mmap
From: Karlsson, Magnus @ 2018-04-12 15:19 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Björn Töpel, Duyck, Alexander H,
	alexander.duyck@gmail.com, john.fastabend@gmail.com, ast@fb.com,
	brouer@redhat.com, willemdebruijn.kernel@gmail.com,
	daniel@iogearbox.net, netdev@vger.kernel.org,
	michael.lundkvist@ericsson.com, Brandeburg, Jesse,
	Singhai, Anjali, Zhang, Qi Z, ravineet.singh@ericsson.com
In-Reply-To: <20180412170110-mutt-send-email-mst@kernel.org>



> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Thursday, April 12, 2018 4:05 PM
> To: Karlsson, Magnus <magnus.karlsson@intel.com>
> Cc: Björn Töpel <bjorn.topel@gmail.com>; Duyck, Alexander H
> <alexander.h.duyck@intel.com>; alexander.duyck@gmail.com;
> john.fastabend@gmail.com; ast@fb.com; brouer@redhat.com;
> willemdebruijn.kernel@gmail.com; daniel@iogearbox.net;
> netdev@vger.kernel.org; michael.lundkvist@ericsson.com; Brandeburg,
> Jesse <jesse.brandeburg@intel.com>; Singhai, Anjali
> <anjali.singhai@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> ravineet.singh@ericsson.com
> Subject: Re: [RFC PATCH v2 03/14] xsk: add umem fill queue support and
> mmap
> 
> On Thu, Apr 12, 2018 at 07:38:25AM +0000, Karlsson, Magnus wrote:
> > I think you are definitely right in that there are ways in which we
> > can improve performance here. That said, the current queue performs
> > slightly better than the previous one we had that was more or less a
> > copy of one of your first virtio 1.1 proposals from little over a year
> > ago. It had bidirectional queues and a valid flag in the descriptor
> > itself. The reason we abandoned this was not poor performance (it was
> > good), but a need to go to unidirectional queues. Maybe I should have
> > only changed that aspect and kept the valid flag.
> 
> Is there a summary about unidirectional queues anywhere?  I'm curious to
> know whether there are any lessons here to be learned for virtio or ptr_ring.

I did a quick hack in which I used your ptr_ring for the fill queue instead of
our head/tail based one. In the corner cases (usually empty or usually full), there
is basically no difference. But for the case when the queue is always half full,
the ptr_ring implementation boosts the performance from 5.6 to 5.7 Mpps 
(as there is no cache line bouncing in this case) 
on my system (slower than Björn's that was used for the numbers in the RFC).

So I think this should be implemented properly so we can get some real numbers.
Especially since 0.1 Mpps with copies will likely become much more with zero-copy
as we are really chasing cycles there. We will get back a better evaluation in a few
days.

Thanks: Magnus

> --
> MST

^ 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