* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Greear @ 2012-07-30 17:23 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1343668602.2667.6.camel@bwh-desktop.uk.solarflarecom.com>
On 07/30/2012 10:16 AM, Ben Hutchings wrote:
> A peer (or local user) may cause TCP to use a nominal MSS of as little
> as 88 (actual MSS of 76 with timestamps). Given that we have a
> sufficiently prodigious local sender and the peer ACKs quickly enough,
> it is nevertheless possible to grow the window for such a connection
> to the point that we will try to send just under 64K at once. This
> results in a single skb that expands to 861 segments.
>
> In some drivers with TSO support, such an skb will require hundreds of
> DMA descriptors; a substantial fraction of a TX ring or even more than
> a full ring. The TX queue selected for the skb may stall and trigger
> the TX watchdog repeatedly (since the problem skb will be retried
> after the TX reset). This particularly affects sfc, for which the
> issue is designated as CVE-2012-3412. However it may be that some
> hardware or firmware also fails to handle such an extreme TSO request
> correctly.
>
> Therefore, limit the number of segments per skb to 100. This should
> make no difference to behaviour unless the actual MSS is less than
> about 700.
Please do not do this...or at least allow over-rides. We love
the trick of seting very small MSS and making the NICs generate
huge numbers of small TCP frames with efficient user-space
logic. We use this for stateful TCP load testing when high
numbers of tcp packets-per-second is desired.
Intel NICs, including 10G, work just fine with minimal MSS
in this scenario.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Eric Dumazet @ 2012-07-30 17:31 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1343668602.2667.6.camel@bwh-desktop.uk.solarflarecom.com>
On Mon, 2012-07-30 at 18:16 +0100, Ben Hutchings wrote:
> A peer (or local user) may cause TCP to use a nominal MSS of as little
> as 88 (actual MSS of 76 with timestamps). Given that we have a
> sufficiently prodigious local sender and the peer ACKs quickly enough,
> it is nevertheless possible to grow the window for such a connection
> to the point that we will try to send just under 64K at once. This
> results in a single skb that expands to 861 segments.
>
> In some drivers with TSO support, such an skb will require hundreds of
> DMA descriptors; a substantial fraction of a TX ring or even more than
> a full ring. The TX queue selected for the skb may stall and trigger
> the TX watchdog repeatedly (since the problem skb will be retried
> after the TX reset). This particularly affects sfc, for which the
> issue is designated as CVE-2012-3412. However it may be that some
> hardware or firmware also fails to handle such an extreme TSO request
> correctly.
>
> Therefore, limit the number of segments per skb to 100. This should
> make no difference to behaviour unless the actual MSS is less than
> about 700.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
Hmm, isnt GRO path also vulnerable ?
An alternative would be to drop such frames in the ndo_start_xmit(), and
cap sk->sk_gso_max_size (since skb are no longer orphaned...)
Or you could introduce a new wk->sk_gso_max_segments, that your sfc
driver sets to whatever limit ?
^ permalink raw reply
* Re: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
From: Olaf Hering @ 2012-07-30 17:33 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, netdev, apw, devel, ben
In-Reply-To: <1343145701-3691-13-git-send-email-kys@microsoft.com>
On Tue, Jul 24, K. Y. Srinivasan wrote:
> +static char *kvp_get_if_name(char *guid)
> +{
> + DIR *dir;
> + struct dirent *entry;
> + FILE *file;
> + char *p, *q, *x;
> + char *if_name = NULL;
> + char buf[256];
> + char *kvp_net_dir = "/sys/class/net/";
> + char dev_id[100];
Perhaps this can be written as char dev_id[100] = "short string";?
> +
> + dir = opendir(kvp_net_dir);
> + if (dir == NULL)
> + return NULL;
> +
> + memset(dev_id, 0, sizeof(dev_id));
> + strcat(dev_id, kvp_net_dir);
> + q = dev_id + strlen(kvp_net_dir);
> +
> + while ((entry = readdir(dir)) != NULL) {
> + /*
> + * Set the state for the next pass.
> + */
> + *q = '\0';
> + strcat(dev_id, entry->d_name);
> + strcat(dev_id, "/device/device_id");
Maybe this (and other) strcat should be changed to snprintf?
> +
> + file = fopen(dev_id, "r");
> + if (file == NULL)
> + continue;
> +
> + p = fgets(buf, sizeof(buf), file);
> + if (p) {
> + x = strchr(p, '\n');
> + if (x)
> + *x = '\0';
> +
> + if (!strcmp(p, guid)) {
> + /*
> + * Found the guid match; return the interface
> + * name. The caller will free the memory.
> + */
> + if_name = strdup(entry->d_name);
> + break;
This will leave 'file' open.
I have seen it in some other place as well.
> + }
> + }
> + fclose(file);
> + }
> +
> + closedir(dir);
> + return if_name;
> +}
> +
> +/*
> + * Retrieve the MAC address given the interface name.
> + */
> +
> +static char *kvp_if_name_to_mac(char *if_name)
> +{
> + FILE *file;
> + char *p, *x;
> + char buf[256];
> + char addr_file[100];
> + int i;
> + char *mac_addr = NULL;
> +
> + memset(addr_file, 0, sizeof(addr_file));
> + strcat(addr_file, "/sys/class/net/");
> + strcat(addr_file, if_name);
> + strcat(addr_file, "/address");
snprintf?
> +
> + file = fopen(addr_file, "r");
> + if (file == NULL)
> + return NULL;
> +
> + p = fgets(buf, sizeof(buf), file);
> + if (p) {
> + x = strchr(p, '\n');
> + if (x)
> + *x = '\0';
> + for (i = 0; i < strlen(p); i++)
> + p[i] = toupper(p[i]);
> + mac_addr = strdup(p);
> + }
> +
> + fclose(file);
> + return mac_addr;
> +}
> +
> +
> static void kvp_process_ipconfig_file(char *cmd,
> char *config_buf, int len,
> int element_size, int offset)
> @@ -800,6 +910,314 @@ getaddr_done:
> }
>
>
> +static int expand_ipv6(char *addr, int type)
> +{
> + int ret;
> + struct in6_addr v6_addr;
> +
> + ret = inet_pton(AF_INET6, addr, &v6_addr);
> +
> + if (ret != 1) {
> + if (type == NETMASK)
> + return 1;
> + return 0;
> + }
> +
> + sprintf(addr, "%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:"
> + "%02x%02x:%02x%02x:%02x%02x",
> + (int)v6_addr.s6_addr[0], (int)v6_addr.s6_addr[1],
> + (int)v6_addr.s6_addr[2], (int)v6_addr.s6_addr[3],
> + (int)v6_addr.s6_addr[4], (int)v6_addr.s6_addr[5],
> + (int)v6_addr.s6_addr[6], (int)v6_addr.s6_addr[7],
> + (int)v6_addr.s6_addr[8], (int)v6_addr.s6_addr[9],
> + (int)v6_addr.s6_addr[10], (int)v6_addr.s6_addr[11],
> + (int)v6_addr.s6_addr[12], (int)v6_addr.s6_addr[13],
> + (int)v6_addr.s6_addr[14], (int)v6_addr.s6_addr[15]);
> +
> + return 1;
> +
> +}
> +
> +static int is_ipv4(char *addr)
> +{
> + int ret;
> + struct in_addr ipv4_addr;
> +
> + ret = inet_pton(AF_INET, addr, &ipv4_addr);
> +
> + if (ret == 1)
> + return 1;
> + return 0;
> +}
> +
> +static int parse_ip_val_buffer(char *in_buf, int *offset,
> + char *out_buf, int out_len)
> +{
> + char *x;
> + char *start;
> +
> + /*
> + * in_buf has sequence of characters that are seperated by
> + * the character ';'. The last sequence does not have the
> + * terminating ";" character.
> + */
> + start = in_buf + *offset;
> +
> + x = strchr(start, ';');
> + if (x)
> + *x = 0;
> + else
> + x = start + strlen(start);
> +
> + if (strlen(start) != 0) {
> + int i = 0;
> + /*
> + * Get rid of leading spaces.
> + */
> + while (start[i] == ' ')
> + i++;
> +
> + if ((x - start) <= out_len) {
> + strcpy(out_buf, (start + i));
> + *offset += (x - start) + 1;
> + return 1;
> + }
> + }
> + return 0;
> +}
> +
> +static int kvp_write_file(FILE *f, char *s1, char *s2, char *s3)
> +{
> + char str[256];
> + int error;
> +
> + memset(str, 0, sizeof(str));
> + strcat(str, s1);
> + if (s2 != NULL)
> + strcat(str, s2);
> + strcat(str, "=");
> + strcat(str, s3);
> + strcat(str, "\n");
> +
> + error = fputs(str, f);
> + if (error == EOF)
> + return HV_E_FAIL;
> +
> + return 0;
> +}
> +
> +
> +static int process_ip_string(FILE *f, char *ip_string, int type)
> +{
> + int error = 0;
> + char addr[INET6_ADDRSTRLEN];
> + int i = 0;
> + int j = 0;
> + char str[256];
> + char sub_str[10];
> + int offset = 0;
> +
> + memset(addr, 0, sizeof(addr));
> +
> + while (parse_ip_val_buffer(ip_string, &offset, addr,
> + (MAX_IP_ADDR_SIZE * 2))) {
> + memset(sub_str, 0, sizeof(sub_str));
> + memset(str, 0, sizeof(str));
> +
> + if (is_ipv4(addr)) {
> + switch (type) {
> + case IPADDR:
> + strcat(str, "IPADDR");
> + break;
> + case NETMASK:
> + strcat(str, "NETMASK");
> + break;
> + case GATEWAY:
> + strcat(str, "GATEWAY");
> + break;
> + case DNS:
> + strcat(str, "DNS");
> + break;
> + }
> + if (i != 0) {
> + if (type != DNS)
> + sprintf(sub_str, "_%d", i++);
> + else
> + sprintf(sub_str, "%d", ++i);
> + } else if (type == DNS) {
> + sprintf(sub_str, "%d", ++i);
> + }
> +
> +
> + } else if (expand_ipv6(addr, type)) {
> + switch (type) {
> + case IPADDR:
> + strcat(str, "IPV6ADDR");
> + break;
> + case NETMASK:
> + strcat(str, "IPV6NETMASK");
> + break;
> + case GATEWAY:
> + strcat(str, "IPV6_DEFAULTGW");
> + break;
> + case DNS:
> + strcat(str, "DNS");
> + break;
> + }
> + if ((j != 0) || (type == DNS)) {
> + if (type != DNS)
> + sprintf(sub_str, "_%d", j++);
> + else
> + sprintf(sub_str, "%d", ++i);
> + } else if (type == DNS) {
> + sprintf(sub_str, "%d", ++i);
> + }
> + } else {
> + return HV_INVALIDARG;
> + }
> +
> + error = kvp_write_file(f, str, sub_str, addr);
> + if (error)
> + return error;
> + memset(addr, 0, sizeof(addr));
> + }
> +
> + return 0;
> +}
> +
> +static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value *new_val)
> +{
> + int error = 0;
> + char if_file[50];
> + FILE *file;
> + char cmd[512];
> + char *mac_addr;
> +
> + /*
> + * Set the configuration for the specified interface with
> + * the information provided. Since there is no standard
> + * way to configure an interface, we will have an external
> + * script that does the job of configuring the interface and
> + * flushing the configuration.
> + *
> + * The parameters passed to this external script are:
> + * 1. A configuration file that has the specified configuration.
> + *
> + * We will embed the name of the interface in the configuration
> + * file: ifcfg-ethx (where ethx is the interface name).
> + *
> + * Here is the format of the ip configuration file:
> + *
> + * HWADDR=macaddr
> + * BOOTPROTO=dhcp (dhcp enabled for the interface)
> + * NM_CONTROLLED=no (this interface will not be controlled by NM)
> + * PEERDNS=yes
> + * IPADDR_x=ipaddr
> + * NETMASK_x=netmask
> + * GATEWAY_x=gateway
> + * DNSx=dns
> + *
> + * IPV6 addresses will be tagged as IPV6ADDR, IPV6 gateway will be
> + * tagged as IPV6_DEFAULTGW and IPV6 NETMASK will be tagged as
> + * IPV6NETMASK.
> + */
> +
> + memset(if_file, 0, sizeof(if_file));
> + strcat(if_file, "/var/opt/hyperv/ifcfg-");
> + strcat(if_file, if_name);
> +
> + file = fopen(if_file, "w");
> +
> + if (file == NULL) {
> + syslog(LOG_ERR, "Failed to open config file");
> + return HV_E_FAIL;
> + }
> +
> + /*
> + * First write out the MAC address.
> + */
> +
> + mac_addr = kvp_if_name_to_mac(if_name);
> + if (mac_addr == NULL) {
> + error = HV_E_FAIL;
> + goto setval_error;
> + }
> +
> + error = kvp_write_file(file, "HWADDR", NULL, mac_addr);
> + if (error)
> + goto setval_error;
> +
> + error = kvp_write_file(file, "ONBOOT", NULL, "yes");
> + if (error)
> + goto setval_error;
> +
> + error = kvp_write_file(file, "IPV6INIT", NULL, "yes");
> + if (error)
> + goto setval_error;
> +
> + error = kvp_write_file(file, "NM_CONTROLLED", NULL, "no");
> + if (error)
> + goto setval_error;
> +
> + error = kvp_write_file(file, "PEERDNS", NULL, "yes");
> + if (error)
> + goto setval_error;
> +
> + if (new_val->dhcp_enabled) {
> + error = kvp_write_file(file, "BOOTPROTO", NULL, "dhcp");
> + if (error)
> + goto setval_error;
> +
> + /*
> + * We are done!.
> + */
> + goto setval_done;
> + }
> +
> + /*
> + * Write the configuration for ipaddress, netmask, gateway and
> + * name servers.
> + */
> +
> + error = process_ip_string(file, (char *)new_val->ip_addr, IPADDR);
> + if (error)
> + goto setval_error;
> +
> + error = process_ip_string(file, (char *)new_val->sub_net, NETMASK);
> + if (error)
> + goto setval_error;
> +
> + error = process_ip_string(file, (char *)new_val->gate_way, GATEWAY);
> + if (error)
> + goto setval_error;
> +
> + error = process_ip_string(file, (char *)new_val->dns_addr, DNS);
> + if (error)
> + goto setval_error;
> +
> +setval_done:
> + free(mac_addr);
> + fclose(file);
> +
> + /*
> + * Now that we have populated the configuration file,
> + * invoke the external script to do its magic.
> + */
> +
> + memset(cmd, 0, sizeof(cmd));
> + strcat(cmd, "/sbin/hv_set_ifconfig ");
> + strcat(cmd, if_file);
The new patch should use "%s %s", not "%s%s" as format string.
^ permalink raw reply
* Re: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
From: Olaf Hering @ 2012-07-30 18:03 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, apw, netdev, ben
In-Reply-To: <1343145701-3691-13-git-send-email-kys@microsoft.com>
On Tue, Jul 24, K. Y. Srinivasan wrote:
> + /*
> + * Set the configuration for the specified interface with
> + * the information provided. Since there is no standard
> + * way to configure an interface, we will have an external
> + * script that does the job of configuring the interface and
> + * flushing the configuration.
> + *
> + * The parameters passed to this external script are:
> + * 1. A configuration file that has the specified configuration.
Maybe this should be written as 'A info file that has the requested
network configuration' or something like that.
> + *
> + * We will embed the name of the interface in the configuration
> + * file: ifcfg-ethx (where ethx is the interface name).
I think the intention here is to use the generated file as is. Depending
on the distro in the guest the file may need some processing. So I think
the actual interface name should also be part of the file.
> + *
> + * Here is the format of the ip configuration file:
> + *
> + * HWADDR=macaddr
> + * BOOTPROTO=dhcp (dhcp enabled for the interface)
While BOOTPROTO= is used in current network config files, its meaning
there is unrelated to what its meant here. Here it means DHCP=yes/no, so
I think the file should contain just that. And as the code is written
now BOOTPROTO= is optional, which is not mentioned in the comment.
> + * NM_CONTROLLED=no (this interface will not be controlled by NM)
I think this is not up to kvp_deamon to decide what controls the
interface. Maybe one day NM is sufficiently advanced technology that it
can cope with such requests?
The helper script should decide if the NM flag should be written to the
final config file.
> + * PEERDNS=yes
> + * IPADDR_x=ipaddr
> + * NETMASK_x=netmask
> + * GATEWAY_x=gateway
This should mention that its either IPADDR=ipaddr1 or 'IPADDR=ipaddr1;
IPADDR_1=ipaddr2'
> + * DNSx=dns
This should mention that its either DNS=ipaddr1 or 'DNS=ipaddr1;
DNS1=ipaddr2'
Olaf
^ permalink raw reply
* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: Jon Mason @ 2012-07-30 18:15 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <CAErSpo4p_hxvCeVGqiTXVq3itXepheQZerpqRHwLvG6A7R2Lcg@mail.gmail.com>
On Mon, Jul 30, 2012 at 10:50:13AM -0600, Bjorn Helgaas wrote:
> On Sun, Jul 29, 2012 at 6:26 PM, Jon Mason <jon.mason@intel.com> wrote:
> > A PCI-Express non-transparent bridge (NTB) is a point-to-point PCIe bus
> > connecting 2 systems, providing electrical isolation between the two subsystems.
> > A non-transparent bridge is functionally similar to a transparent bridge except
> > that both sides of the bridge have their own independent address domains. The
> > host on one side of the bridge will not have the visibility of the complete
> > memory or I/O space on the other side of the bridge. To communicate across the
> > non-transparent bridge, each NTB endpoint has one (or more) apertures exposed to
> > the local system. Writes to these apertures are mirrored to memory on the
> > remote system. Communications can also occur through the use of doorbell
> > registers that initiate interrupts to the alternate domain, and scratch-pad
> > registers accessible from both sides.
> >
> > The NTB device driver is needed to configure these memory windows, doorbell, and
> > scratch-pad registers as well as use them in such a way as they can be turned
> > into a viable communication channel to the remote system. ntb_hw.[ch]
> > determines the usage model (NTB to NTB or NTB to Root Port) and abstracts away
> > the underlying hardware to provide access and a common interface to the doorbell
> > registers, scratch pads, and memory windows. These hardware interfaces are
> > exported so that other, non-mainlined kernel drivers can access these.
> > ntb_transport.[ch] also uses the exported interfaces in ntb_hw.[ch] to setup a
> > communication channel(s) and provide a reliable way of transferring data from
> > one side to the other, which it then exports so that "client" drivers can access
> > them. These client drivers are used to provide a standard kernel interface
> > (i.e., Ethernet device) to NTB, such that Linux can transfer data from one
> > system to the other in a standard way.
> >
> > Signed-off-by: Jon Mason <jon.mason@intel.com>
> > ---
> > MAINTAINERS | 6 +
> > drivers/Kconfig | 2 +
> > drivers/Makefile | 1 +
> > drivers/ntb/Kconfig | 13 +
> > drivers/ntb/Makefile | 3 +
> > drivers/ntb/ntb_hw.c | 1178 ++++++++++++++++++++++++++++++++++++
> > drivers/ntb/ntb_hw.h | 206 +++++++
> > drivers/ntb/ntb_regs.h | 150 +++++
> > drivers/ntb/ntb_transport.c | 1387 +++++++++++++++++++++++++++++++++++++++++++
> > include/linux/ntb.h | 92 +++
>
> Where will drivers for non-Intel NTBs fit in this hierarchy? It seems
> a bit presumptuous to claim the generic "ntb" names just for Intel
> devices.
I've tried to make it all generic enough that non-Intel NTBs should plug in with
minimal changes to ntb_hw.c. If their design is too divergent, then a slight
redesign of ntb_hw.c might be necessary. But from what I've seen of other
designs on the internet, they appear to be extremely similar. The transport and
client drivers were written with the hardware abstracted away as much as
possible to prevent the need to modify it for different hardware. If there is
anything which is Intel hardware specific, I'd be happy to change it to make it
more generic.
Thanks,
Jon
^ permalink raw reply
* Re: [RFC v2 2/2] net: Add support for NTB virtual ethernet device
From: Jon Mason @ 2012-07-30 18:19 UTC (permalink / raw)
To: Jiri Pirko; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120730140216.GA1550@minipsycho.orion>
On Mon, Jul 30, 2012 at 04:02:16PM +0200, Jiri Pirko wrote:
> Mon, Jul 30, 2012 at 02:26:34AM CEST, jon.mason@intel.com wrote:
> >+static int __devinit ntb_netdev_probe(struct pci_dev *pdev)
> >+{
> >+ struct net_device *ndev;
> >+ struct ntb_netdev *dev;
> >+ int rc;
> >+
> >+ ndev = alloc_etherdev(sizeof(struct ntb_netdev));
> >+ if (!ndev)
> >+ return -ENOMEM;
> >+
> >+ dev = netdev_priv(ndev);
> >+ dev->ndev = ndev;
> >+ dev->pdev = pdev;
> >+ BUG_ON(!dev->pdev);
> >+ ndev->features = NETIF_F_HIGHDMA;
> >+
> >+ //ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> ^^ I guess you forgot to un-comment this.
Oops. An easy fix. Anything else?
Thanks,
Jon
^ permalink raw reply
* RE: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
From: KY Srinivasan @ 2012-07-30 18:32 UTC (permalink / raw)
To: Olaf Hering
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, apw@canonical.com,
netdev@vger.kernel.org, ben@decadent.org.uk
In-Reply-To: <20120730180326.GB533@aepfle.de>
> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Monday, July 30, 2012 2:03 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; apw@canonical.com; netdev@vger.kernel.org;
> ben@decadent.org.uk
> Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> KVP_OP_SET_IP_INFO
>
> On Tue, Jul 24, K. Y. Srinivasan wrote:
>
> > + /*
> > + * Set the configuration for the specified interface with
> > + * the information provided. Since there is no standard
> > + * way to configure an interface, we will have an external
> > + * script that does the job of configuring the interface and
> > + * flushing the configuration.
> > + *
> > + * The parameters passed to this external script are:
> > + * 1. A configuration file that has the specified configuration.
>
> Maybe this should be written as 'A info file that has the requested
> network configuration' or something like that.
That is the idea. This configuration file simply reflects all the information we have
perhaps with some additional constant information. The script is free to ignore what
it does not need.
>
> > + *
> > + * We will embed the name of the interface in the configuration
> > + * file: ifcfg-ethx (where ethx is the interface name).
>
> I think the intention here is to use the generated file as is. Depending
> on the distro in the guest the file may need some processing. So I think
> the actual interface name should also be part of the file.
That is not the intention although on some distros, the format of this file
may be closer to the distro specific configuration file than others. I will
however include the name of the interface in the file as well.
>
> > + *
> > + * Here is the format of the ip configuration file:
> > + *
> > + * HWADDR=macaddr
> > + * BOOTPROTO=dhcp (dhcp enabled for the interface)
>
> While BOOTPROTO= is used in current network config files, its meaning
> there is unrelated to what its meant here. Here it means DHCP=yes/no, so
> I think the file should contain just that. And as the code is written
> now BOOTPROTO= is optional, which is not mentioned in the comment.
I will fix this.
>
> > + * NM_CONTROLLED=no (this interface will not be controlled by NM)
>
> I think this is not up to kvp_deamon to decide what controls the
> interface. Maybe one day NM is sufficiently advanced technology that it
> can cope with such requests?
> The helper script should decide if the NM flag should be written to the
> final config file.
As I noted earlier, the external script can choose to interpret the contents of this
file in a way that it makes sense for the distribution. Maybe, I will get rid of all the
constant information.
>
> > + * PEERDNS=yes
> > + * IPADDR_x=ipaddr
> > + * NETMASK_x=netmask
> > + * GATEWAY_x=gateway
>
> This should mention that its either IPADDR=ipaddr1 or 'IPADDR=ipaddr1;
> IPADDR_1=ipaddr2'
>
> > + * DNSx=dns
>
> This should mention that its either DNS=ipaddr1 or 'DNS=ipaddr1;
> DNS1=ipaddr2'
I take it that you want the comments to be more explicit on the format.
>
> Olaf
>
>
Thank you Olaf,
K. Y
^ permalink raw reply
* [PATCH] bridge: make port attributes const
From: Stephen Hemminger @ 2012-07-30 18:55 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Simple table that can be marked const.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
net/bridge/br_sysfs_br.c | 136 +++++++++++++++++++++++------------------------
net/bridge/br_sysfs_if.c | 6 +-
3 files changed, 73 insertions(+), 71 deletions(-)
--- a/net/bridge/br_sysfs_if.c 2012-07-19 15:35:29.062250368 -0700
+++ b/net/bridge/br_sysfs_if.c 2012-07-19 15:36:10.843427691 -0700
@@ -27,7 +27,7 @@ struct brport_attribute {
};
#define BRPORT_ATTR(_name,_mode,_show,_store) \
-struct brport_attribute brport_attr_##_name = { \
+const struct brport_attribute brport_attr_##_name = { \
.attr = {.name = __stringify(_name), \
.mode = _mode }, \
.show = _show, \
@@ -164,7 +164,7 @@ static BRPORT_ATTR(multicast_router, S_I
store_multicast_router);
#endif
-static struct brport_attribute *brport_attrs[] = {
+static const struct brport_attribute *brport_attrs[] = {
&brport_attr_path_cost,
&brport_attr_priority,
&brport_attr_port_id,
@@ -241,7 +241,7 @@ const struct sysfs_ops brport_sysfs_ops
int br_sysfs_addif(struct net_bridge_port *p)
{
struct net_bridge *br = p->br;
- struct brport_attribute **a;
+ const struct brport_attribute **a;
int err;
err = sysfs_create_link(&p->kobj, &br->dev->dev.kobj,
^ permalink raw reply
* v3.5: NETDEV WATCHDOG: eth0 (r8169): transmit queue 0 timed out
From: George Spelvin @ 2012-07-30 19:02 UTC (permalink / raw)
To: netdev; +Cc: linux
This actually isn't new to 3.5, but I figured I'd mention it again
anyway. The machine serves firewalll duty. A quad-tulip fast ethernet
card serves the external interfaces and DMZ, while an on-board Realtek
gigabit interface serves the main LAN. The latter is hitting a transmit
queue timeout.
02:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express Gigabit Ethernet controller [10ec:8168] (rev 01)
Soon after startup, the following WARN_ONCE triggers:
r8169 0000:02:00.0: inside: link down
r8169 0000:02:00.0: inside: link down
IPv6: ADDRCONF(NETDEV_UP): inside: link is not ready
r8169 0000:02:00.0: inside: link up
IPv6: ADDRCONF(NETDEV_CHANGE): inside: link becomes ready
net dmz: Setting full-duplex based on MII#1 link partner capability of 45e1
net cable: Setting full-duplex based on MII#1 link partner capability of 41e1
postgres (5652): /proc/5652/oom_adj is deprecated, please use /proc/5652/oom_score_adj instead.
pps pps0: new PPS source serial3 at ID 0
pps pps0: source "/dev/ttyS3" added
pps pps1: new PPS source serial4 at ID 1
pps pps1: source "/dev/ttyS4" added
device cable entered promiscuous mode
device dmz entered promiscuous mode
nf_conntrack: automatic helper assignment is deprecated and it will be removed soon. Use the iptables CT target to attach helpers instead.
ntop[6305]: segfault at 0 ip 00000000f750644b sp 00000000f01fbd50 error 4 in libntop-4.99.3.so[f74bf000+7e000]
device cable left promiscuous mode
device dmz left promiscuous mode
------------[ cut here ]------------
WARNING: at net/sched/sch_generic.c:255 dev_watchdog+0xe9/0x15c()
Hardware name: MS-7376
NETDEV WATCHDOG: inside (r8169): transmit queue 0 timed out
Pid: 0, comm: swapper/3 Not tainted 3.5.0-00020-g7bd6501 #160
Call Trace:
<IRQ> [<ffffffff8131d16e>] ? dev_watchdog+0xe9/0x15c
[<ffffffff810254bd>] ? warn_slowpath_common+0x71/0x85
[<ffffffff8131d085>] ? netif_tx_lock+0x7a/0x7a
[<ffffffff81025535>] ? warn_slowpath_fmt+0x45/0x4a
[<ffffffff8103ce9c>] ? hrtimer_interrupt+0x100/0x1a4
[<ffffffff8131d072>] ? netif_tx_lock+0x67/0x7a
[<ffffffff8131d16e>] ? dev_watchdog+0xe9/0x15c
[<ffffffff8104e764>] ? clockevents_program_event+0x9a/0xb6
[<ffffffff8102e9bc>] ? run_timer_softirq+0x17e/0x20b
[<ffffffff8102a945>] ? __do_softirq+0x80/0x102
[<ffffffff81411d4c>] ? call_softirq+0x1c/0x30
[<ffffffff810032f4>] ? do_softirq+0x2c/0x60
[<ffffffff8102ab78>] ? irq_exit+0x3a/0x91
[<ffffffff81003141>] ? do_IRQ+0x81/0x97
[<ffffffff81410367>] ? common_interrupt+0x67/0x67
<EOI> [<ffffffff81007d63>] ? default_idle+0x1e/0x32
[<ffffffff81007e87>] ? amd_e400_idle+0xb7/0xd4
[<ffffffff81008501>] ? cpu_idle+0x58/0x98
---[ end trace 9ca7e03889ac2abb ]---
r8169 0000:02:00.0: inside: link up
udevd[30490]: starting version 175
UDP: short packet: From 151.60.217.248:59748 27692/73 to 71.41.210.146:43935
r8169 0000:02:00.0: inside: link up
There are a few local patches, but only two are anywhere close to the
network interface, and they are test patches designed to fix this.
Should I get rid of them?
It's a production system, so I don't like to bounce it too often, but I can
test after hours.
One local patch I've thought about writing is something to count
how often this happens (probably in sysfs).
commit 526ef2edd6ffb8f529256a033ca3a266530739d3
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed May 23 22:18:35 2012 +0200
r8169: avoid clearing the end of Tx descriptor ring marker bit.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index d7a04e0..9c35130 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -5346,7 +5346,7 @@ static void rtl8169_unmap_tx_skb(struct device *d, struct ring_info *tx_skb,
dma_unmap_single(d, le64_to_cpu(desc->addr), len, DMA_TO_DEVICE);
- desc->opts1 = 0x00;
+ desc->opts1 &= cpu_to_le32(RingEnd);
desc->opts2 = 0x00;
desc->addr = 0x00;
tx_skb->len = 0;
commit 7bd6501f52c2c0382b41fe77b9bd003ff768a44c
Author: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed May 23 23:21:13 2012 +0200
r8169: TxPoll hack rework.
I don't want to try and convince myself that it is completely safe to
issue a TxPoll request from the NAPI handler right in the middle of
a start_xmit, whence the tx_lock.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 9c35130..be5cd33 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -5545,19 +5545,20 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
status = opts[0] | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
txd->opts1 = cpu_to_le32(status);
+ smp_wmb();
+
tp->cur_tx += frags + 1;
- wmb();
+ smp_wmb();
RTL_W8(TxPoll, NPQ);
mmiowb();
if (!TX_FRAGS_READY_FOR(tp, MAX_SKB_FRAGS)) {
- /* Avoid wrongly optimistic queue wake-up: rtl_tx thread must
- * not miss a ring update when it notices a stopped queue.
- */
- smp_wmb();
+ /* rtl_tx thread must not miss a ring update when it notices
+ * a stopped queue. The TxPoll hack requires the smp_wmb
+ * above so we can go ahead. */
netif_stop_queue(dev);
/* Sync with rtl_tx:
* - publish queue status and cur_tx ring index (write barrier)
@@ -5641,22 +5642,36 @@ struct rtl_txc {
static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
{
struct rtl8169_stats *tx_stats = &tp->tx_stats;
- unsigned int dirty_tx, tx_left;
+ unsigned int dirty_tx, cur_tx;
struct rtl_txc txc = { 0, 0 };
dirty_tx = tp->dirty_tx;
- smp_rmb();
- tx_left = tp->cur_tx - dirty_tx;
-
- while (tx_left > 0) {
+xmit_race:
+ for (cur_tx = tp->cur_tx; dirty_tx != cur_tx; dirty_tx++) {
unsigned int entry = dirty_tx % NUM_TX_DESC;
struct ring_info *tx_skb = tp->tx_skb + entry;
u32 status;
- rmb();
status = le32_to_cpu(tp->TxDescArray[entry].opts1);
- if (status & DescOwn)
+
+ /* 8168 (only ?) hack: TxPoll requests are lost when the Tx
+ * packets are too close. Let's kick an extra TxPoll request
+ * when a burst of start_xmit activity is detected (if it is
+ * not detected, it is slow enough).
+ * The NPQ bit is cleared automatically by the chipset.
+ * The code assumes that the chipset is sane enough to clear
+ * it at a sensible time.*/
+ if (unlikely(status & DescOwn)) {
+ void __iomem *ioaddr = tp->mmio_addr;
+
+ if (!(RTL_R8(TxPoll) & NPQ)) {
+ netif_tx_lock(dev);
+ RTL_W8(TxPoll, NPQ);
+ netif_tx_unlock(dev);
+ goto done;
+ }
break;
+ }
rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
tp->TxDescArray + entry);
@@ -5668,10 +5683,15 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
dev_kfree_skb(skb);
tx_skb->skb = NULL;
}
- dirty_tx++;
- tx_left--;
}
+ /* Rationale: if chipset stopped DMAing, enforce TxPoll write either
+ * here or in start_xmit. If chipset is still DMAing, this code
+ * will be run later anyway. */
+ smp_mb();
+ if (cur_tx != tp->cur_tx)
+ goto xmit_race;
+done:
u64_stats_update_begin(&tx_stats->syncp);
tx_stats->packets += txc.packets;
tx_stats->bytes += txc.bytes;
@@ -5693,17 +5713,6 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
TX_FRAGS_READY_FOR(tp, MAX_SKB_FRAGS)) {
netif_wake_queue(dev);
}
- /*
- * 8168 hack: TxPoll requests are lost when the Tx packets are
- * too close. Let's kick an extra TxPoll request when a burst
- * of start_xmit activity is detected (if it is not detected,
- * it is slow enough). -- FR
- */
- if (tp->cur_tx != dirty_tx) {
- void __iomem *ioaddr = tp->mmio_addr;
-
- RTL_W8(TxPoll, NPQ);
- }
}
}
^ permalink raw reply related
* RE: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
From: KY Srinivasan @ 2012-07-30 19:12 UTC (permalink / raw)
To: Olaf Hering
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, apw@canonical.com,
netdev@vger.kernel.org, ben@decadent.org.uk
In-Reply-To: <20120730173323.GA533@aepfle.de>
> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Monday, July 30, 2012 1:33 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; apw@canonical.com; netdev@vger.kernel.org;
> ben@decadent.org.uk
> Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> KVP_OP_SET_IP_INFO
>
> On Tue, Jul 24, K. Y. Srinivasan wrote:
>
> > +static char *kvp_get_if_name(char *guid)
> > +{
> > + DIR *dir;
> > + struct dirent *entry;
> > + FILE *file;
> > + char *p, *q, *x;
> > + char *if_name = NULL;
> > + char buf[256];
> > + char *kvp_net_dir = "/sys/class/net/";
> > + char dev_id[100];
>
> Perhaps this can be written as char dev_id[100] = "short string";?
Why?
>
> > +
> > + dir = opendir(kvp_net_dir);
> > + if (dir == NULL)
> > + return NULL;
> > +
> > + memset(dev_id, 0, sizeof(dev_id));
> > + strcat(dev_id, kvp_net_dir);
> > + q = dev_id + strlen(kvp_net_dir);
> > +
> > + while ((entry = readdir(dir)) != NULL) {
> > + /*
> > + * Set the state for the next pass.
> > + */
> > + *q = '\0';
> > + strcat(dev_id, entry->d_name);
> > + strcat(dev_id, "/device/device_id");
>
> Maybe this (and other) strcat should be changed to snprintf?
Already done.
>
> > +
> > + file = fopen(dev_id, "r");
> > + if (file == NULL)
> > + continue;
> > +
> > + p = fgets(buf, sizeof(buf), file);
> > + if (p) {
> > + x = strchr(p, '\n');
> > + if (x)
> > + *x = '\0';
> > +
> > + if (!strcmp(p, guid)) {
> > + /*
> > + * Found the guid match; return the interface
> > + * name. The caller will free the memory.
> > + */
> > + if_name = strdup(entry->d_name);
> > + break;
>
> This will leave 'file' open.
> I have seen it in some other place as well.
I will audit all instances and fix it.
>
> > + }
> > + }
> > + fclose(file);
> > + }
> > +
> > + closedir(dir);
> > + return if_name;
> > +}
> > +
> > +/*
> > + * Retrieve the MAC address given the interface name.
> > + */
> > +
> > +static char *kvp_if_name_to_mac(char *if_name)
> > +{
> > + FILE *file;
> > + char *p, *x;
> > + char buf[256];
> > + char addr_file[100];
> > + int i;
> > + char *mac_addr = NULL;
> > +
> > + memset(addr_file, 0, sizeof(addr_file));
> > + strcat(addr_file, "/sys/class/net/");
> > + strcat(addr_file, if_name);
> > + strcat(addr_file, "/address");
>
> snprintf?
Already fixed.
>
> > +
> > + file = fopen(addr_file, "r");
> > + if (file == NULL)
> > + return NULL;
> > +
> > + p = fgets(buf, sizeof(buf), file);
> > + if (p) {
> > + x = strchr(p, '\n');
> > + if (x)
> > + *x = '\0';
> > + for (i = 0; i < strlen(p); i++)
> > + p[i] = toupper(p[i]);
> > + mac_addr = strdup(p);
> > + }
> > +
> > + fclose(file);
> > + return mac_addr;
> > +}
> > +
> > +
> > static void kvp_process_ipconfig_file(char *cmd,
> > char *config_buf, int len,
> > int element_size, int offset)
> > @@ -800,6 +910,314 @@ getaddr_done:
> > }
> >
> >
> > +static int expand_ipv6(char *addr, int type)
> > +{
> > + int ret;
> > + struct in6_addr v6_addr;
> > +
> > + ret = inet_pton(AF_INET6, addr, &v6_addr);
> > +
> > + if (ret != 1) {
> > + if (type == NETMASK)
> > + return 1;
> > + return 0;
> > + }
> > +
> > + sprintf(addr, "%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:"
> > + "%02x%02x:%02x%02x:%02x%02x",
> > + (int)v6_addr.s6_addr[0], (int)v6_addr.s6_addr[1],
> > + (int)v6_addr.s6_addr[2], (int)v6_addr.s6_addr[3],
> > + (int)v6_addr.s6_addr[4], (int)v6_addr.s6_addr[5],
> > + (int)v6_addr.s6_addr[6], (int)v6_addr.s6_addr[7],
> > + (int)v6_addr.s6_addr[8], (int)v6_addr.s6_addr[9],
> > + (int)v6_addr.s6_addr[10], (int)v6_addr.s6_addr[11],
> > + (int)v6_addr.s6_addr[12], (int)v6_addr.s6_addr[13],
> > + (int)v6_addr.s6_addr[14], (int)v6_addr.s6_addr[15]);
> > +
> > + return 1;
> > +
> > +}
> > +
> > +static int is_ipv4(char *addr)
> > +{
> > + int ret;
> > + struct in_addr ipv4_addr;
> > +
> > + ret = inet_pton(AF_INET, addr, &ipv4_addr);
> > +
> > + if (ret == 1)
> > + return 1;
> > + return 0;
> > +}
> > +
> > +static int parse_ip_val_buffer(char *in_buf, int *offset,
> > + char *out_buf, int out_len)
> > +{
> > + char *x;
> > + char *start;
> > +
> > + /*
> > + * in_buf has sequence of characters that are seperated by
> > + * the character ';'. The last sequence does not have the
> > + * terminating ";" character.
> > + */
> > + start = in_buf + *offset;
> > +
> > + x = strchr(start, ';');
> > + if (x)
> > + *x = 0;
> > + else
> > + x = start + strlen(start);
> > +
> > + if (strlen(start) != 0) {
> > + int i = 0;
> > + /*
> > + * Get rid of leading spaces.
> > + */
> > + while (start[i] == ' ')
> > + i++;
> > +
> > + if ((x - start) <= out_len) {
> > + strcpy(out_buf, (start + i));
> > + *offset += (x - start) + 1;
> > + return 1;
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +static int kvp_write_file(FILE *f, char *s1, char *s2, char *s3)
> > +{
> > + char str[256];
> > + int error;
> > +
> > + memset(str, 0, sizeof(str));
> > + strcat(str, s1);
> > + if (s2 != NULL)
> > + strcat(str, s2);
> > + strcat(str, "=");
> > + strcat(str, s3);
> > + strcat(str, "\n");
> > +
> > + error = fputs(str, f);
> > + if (error == EOF)
> > + return HV_E_FAIL;
> > +
> > + return 0;
> > +}
> > +
> > +
> > +static int process_ip_string(FILE *f, char *ip_string, int type)
> > +{
> > + int error = 0;
> > + char addr[INET6_ADDRSTRLEN];
> > + int i = 0;
> > + int j = 0;
> > + char str[256];
> > + char sub_str[10];
> > + int offset = 0;
> > +
> > + memset(addr, 0, sizeof(addr));
> > +
> > + while (parse_ip_val_buffer(ip_string, &offset, addr,
> > + (MAX_IP_ADDR_SIZE * 2))) {
> > + memset(sub_str, 0, sizeof(sub_str));
> > + memset(str, 0, sizeof(str));
> > +
> > + if (is_ipv4(addr)) {
> > + switch (type) {
> > + case IPADDR:
> > + strcat(str, "IPADDR");
> > + break;
> > + case NETMASK:
> > + strcat(str, "NETMASK");
> > + break;
> > + case GATEWAY:
> > + strcat(str, "GATEWAY");
> > + break;
> > + case DNS:
> > + strcat(str, "DNS");
> > + break;
> > + }
> > + if (i != 0) {
> > + if (type != DNS)
> > + sprintf(sub_str, "_%d", i++);
> > + else
> > + sprintf(sub_str, "%d", ++i);
> > + } else if (type == DNS) {
> > + sprintf(sub_str, "%d", ++i);
> > + }
> > +
> > +
> > + } else if (expand_ipv6(addr, type)) {
> > + switch (type) {
> > + case IPADDR:
> > + strcat(str, "IPV6ADDR");
> > + break;
> > + case NETMASK:
> > + strcat(str, "IPV6NETMASK");
> > + break;
> > + case GATEWAY:
> > + strcat(str, "IPV6_DEFAULTGW");
> > + break;
> > + case DNS:
> > + strcat(str, "DNS");
> > + break;
> > + }
> > + if ((j != 0) || (type == DNS)) {
> > + if (type != DNS)
> > + sprintf(sub_str, "_%d", j++);
> > + else
> > + sprintf(sub_str, "%d", ++i);
> > + } else if (type == DNS) {
> > + sprintf(sub_str, "%d", ++i);
> > + }
> > + } else {
> > + return HV_INVALIDARG;
> > + }
> > +
> > + error = kvp_write_file(f, str, sub_str, addr);
> > + if (error)
> > + return error;
> > + memset(addr, 0, sizeof(addr));
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int kvp_set_ip_info(char *if_name, struct hv_kvp_ipaddr_value
> *new_val)
> > +{
> > + int error = 0;
> > + char if_file[50];
> > + FILE *file;
> > + char cmd[512];
> > + char *mac_addr;
> > +
> > + /*
> > + * Set the configuration for the specified interface with
> > + * the information provided. Since there is no standard
> > + * way to configure an interface, we will have an external
> > + * script that does the job of configuring the interface and
> > + * flushing the configuration.
> > + *
> > + * The parameters passed to this external script are:
> > + * 1. A configuration file that has the specified configuration.
> > + *
> > + * We will embed the name of the interface in the configuration
> > + * file: ifcfg-ethx (where ethx is the interface name).
> > + *
> > + * Here is the format of the ip configuration file:
> > + *
> > + * HWADDR=macaddr
> > + * BOOTPROTO=dhcp (dhcp enabled for the interface)
> > + * NM_CONTROLLED=no (this interface will not be controlled by NM)
> > + * PEERDNS=yes
> > + * IPADDR_x=ipaddr
> > + * NETMASK_x=netmask
> > + * GATEWAY_x=gateway
> > + * DNSx=dns
> > + *
> > + * IPV6 addresses will be tagged as IPV6ADDR, IPV6 gateway will be
> > + * tagged as IPV6_DEFAULTGW and IPV6 NETMASK will be tagged as
> > + * IPV6NETMASK.
> > + */
> > +
> > + memset(if_file, 0, sizeof(if_file));
> > + strcat(if_file, "/var/opt/hyperv/ifcfg-");
> > + strcat(if_file, if_name);
> > +
> > + file = fopen(if_file, "w");
> > +
> > + if (file == NULL) {
> > + syslog(LOG_ERR, "Failed to open config file");
> > + return HV_E_FAIL;
> > + }
> > +
> > + /*
> > + * First write out the MAC address.
> > + */
> > +
> > + mac_addr = kvp_if_name_to_mac(if_name);
> > + if (mac_addr == NULL) {
> > + error = HV_E_FAIL;
> > + goto setval_error;
> > + }
> > +
> > + error = kvp_write_file(file, "HWADDR", NULL, mac_addr);
> > + if (error)
> > + goto setval_error;
> > +
> > + error = kvp_write_file(file, "ONBOOT", NULL, "yes");
> > + if (error)
> > + goto setval_error;
> > +
> > + error = kvp_write_file(file, "IPV6INIT", NULL, "yes");
> > + if (error)
> > + goto setval_error;
> > +
> > + error = kvp_write_file(file, "NM_CONTROLLED", NULL, "no");
> > + if (error)
> > + goto setval_error;
> > +
> > + error = kvp_write_file(file, "PEERDNS", NULL, "yes");
> > + if (error)
> > + goto setval_error;
> > +
> > + if (new_val->dhcp_enabled) {
> > + error = kvp_write_file(file, "BOOTPROTO", NULL, "dhcp");
> > + if (error)
> > + goto setval_error;
> > +
> > + /*
> > + * We are done!.
> > + */
> > + goto setval_done;
> > + }
> > +
> > + /*
> > + * Write the configuration for ipaddress, netmask, gateway and
> > + * name servers.
> > + */
> > +
> > + error = process_ip_string(file, (char *)new_val->ip_addr, IPADDR);
> > + if (error)
> > + goto setval_error;
> > +
> > + error = process_ip_string(file, (char *)new_val->sub_net, NETMASK);
> > + if (error)
> > + goto setval_error;
> > +
> > + error = process_ip_string(file, (char *)new_val->gate_way, GATEWAY);
> > + if (error)
> > + goto setval_error;
> > +
> > + error = process_ip_string(file, (char *)new_val->dns_addr, DNS);
> > + if (error)
> > + goto setval_error;
> > +
> > +setval_done:
> > + free(mac_addr);
> > + fclose(file);
> > +
> > + /*
> > + * Now that we have populated the configuration file,
> > + * invoke the external script to do its magic.
> > + */
> > +
> > + memset(cmd, 0, sizeof(cmd));
> > + strcat(cmd, "/sbin/hv_set_ifconfig ");
> > + strcat(cmd, if_file);
>
> The new patch should use "%s %s", not "%s%s" as format string.
>
>
>
Thanks Olaf,
K. Y
^ permalink raw reply
* Re: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
From: Ben Hutchings @ 2012-07-30 19:19 UTC (permalink / raw)
To: KY Srinivasan
Cc: Olaf Hering, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
apw@canonical.com, netdev@vger.kernel.org
In-Reply-To: <426367E2313C2449837CD2DE46E7EAF9236AA7DB@SN2PRD0310MB382.namprd03.prod.outlook.com>
On Mon, Jul 30, 2012 at 06:32:15PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Olaf Hering [mailto:olaf@aepfle.de]
> > Sent: Monday, July 30, 2012 2:03 PM
> > To: KY Srinivasan
> > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; apw@canonical.com; netdev@vger.kernel.org;
> > ben@decadent.org.uk
> > Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> > KVP_OP_SET_IP_INFO
> >
> > On Tue, Jul 24, K. Y. Srinivasan wrote:
> >
> > > + /*
> > > + * Set the configuration for the specified interface with
> > > + * the information provided. Since there is no standard
> > > + * way to configure an interface, we will have an external
> > > + * script that does the job of configuring the interface and
> > > + * flushing the configuration.
> > > + *
> > > + * The parameters passed to this external script are:
> > > + * 1. A configuration file that has the specified configuration.
> >
> > Maybe this should be written as 'A info file that has the requested
> > network configuration' or something like that.
>
> That is the idea. This configuration file simply reflects all the
> information we have perhaps with some additional constant
> information. The script is free to ignore what it does not need.
[...]
This does not strike me as a sensible interface. If scripts are
'free to ignore' information then the KVP interface becomes unreliable
as a means for managing networking on Linux guests. I would suggest
that at the least the script should be able to report that it did not
recognise some parts of the configuration. This would be logged
and/or reported back to the hypervisor.
(This is separate from the issue of constant configuration lines;
for some distributions the script might recognise but ignore them
because they have no use on that distribution. I don't see the
point in constant lines, but they don't seem to result in any
unreliability.)
Ben.
--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus
^ permalink raw reply
* imx6q: poor FEC Gigabit implementation
From: Wolfgang Grandegger @ 2012-07-30 19:24 UTC (permalink / raw)
To: linux-arm-kernel, Linux Netdev List
Hello,
I did some Gigabit performance measurements with the i.MX6Q Sabrelite
board. I measure an UDP throughput of 470 Mb/s to an external host,
which is already very close to the theoretical limit listed in the Chip
Errata ERR004512 for the i.MX 6Dual/6Quad, Rev. C, 2/2012.
At a closer look to the driver I realized that there is no support for
- NAPI
- IP checksum offloading
- Jumbo frames
According to the manual, these features are support by the ENET via
enhanced buffer descriptors. Even if they will not increase the
performance due to the mentioned system limitation, they should reduce
the interrupt and CPU load significantly. I did some quick tests with IP
checksum offloading and Jumbo frames but without success. Are there any
known issues? Is anybody already working on an enhanced FEC Gigabit
ethernet driver?
TIA.
Wolfgang.
^ permalink raw reply
* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 19:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1343669507.21269.33.camel@edumazet-glaptop>
On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> On Mon, 2012-07-30 at 18:16 +0100, Ben Hutchings wrote:
> > A peer (or local user) may cause TCP to use a nominal MSS of as little
> > as 88 (actual MSS of 76 with timestamps). Given that we have a
> > sufficiently prodigious local sender and the peer ACKs quickly enough,
> > it is nevertheless possible to grow the window for such a connection
> > to the point that we will try to send just under 64K at once. This
> > results in a single skb that expands to 861 segments.
> >
> > In some drivers with TSO support, such an skb will require hundreds of
> > DMA descriptors; a substantial fraction of a TX ring or even more than
> > a full ring. The TX queue selected for the skb may stall and trigger
> > the TX watchdog repeatedly (since the problem skb will be retried
> > after the TX reset). This particularly affects sfc, for which the
> > issue is designated as CVE-2012-3412. However it may be that some
> > hardware or firmware also fails to handle such an extreme TSO request
> > correctly.
> >
> > Therefore, limit the number of segments per skb to 100. This should
> > make no difference to behaviour unless the actual MSS is less than
> > about 700.
> >
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
>
>
> Hmm, isnt GRO path also vulnerable ?
You mean, for forwarding? If page fragments are used, the number of
segments is limited to MAX_SKB_FRAGS < 100. But if skbs are aggregated
and build_skb() is not used (e.g. due to jumbo MTU) it appears we would
need an explicit limit. Something like this:
---
From: Ben Hutchings <bhutchings@solarflare.com>
Subject: [PATCH net] tcp: Limit number of segments merged by GRO
In the case where GRO aggregates skbs that cannot be converted to
page-fragments, there is currently no limit to the number of
segments that may be merged and subsequently re-segmented by GSO.
Apply the same limit as was introduced for locally-generated GSO skbs
in 'tcp: Limit number of segments generated by GSO per skb'.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
net/ipv4/tcp.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 51d8daf..a052d07 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3144,7 +3144,8 @@ out_check_final:
TCP_FLAG_RST | TCP_FLAG_SYN |
TCP_FLAG_FIN));
- if (p && (!NAPI_GRO_CB(skb)->same_flow || flush))
+ if (p && (!NAPI_GRO_CB(skb)->same_flow || flush ||
+ NAPI_GRO_CB(p)->count == TCP_MAX_GSO_SEGS))
pp = head;
out:
---
> An alternative would be to drop such frames in the ndo_start_xmit(), and
> cap sk->sk_gso_max_size (since skb are no longer orphaned...)
I have implemented that workaround for the out-of-tree version of sfc.
For the in-tree driver, I thought it would be better to limit the number
of segments at source, which will avoid penalising any cases where the
window can grow so much larger than MSS.
> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
> driver sets to whatever limit ?
Yes, that's another option.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 19:41 UTC (permalink / raw)
To: Ben Greear; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <5016C305.7080907@candelatech.com>
On Mon, 2012-07-30 at 10:23 -0700, Ben Greear wrote:
> On 07/30/2012 10:16 AM, Ben Hutchings wrote:
> > A peer (or local user) may cause TCP to use a nominal MSS of as little
> > as 88 (actual MSS of 76 with timestamps). Given that we have a
> > sufficiently prodigious local sender and the peer ACKs quickly enough,
> > it is nevertheless possible to grow the window for such a connection
> > to the point that we will try to send just under 64K at once. This
> > results in a single skb that expands to 861 segments.
> >
> > In some drivers with TSO support, such an skb will require hundreds of
> > DMA descriptors; a substantial fraction of a TX ring or even more than
> > a full ring. The TX queue selected for the skb may stall and trigger
> > the TX watchdog repeatedly (since the problem skb will be retried
> > after the TX reset). This particularly affects sfc, for which the
> > issue is designated as CVE-2012-3412. However it may be that some
> > hardware or firmware also fails to handle such an extreme TSO request
> > correctly.
> >
> > Therefore, limit the number of segments per skb to 100. This should
> > make no difference to behaviour unless the actual MSS is less than
> > about 700.
>
> Please do not do this...or at least allow over-rides. We love
> the trick of seting very small MSS and making the NICs generate
> huge numbers of small TCP frames with efficient user-space
> logic. We use this for stateful TCP load testing when high
> numbers of tcp packets-per-second is desired.
Please test whether this actually makes a difference - my suspicion is
that 100 segments per skb is easily enough to prevent the host being a
bottleneck.
> Intel NICs, including 10G, work just fine with minimal MSS
> in this scenario.
I'll leave this to the Intel maintainers to answer.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* RE: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests
From: Haiyang Zhang @ 2012-07-30 19:48 UTC (permalink / raw)
To: Olaf Hering
Cc: davem@davemloft.net, netdev@vger.kernel.org, KY Srinivasan,
linux-kernel@vger.kernel.org, devel@linuxdriverproject.org
In-Reply-To: <20120730123927.GA29459@aepfle.de>
> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Monday, July 30, 2012 8:39 AM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; KY Srinivasan; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: Re: [PATCH net,1/1] hyperv: Add support for setting MAC from
> within guests
>
> On Tue, Jul 10, Haiyang Zhang wrote:
>
> > diff --git a/drivers/net/hyperv/rndis_filter.c
> > b/drivers/net/hyperv/rndis_filter.c
> > index 981ebb1..fbf5394 100644
> > --- a/drivers/net/hyperv/rndis_filter.c
> > +++ b/drivers/net/hyperv/rndis_filter.c
> > @@ -47,6 +48,7 @@ struct rndis_request {
> > struct hv_page_buffer buf;
> > /* FIXME: We assumed a fixed size request here. */
> > struct rndis_message request_msg;
> > + u8 ext[100];
>
> This array is not referenced in the patch.
> Please add a comment to the code what the purpose of this array is, and why
> its size is 100 bytes.
It's a buffer for the extended info after the RNDIS message. It's referenced based
on the data offset in the RNDIS message. 100 byte size is enough for current needs,
and should be sufficient for the near future.
I will add a comment to the code.
Thanks,
- Haiyang
^ permalink raw reply
* [PATCH iproute2] ss: Report MSS from internal TCP information
From: Ben Hutchings @ 2012-07-30 19:51 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
misc/ss.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index cf529ef..8ad830b 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1392,6 +1392,8 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r)
(double)info->tcpi_rttvar/1000);
if (info->tcpi_ato)
printf(" ato:%g", (double)info->tcpi_ato/1000);
+ if (info->tcpi_snd_mss)
+ printf(" mss:%d", info->tcpi_snd_mss);
if (info->tcpi_snd_cwnd != 2)
printf(" cwnd:%d", info->tcpi_snd_cwnd);
if (info->tcpi_snd_ssthresh < 0xFFFF)
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 19:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1343676952.2667.26.camel@bwh-desktop.uk.solarflarecom.com>
On Mon, 2012-07-30 at 20:35 +0100, Ben Hutchings wrote:
> On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> > On Mon, 2012-07-30 at 18:16 +0100, Ben Hutchings wrote:
> > > A peer (or local user) may cause TCP to use a nominal MSS of as little
> > > as 88 (actual MSS of 76 with timestamps). Given that we have a
> > > sufficiently prodigious local sender and the peer ACKs quickly enough,
> > > it is nevertheless possible to grow the window for such a connection
> > > to the point that we will try to send just under 64K at once. This
> > > results in a single skb that expands to 861 segments.
> > >
> > > In some drivers with TSO support, such an skb will require hundreds of
> > > DMA descriptors; a substantial fraction of a TX ring or even more than
> > > a full ring. The TX queue selected for the skb may stall and trigger
> > > the TX watchdog repeatedly (since the problem skb will be retried
> > > after the TX reset). This particularly affects sfc, for which the
> > > issue is designated as CVE-2012-3412. However it may be that some
> > > hardware or firmware also fails to handle such an extreme TSO request
> > > correctly.
> > >
> > > Therefore, limit the number of segments per skb to 100. This should
> > > make no difference to behaviour unless the actual MSS is less than
> > > about 700.
[...]
> > An alternative would be to drop such frames in the ndo_start_xmit(), and
> > cap sk->sk_gso_max_size (since skb are no longer orphaned...)
>
> I have implemented that workaround for the out-of-tree version of sfc.
> For the in-tree driver, I thought it would be better to limit the number
> of segments at source, which will avoid penalising any cases where the
> window can grow so much larger than MSS.
[...]
I mean any *legitimate* cases where this can happen.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC v2 2/2] net: Add support for NTB virtual ethernet device
From: Jiri Pirko @ 2012-07-30 20:09 UTC (permalink / raw)
To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120730181910.GB987@jonmason-lab>
Mon, Jul 30, 2012 at 08:19:11PM CEST, jon.mason@intel.com wrote:
>On Mon, Jul 30, 2012 at 04:02:16PM +0200, Jiri Pirko wrote:
>> Mon, Jul 30, 2012 at 02:26:34AM CEST, jon.mason@intel.com wrote:
>> >+static int __devinit ntb_netdev_probe(struct pci_dev *pdev)
>> >+{
>> >+ struct net_device *ndev;
>> >+ struct ntb_netdev *dev;
>> >+ int rc;
>> >+
>> >+ ndev = alloc_etherdev(sizeof(struct ntb_netdev));
>> >+ if (!ndev)
>> >+ return -ENOMEM;
>> >+
>> >+ dev = netdev_priv(ndev);
>> >+ dev->ndev = ndev;
>> >+ dev->pdev = pdev;
>> >+ BUG_ON(!dev->pdev);
>> >+ ndev->features = NETIF_F_HIGHDMA;
>> >+
>> >+ //ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
>> ^^ I guess you forgot to un-comment this.
>
>Oops. An easy fix. Anything else?
The rest looks good to me.
>
>Thanks,
>Jon
^ permalink raw reply
* [PATCH 1/2] drivers: net: ethernet: cpsw: Add SOC dependency support for cpsw dependent modules
From: Mugunthan V N @ 2012-07-30 20:17 UTC (permalink / raw)
To: netdev; +Cc: davem, Mugunthan V N
In-Reply-To: <1343679434-2369-1-git-send-email-mugunthanvnm@ti.com>
cpsw is dependent on davinci_cpdma and davinci_mdio, so adding SOC support for
dependent modules
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
drivers/net/ethernet/ti/Kconfig | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 1b173a6..b26cbda 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -32,7 +32,7 @@ config TI_DAVINCI_EMAC
config TI_DAVINCI_MDIO
tristate "TI DaVinci MDIO Support"
- depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 )
+ depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX )
select PHYLIB
---help---
This driver supports TI's DaVinci MDIO module.
@@ -42,7 +42,7 @@ config TI_DAVINCI_MDIO
config TI_DAVINCI_CPDMA
tristate "TI DaVinci CPDMA Support"
- depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 )
+ depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX )
---help---
This driver supports TI's DaVinci CPDMA dma engine.
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/2] drivers: net: ethernet: cpsw: Add device tree support to CPSW
From: Mugunthan V N @ 2012-07-30 20:17 UTC (permalink / raw)
To: netdev; +Cc: davem, Mugunthan V N
In-Reply-To: <1343679434-2369-1-git-send-email-mugunthanvnm@ti.com>
This patch adds device tree support for cpsw driver
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
Documentation/devicetree/bindings/net/cpsw.txt | 104 ++++++++++++++
drivers/net/ethernet/ti/cpsw.c | 174 +++++++++++++++++++++++-
2 files changed, 272 insertions(+), 6 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/cpsw.txt
diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
new file mode 100644
index 0000000..acca48c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -0,0 +1,104 @@
+TI SoC Ethernet Switch Controller Device Tree Bindings
+------------------------------------------------------
+
+Required properties:
+- compatible : Should be "ti,cpsw"
+- reg : physical base address and size of the cpsw
+ registers map
+- interrupts : property with a value describing the interrupt
+ number
+- interrupt-parent : The parent interrupt controller
+- cpdma_channels : Specifies number of channels in CPDMA
+- host_port_no : Specifies host port shift
+- cpdma_reg_ofs : Specifies CPDMA submodule register offset
+- ale_reg_ofs : Specifies ALE submodule register offset
+- ale_entries : Specifies No of entries ALE can hold
+- host_port_reg_ofs : Specifies host port register offset
+- hw_stats_reg_ofs : Specifies hardware statistics register offset
+- bd_ram_ofs : Specifies internal desciptor RAM offset
+- bd_ram_size : Specifies internal descriptor RAM size
+- rx_descs : Specifies number of Rx descriptors
+- mac_control : Specifies Default MAC control register content
+ for the specific platform
+- slaves : Specifies number for slaves
+- slave_reg_ofs : Specifies slave register offset
+- sliver_reg_ofs : Specifies slave sliver register offset
+- phy_id : Specifies slave phy id
+- mac-address : Specifies slave MAC address
+
+Optional properties:
+- ti,hwmods : Must be "cpgmac0"
+- no_bd_ram : Must be 0 or 1
+
+Note: "ti,hwmods" field is used to fetch the base address and irq
+resources from TI, omap hwmod data base during device registration.
+Future plan is to migrate hwmod data base contents into device tree
+blob so that, all the required data will be used from device tree dts
+file.
+
+Examples:
+
+ mac: ethernet@4A100000 {
+ compatible = "ti,cpsw";
+ reg = <0x4A100000 0x1000>;
+ interrupts = <55 0x4>;
+ interrupt-parent = <&intc>;
+ cpdma_channels = 8;
+ host_port_no = 0;
+ cpdma_reg_ofs = 0x800;
+ ale_reg_ofs = 0xd00;
+ ale_entries = 1024;
+ host_port_reg_ofs = 0x108;
+ hw_stats_reg_ofs = 0x900;
+ bd_ram_ofs = 0x2000;
+ bd_ram_size = 0x2000;
+ no_bd_ram = 0;
+ rx_descs = 64;
+ mac_control = 0x20;
+ slaves = 2;
+ slave@0 {
+ slave_reg_ofs = 0x208;
+ sliver_reg_ofs = 0xd80;
+ phy_id = "davinci_mdio-0:00"
+ mac-address = [00 04 9F 01 1B B8];
+ };
+ slave@1 {
+ slave_reg_ofs = 0x208;
+ sliver_reg_ofs = 0xd80;
+ phy_id = "davinci_mdio-0:01"
+ mac-address = [00 04 9F 01 1B B9];
+ };
+ };
+
+(or)
+
+ mac: ethernet@4A100000 {
+ compatible = "ti,cpsw";
+ ti,hwmods = "cpgmac0";
+ cpdma_channels = 8;
+ host_port_no = 0;
+ cpdma_reg_ofs = 0x800;
+ ale_reg_ofs = 0xd00;
+ ale_entries = 1024;
+ host_port_reg_ofs = 0x108;
+ hw_stats_reg_ofs = 0x900;
+ bd_ram_ofs = 0x2000;
+ bd_ram_size = 0x2000;
+ no_bd_ram = 0;
+ rx_descs = 64;
+ mac_control = 0x20;
+ slaves = 2;
+ slave@0 {
+ slave_reg_ofs = 0x208;
+ sliver_reg_ofs = 0xd80;
+ phy_id = "davinci_mdio-0:00"
+ mac-address = [00 04 9F 01 1B B8];
+ };
+ slave@1 {
+ slave_reg_ofs = 0x208;
+ sliver_reg_ofs = 0xd80;
+ phy_id = "davinci_mdio-0:01"
+ mac-address = [00 04 9F 01 1B B9];
+ };
+
+ };
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 1e5d85b..0cbc0e5 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -28,6 +28,9 @@
#include <linux/workqueue.h>
#include <linux/delay.h>
#include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/of_device.h>
#include <linux/platform_data/cpsw.h>
@@ -709,6 +712,158 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv)
slave->sliver = regs + data->sliver_reg_ofs;
}
+static int cpsw_probe_dt(struct cpsw_platform_data *data,
+ struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct device_node *slave_node;
+ int i = 0, ret;
+ u32 prop;
+
+ if (!node)
+ return -EINVAL;
+
+ if (of_property_read_u32(node, "slaves", &prop)) {
+ pr_err("Missing slaves property in the DT.\n");
+ return -EINVAL;
+ }
+ data->slaves = prop;
+
+ data->slave_data = kzalloc(sizeof(struct cpsw_slave_data) *
+ data->slaves, GFP_KERNEL);
+ if (!data->slave_data) {
+ pr_err("Could not allocate slave memory.\n");
+ return -EINVAL;
+ }
+
+ data->no_bd_ram = of_property_read_bool(node, "no_bd_ram");
+
+ if (of_property_read_u32(node, "cpdma_channels", &prop)) {
+ pr_err("Missing cpdma_channels property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->channels = prop;
+
+ if (of_property_read_u32(node, "host_port_no", &prop)) {
+ pr_err("Missing host_port_no property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->host_port_num = prop;
+
+ if (of_property_read_u32(node, "cpdma_reg_ofs", &prop)) {
+ pr_err("Missing cpdma_reg_ofs property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->cpdma_reg_ofs = prop;
+
+ if (of_property_read_u32(node, "cpdma_sram_ofs", &prop)) {
+ pr_err("Missing cpdma_sram_ofs property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->cpdma_sram_ofs = prop;
+
+ if (of_property_read_u32(node, "ale_reg_ofs", &prop)) {
+ pr_err("Missing ale_reg_ofs property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->ale_reg_ofs = prop;
+
+ if (of_property_read_u32(node, "ale_entries", &prop)) {
+ pr_err("Missing ale_entries property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->ale_entries = prop;
+
+ if (of_property_read_u32(node, "host_port_reg_ofs", &prop)) {
+ pr_err("Missing host_port_reg_ofs property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->host_port_reg_ofs = prop;
+
+ if (of_property_read_u32(node, "hw_stats_reg_ofs", &prop)) {
+ pr_err("Missing hw_stats_reg_ofs property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->hw_stats_reg_ofs = prop;
+
+ if (of_property_read_u32(node, "bd_ram_ofs", &prop)) {
+ pr_err("Missing bd_ram_ofs property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->bd_ram_ofs = prop;
+
+ if (of_property_read_u32(node, "bd_ram_size", &prop)) {
+ pr_err("Missing bd_ram_size property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->bd_ram_size = prop;
+
+ if (of_property_read_u32(node, "rx_descs", &prop)) {
+ pr_err("Missing rx_descs property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->rx_descs = prop;
+
+ if (of_property_read_u32(node, "mac_control", &prop)) {
+ pr_err("Missing mac_control property in the DT.\n");
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ data->mac_control = prop;
+
+ for_each_child_of_node(node, slave_node) {
+ struct cpsw_slave_data *slave_data = data->slave_data + i;
+ const char *phy_id = NULL;
+ const void *mac_addr = NULL;
+
+ if (of_property_read_string(slave_node, "phy_id", &phy_id)) {
+ pr_err("Missing slave[%d] phy_id property\n", i);
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ slave_data->phy_id = phy_id;
+
+ if (of_property_read_u32(slave_node, "slave_reg_ofs", &prop)) {
+ pr_err("Missing slave[%d] slave_reg_ofs property\n", i);
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ slave_data->slave_reg_ofs = prop;
+
+ if (of_property_read_u32(slave_node, "sliver_reg_ofs",
+ &prop)) {
+ pr_err("Missing slave[%d] sliver_reg_ofs property\n",
+ i);
+ ret = -EINVAL;
+ goto error_ret;
+ }
+ slave_data->sliver_reg_ofs = prop;
+
+ mac_addr = of_get_mac_address(slave_node);
+ if (mac_addr)
+ memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
+
+ i++;
+ }
+
+ return 0;
+
+error_ret:
+ kfree(data->slave_data);
+ return ret;
+}
+
static int __devinit cpsw_probe(struct platform_device *pdev)
{
struct cpsw_platform_data *data = pdev->dev.platform_data;
@@ -720,11 +875,6 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
struct resource *res;
int ret = 0, i, k = 0;
- if (!data) {
- pr_err("platform data missing\n");
- return -ENODEV;
- }
-
ndev = alloc_etherdev(sizeof(struct cpsw_priv));
if (!ndev) {
pr_err("error allocating net_device\n");
@@ -734,13 +884,19 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ndev);
priv = netdev_priv(ndev);
spin_lock_init(&priv->lock);
- priv->data = *data;
priv->pdev = pdev;
priv->ndev = ndev;
priv->dev = &ndev->dev;
priv->msg_enable = netif_msg_init(debug_level, CPSW_DEBUG);
priv->rx_packet_max = max(rx_packet_max, 128);
+ if (cpsw_probe_dt(&priv->data, pdev)) {
+ pr_err("cpsw: platform data missing\n");
+ ret = -ENODEV;
+ goto clean_ndev_ret;
+ }
+ data = &priv->data;
+
if (is_valid_ether_addr(data->slave_data[0].mac_addr)) {
memcpy(priv->mac_addr, data->slave_data[0].mac_addr, ETH_ALEN);
pr_info("Detected MACID = %pM", priv->mac_addr);
@@ -996,11 +1152,17 @@ static const struct dev_pm_ops cpsw_pm_ops = {
.resume = cpsw_resume,
};
+static const struct of_device_id cpsw_of_mtable[] = {
+ { .compatible = "ti,cpsw", },
+ { /* sentinel */ },
+};
+
static struct platform_driver cpsw_driver = {
.driver = {
.name = "cpsw",
.owner = THIS_MODULE,
.pm = &cpsw_pm_ops,
+ .of_match_table = of_match_ptr(cpsw_of_mtable),
},
.probe = cpsw_probe,
.remove = __devexit_p(cpsw_remove),
--
1.7.0.4
^ permalink raw reply related
* [PATCH 0/2] Add device tree support and resolving SOC dependency to cpsw driver
From: Mugunthan V N @ 2012-07-30 20:17 UTC (permalink / raw)
To: netdev; +Cc: davem, Mugunthan V N
This patch set adds SOC dependency for CPSW dependent modules and adds support
for device tree for CPSW driver
Mugunthan V N (2):
drivers: net: ethernet: cpsw: Add SOC dependency support for cpsw
dependent modules
drivers: net: ethernet: cpsw: Add device tree support to CPSW
Documentation/devicetree/bindings/net/cpsw.txt | 104 ++++++++++++++
drivers/net/ethernet/ti/Kconfig | 4 +-
drivers/net/ethernet/ti/cpsw.c | 174 +++++++++++++++++++++++-
3 files changed, 274 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/cpsw.txt
^ permalink raw reply
* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Greear @ 2012-07-30 21:00 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1343677270.2667.31.camel@bwh-desktop.uk.solarflarecom.com>
On 07/30/2012 12:41 PM, Ben Hutchings wrote:
> On Mon, 2012-07-30 at 10:23 -0700, Ben Greear wrote:
>> On 07/30/2012 10:16 AM, Ben Hutchings wrote:
>>> A peer (or local user) may cause TCP to use a nominal MSS of as little
>>> as 88 (actual MSS of 76 with timestamps). Given that we have a
>>> sufficiently prodigious local sender and the peer ACKs quickly enough,
>>> it is nevertheless possible to grow the window for such a connection
>>> to the point that we will try to send just under 64K at once. This
>>> results in a single skb that expands to 861 segments.
>>>
>>> In some drivers with TSO support, such an skb will require hundreds of
>>> DMA descriptors; a substantial fraction of a TX ring or even more than
>>> a full ring. The TX queue selected for the skb may stall and trigger
>>> the TX watchdog repeatedly (since the problem skb will be retried
>>> after the TX reset). This particularly affects sfc, for which the
>>> issue is designated as CVE-2012-3412. However it may be that some
>>> hardware or firmware also fails to handle such an extreme TSO request
>>> correctly.
>>>
>>> Therefore, limit the number of segments per skb to 100. This should
>>> make no difference to behaviour unless the actual MSS is less than
>>> about 700.
>>
>> Please do not do this...or at least allow over-rides. We love
>> the trick of seting very small MSS and making the NICs generate
>> huge numbers of small TCP frames with efficient user-space
>> logic. We use this for stateful TCP load testing when high
>> numbers of tcp packets-per-second is desired.
>
> Please test whether this actually makes a difference - my suspicion is
> that 100 segments per skb is easily enough to prevent the host being a
> bottleneck.
Any CPU I can save I can use for other tasks. If we can use the
NIC's offload features to segment pkts, then we get near linear
increase in pkts-per-second by adding NICs..at least up to whatever
the total bandwidth of the system is...
If you want to have the OS default to a safe value, that is
fine by me..but please give us a tunable so that we can get
the old behaviour.
It's always possible I'm not the only one using this,
and I think it would be considered bad form to break
existing features and provide no work-around.
Thanks,
Ben
>
>> Intel NICs, including 10G, work just fine with minimal MSS
>> in this scenario.
>
> I'll leave this to the Intel maintainers to answer.
>
> Ben.
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [PATCH net] net: Remove unused variables in rt_cache_stat
From: Vijay Subramanian @ 2012-07-30 21:45 UTC (permalink / raw)
To: netdev; +Cc: davem, Vijay Subramanian
With the removal of the routing cache, some variables in rt_cache_stat are no
longer used. Remove them from rt_cache_stat and do not print them out in
/proc/net/stat/rt_cache.
Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
checkpatch complains that the seq_printf line is over 80 chars which was already
the case. I left it as is to aid in grepping the sources.
include/net/route.h | 8 --------
net/ipv4/route.c | 16 +++-------------
2 files changed, 3 insertions(+), 21 deletions(-)
diff --git a/include/net/route.h b/include/net/route.h
index 8c52bc6..69e54f9 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -84,22 +84,14 @@ struct ip_rt_acct {
};
struct rt_cache_stat {
- unsigned int in_hit;
unsigned int in_slow_tot;
unsigned int in_slow_mc;
unsigned int in_no_route;
unsigned int in_brd;
unsigned int in_martian_dst;
unsigned int in_martian_src;
- unsigned int out_hit;
unsigned int out_slow_tot;
unsigned int out_slow_mc;
- unsigned int gc_total;
- unsigned int gc_ignored;
- unsigned int gc_goal_miss;
- unsigned int gc_dst_overflow;
- unsigned int in_hlist_search;
- unsigned int out_hlist_search;
};
extern struct ip_rt_acct __percpu *ip_rt_acct;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index fc1a81c..114a6c9 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -298,14 +298,12 @@ static int rt_cpu_seq_show(struct seq_file *seq, void *v)
struct rt_cache_stat *st = v;
if (v == SEQ_START_TOKEN) {
- seq_printf(seq, "entries in_hit in_slow_tot in_slow_mc in_no_route in_brd in_martian_dst in_martian_src out_hit out_slow_tot out_slow_mc gc_total gc_ignored gc_goal_miss gc_dst_overflow in_hlist_search out_hlist_search\n");
+ seq_printf(seq, "entries in_slow_tot in_slow_mc in_no_route in_brd in_martian_dst in_martian_src out_slow_tot out_slow_mc\n");
return 0;
}
- seq_printf(seq,"%08x %08x %08x %08x %08x %08x %08x %08x "
- " %08x %08x %08x %08x %08x %08x %08x %08x %08x \n",
+ seq_printf(seq, "%08x %08x %08x %08x %08x %08x %08x %08x %08x\n",
dst_entries_get_slow(&ipv4_dst_ops),
- st->in_hit,
st->in_slow_tot,
st->in_slow_mc,
st->in_no_route,
@@ -313,16 +311,8 @@ static int rt_cpu_seq_show(struct seq_file *seq, void *v)
st->in_martian_dst,
st->in_martian_src,
- st->out_hit,
st->out_slow_tot,
- st->out_slow_mc,
-
- st->gc_total,
- st->gc_ignored,
- st->gc_goal_miss,
- st->gc_dst_overflow,
- st->in_hlist_search,
- st->out_hlist_search
+ st->out_slow_mc
);
return 0;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: David Miller @ 2012-07-30 21:46 UTC (permalink / raw)
To: bhutchings; +Cc: eric.dumazet, netdev, linux-net-drivers
In-Reply-To: <1343676952.2667.26.camel@bwh-desktop.uk.solarflarecom.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 30 Jul 2012 20:35:52 +0100
> On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
>> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
>> driver sets to whatever limit ?
>
> Yes, that's another option.
This is how I want this handled.
^ permalink raw reply
* Re: [PATCH net] net: Remove unused variables in rt_cache_stat
From: David Miller @ 2012-07-30 21:52 UTC (permalink / raw)
To: subramanian.vijay; +Cc: netdev
In-Reply-To: <1343684750-2987-1-git-send-email-subramanian.vijay@gmail.com>
From: Vijay Subramanian <subramanian.vijay@gmail.com>
Date: Mon, 30 Jul 2012 14:45:50 -0700
> With the removal of the routing cache, some variables in rt_cache_stat are no
> longer used. Remove them from rt_cache_stat and do not print them out in
> /proc/net/stat/rt_cache.
>
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> ---
> checkpatch complains that the seq_printf line is over 80 chars which was already
> the case. I left it as is to aid in grepping the sources.
You cannot make this change, these fields are exported via procfs and
therefore you will break any application that is parsing the existing
layout.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox