* Re: [RFC v2 2/2] net: Add support for NTB virtual ethernet device
From: Jiri Pirko @ 2012-07-30 14:02 UTC (permalink / raw)
To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <1343607994-32415-3-git-send-email-jon.mason@intel.com>
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.
^ permalink raw reply
* [PATCH] net: batch nf_conntrack_net_exit
From: Vladimir Davydov @ 2012-07-30 15:16 UTC (permalink / raw)
To: David S. Miller; +Cc: Vladimir Davydov, netdev, linux-kernel
The patch introduces nf_conntrack_cleanup_list(), which cleanups
nf_conntracks for a list of netns and calls synchronize_net() only
once for them all.
---
include/net/netfilter/nf_conntrack_core.h | 10 +++++++++-
net/netfilter/nf_conntrack_core.c | 21 +++++++++++++--------
net/netfilter/nf_conntrack_standalone.c | 14 +++++++++-----
3 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index d8f5b9f..f53b855 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -26,7 +26,15 @@ extern unsigned int nf_conntrack_in(struct net *net,
struct sk_buff *skb);
extern int nf_conntrack_init(struct net *net);
-extern void nf_conntrack_cleanup(struct net *net);
+extern void nf_conntrack_cleanup_list(struct list_head *net_exit_list);
+
+static inline void nf_conntrack_cleanup(struct net *net)
+{
+ LIST_HEAD(single);
+
+ list_add(&net->exit_list, &single);
+ nf_conntrack_cleanup_list(&single);
+}
extern int nf_conntrack_proto_init(struct net *net);
extern void nf_conntrack_proto_fini(struct net *net);
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index cf48755..afa62f7 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1363,21 +1363,26 @@ static void nf_conntrack_cleanup_net(struct net *net)
/* Mishearing the voices in his head, our hero wonders how he's
supposed to kill the mall. */
-void nf_conntrack_cleanup(struct net *net)
+void nf_conntrack_cleanup_list(struct list_head *net_exit_list)
{
- if (net_eq(net, &init_net))
- RCU_INIT_POINTER(ip_ct_attach, NULL);
+ struct net *net;
+
+ list_for_each_entry(net, net_exit_list, exit_list)
+ if (net_eq(net, &init_net))
+ RCU_INIT_POINTER(ip_ct_attach, NULL);
/* This makes sure all current packets have passed through
netfilter framework. Roll on, two-stage module
delete... */
synchronize_net();
- nf_conntrack_proto_fini(net);
- nf_conntrack_cleanup_net(net);
- if (net_eq(net, &init_net)) {
- RCU_INIT_POINTER(nf_ct_destroy, NULL);
- nf_conntrack_cleanup_init_net();
+ list_for_each_entry(net, net_exit_list, exit_list) {
+ nf_conntrack_proto_fini(net);
+ nf_conntrack_cleanup_net(net);
+ if (net_eq(net, &init_net)) {
+ RCU_INIT_POINTER(nf_ct_destroy, NULL);
+ nf_conntrack_cleanup_init_net();
+ }
}
}
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 9b39432..8f60708 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -551,16 +551,20 @@ out_init:
return ret;
}
-static void nf_conntrack_net_exit(struct net *net)
+static void nf_conntrack_net_exit(struct list_head *net_exit_list)
{
- nf_conntrack_standalone_fini_sysctl(net);
- nf_conntrack_standalone_fini_proc(net);
- nf_conntrack_cleanup(net);
+ struct net *net;
+
+ list_for_each_entry(net, net_exit_list, exit_list) {
+ nf_conntrack_standalone_fini_sysctl(net);
+ nf_conntrack_standalone_fini_proc(net);
+ }
+ nf_conntrack_cleanup_list(net_exit_list);
}
static struct pernet_operations nf_conntrack_net_ops = {
.init = nf_conntrack_net_init,
- .exit = nf_conntrack_net_exit,
+ .exit_batch = nf_conntrack_net_exit,
};
static int __init nf_conntrack_standalone_init(void)
--
1.7.1
^ permalink raw reply related
* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: Bjorn Helgaas @ 2012-07-30 16:50 UTC (permalink / raw)
To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <1343607994-32415-2-git-send-email-jon.mason@intel.com>
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.
^ permalink raw reply
* [PATCH] ipv4: remove rt_cache_rebuild_count
From: Eric Dumazet @ 2012-07-30 17:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
After IP route cache removal, rt_cache_rebuild_count is no longer
used.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Documentation/networking/ip-sysctl.txt | 6 ------
include/net/netns/ipv4.h | 2 --
net/ipv4/sysctl_net_ipv4.c | 11 -----------
3 files changed, 19 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 406a522..ca447b3 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -48,12 +48,6 @@ min_adv_mss - INTEGER
The advertised MSS depends on the first hop route MTU, but will
never be lower than this setting.
-rt_cache_rebuild_count - INTEGER
- The per net-namespace route cache emergency rebuild threshold.
- Any net-namespace having its route cache rebuilt due to
- a hash bucket chain being too long more than this many times
- will have its route caching disabled
-
IP Fragmentation:
ipfrag_high_thresh - INTEGER
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 0ffb8e3..1474dd6 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -61,8 +61,6 @@ struct netns_ipv4 {
int sysctl_icmp_ratelimit;
int sysctl_icmp_ratemask;
int sysctl_icmp_errors_use_inbound_ifaddr;
- int sysctl_rt_cache_rebuild_count;
- int current_rt_cache_rebuild_count;
unsigned int sysctl_ping_group_range[2];
long sysctl_tcp_mem[3];
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 5840c32..4b6487a 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -784,13 +784,6 @@ static struct ctl_table ipv4_net_table[] = {
.proc_handler = proc_dointvec
},
{
- .procname = "rt_cache_rebuild_count",
- .data = &init_net.ipv4.sysctl_rt_cache_rebuild_count,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec
- },
- {
.procname = "ping_group_range",
.data = &init_net.ipv4.sysctl_ping_group_range,
.maxlen = sizeof(init_net.ipv4.sysctl_ping_group_range),
@@ -829,8 +822,6 @@ static __net_init int ipv4_sysctl_init_net(struct net *net)
table[5].data =
&net->ipv4.sysctl_icmp_ratemask;
table[6].data =
- &net->ipv4.sysctl_rt_cache_rebuild_count;
- table[7].data =
&net->ipv4.sysctl_ping_group_range;
}
@@ -842,8 +833,6 @@ static __net_init int ipv4_sysctl_init_net(struct net *net)
net->ipv4.sysctl_ping_group_range[0] = 1;
net->ipv4.sysctl_ping_group_range[1] = 0;
- net->ipv4.sysctl_rt_cache_rebuild_count = 4;
-
tcp_init_mem(net);
net->ipv4.ipv4_hdr = register_net_sysctl(net, "net/ipv4", table);
^ permalink raw reply related
* [PATCH net 0/2] Prevent extreme TSO parameters from stalling TX queues
From: Ben Hutchings @ 2012-07-30 17:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
The following changes fix a potential DoS by peers or local users on
network interfaces using the sfc driver (and possibly others) with TSO
enabled (as it is by default). Please apply them to the net tree and
your stable update queue.
Ben.
Ben Hutchings (2):
tcp: Limit number of segments generated by GSO per skb
sfc: Correct the minimum TX queue size
drivers/net/ethernet/sfc/efx.c | 5 +++++
drivers/net/ethernet/sfc/efx.h | 11 +++++++----
drivers/net/ethernet/sfc/ethtool.c | 16 +++++++++++-----
drivers/net/ethernet/sfc/tx.c | 20 ++++++++++++++++++++
include/net/tcp.h | 3 +++
net/ipv4/tcp.c | 4 +++-
net/ipv4/tcp_output.c | 17 +++++++++--------
7 files changed, 58 insertions(+), 18 deletions(-)
--
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
* [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 17:16 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1343668498.2667.5.camel@bwh-desktop.uk.solarflarecom.com>
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>
---
include/net/tcp.h | 3 +++
net/ipv4/tcp.c | 4 +++-
net/ipv4/tcp_output.c | 17 +++++++++--------
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index e19124b..098a2d0 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -70,6 +70,9 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
/* The least MTU to use for probing */
#define TCP_BASE_MSS 512
+/* Maximum number of segments we may require GSO to generate from an skb. */
+#define TCP_MAX_GSO_SEGS 100
+
/* After receiving this amount of duplicate ACKs fast retransmit starts. */
#define TCP_FASTRETRANS_THRESH 3
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e7e6eea..51d8daf 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -811,7 +811,9 @@ static unsigned int tcp_xmit_size_goal(struct sock *sk, u32 mss_now,
old_size_goal + mss_now > xmit_size_goal)) {
xmit_size_goal = old_size_goal;
} else {
- tp->xmit_size_goal_segs = xmit_size_goal / mss_now;
+ tp->xmit_size_goal_segs =
+ min_t(u32, xmit_size_goal / mss_now,
+ TCP_MAX_GSO_SEGS);
xmit_size_goal = tp->xmit_size_goal_segs * mss_now;
}
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 33cd065..c86c288 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1522,21 +1522,21 @@ static void tcp_cwnd_validate(struct sock *sk)
* when we would be allowed to send the split-due-to-Nagle skb fully.
*/
static unsigned int tcp_mss_split_point(const struct sock *sk, const struct sk_buff *skb,
- unsigned int mss_now, unsigned int cwnd)
+ unsigned int mss_now, unsigned int max_segs)
{
const struct tcp_sock *tp = tcp_sk(sk);
- u32 needed, window, cwnd_len;
+ u32 needed, window, max_len;
window = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
- cwnd_len = mss_now * cwnd;
+ max_len = mss_now * max_segs;
- if (likely(cwnd_len <= window && skb != tcp_write_queue_tail(sk)))
- return cwnd_len;
+ if (likely(max_len <= window && skb != tcp_write_queue_tail(sk)))
+ return max_len;
needed = min(skb->len, window);
- if (cwnd_len <= needed)
- return cwnd_len;
+ if (max_len <= needed)
+ return max_len;
return needed - needed % mss_now;
}
@@ -1999,7 +1999,8 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
limit = mss_now;
if (tso_segs > 1 && !tcp_urg_mode(tp))
limit = tcp_mss_split_point(sk, skb, mss_now,
- cwnd_quota);
+ min(cwnd_quota,
+ TCP_MAX_GSO_SEGS));
if (skb->len > limit &&
unlikely(tso_fragment(sk, skb, limit, mss_now, gfp)))
--
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
* [PATCH net 2/2] sfc: Correct the minimum TX queue size
From: Ben Hutchings @ 2012-07-30 17:17 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1343668498.2667.5.camel@bwh-desktop.uk.solarflarecom.com>
Currently an skb requiring TSO may not fit within a minimum-size TX
queue. 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 issue is designated as CVE-2012-3412.
The preceding change 'tcp: Limit number of segments generated by GSO
per skb' fixed this for the default queue size. Increase the minimum
to allow for 2 worst-case skbs, such that there will definitely be
space to add an skb after we wake a queue.
To avoid invalidating existing configurations, change
efx_ethtool_set_ringparam() to fix up values that are too small rather
than returning -EINVAL.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 5 +++++
drivers/net/ethernet/sfc/efx.h | 11 +++++++----
drivers/net/ethernet/sfc/ethtool.c | 16 +++++++++++-----
drivers/net/ethernet/sfc/tx.c | 20 ++++++++++++++++++++
4 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 70554a1..a5b5a75 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1503,6 +1503,11 @@ static int efx_probe_all(struct efx_nic *efx)
goto fail2;
}
+ BUILD_BUG_ON(EFX_DEFAULT_DMAQ_SIZE < EFX_RXQ_MIN_ENT);
+ if (WARN_ON(EFX_DEFAULT_DMAQ_SIZE < EFX_TXQ_MIN_ENT(efx))) {
+ rc = -EINVAL;
+ goto fail3;
+ }
efx->rxq_entries = efx->txq_entries = EFX_DEFAULT_DMAQ_SIZE;
rc = efx_probe_filters(efx);
diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
index be8f915..d95309c 100644
--- a/drivers/net/ethernet/sfc/efx.h
+++ b/drivers/net/ethernet/sfc/efx.h
@@ -30,6 +30,7 @@ extern netdev_tx_t
efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb);
extern void efx_xmit_done(struct efx_tx_queue *tx_queue, unsigned int index);
extern int efx_setup_tc(struct net_device *net_dev, u8 num_tc);
+extern unsigned int efx_tx_max_skb_descs(struct efx_nic *efx);
/* RX */
extern int efx_probe_rx_queue(struct efx_rx_queue *rx_queue);
@@ -52,10 +53,12 @@ extern void efx_schedule_slow_fill(struct efx_rx_queue *rx_queue);
#define EFX_MAX_EVQ_SIZE 16384UL
#define EFX_MIN_EVQ_SIZE 512UL
-/* The smallest [rt]xq_entries that the driver supports. Callers of
- * efx_wake_queue() assume that they can subsequently send at least one
- * skb. Falcon/A1 may require up to three descriptors per skb_frag. */
-#define EFX_MIN_RING_SIZE (roundup_pow_of_two(2 * 3 * MAX_SKB_FRAGS))
+/* The smallest [rt]xq_entries that the driver supports. RX minimum
+ * is a bit arbitrary. For TX, we must have space for at least 2
+ * TSO skbs.
+ */
+#define EFX_RXQ_MIN_ENT 128U
+#define EFX_TXQ_MIN_ENT(efx) (2 * efx_tx_max_skb_descs(efx))
/* Filters */
extern int efx_probe_filters(struct efx_nic *efx);
diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index 10536f9..8cba2df 100644
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -680,21 +680,27 @@ static int efx_ethtool_set_ringparam(struct net_device *net_dev,
struct ethtool_ringparam *ring)
{
struct efx_nic *efx = netdev_priv(net_dev);
+ u32 txq_entries;
if (ring->rx_mini_pending || ring->rx_jumbo_pending ||
ring->rx_pending > EFX_MAX_DMAQ_SIZE ||
ring->tx_pending > EFX_MAX_DMAQ_SIZE)
return -EINVAL;
- if (ring->rx_pending < EFX_MIN_RING_SIZE ||
- ring->tx_pending < EFX_MIN_RING_SIZE) {
+ if (ring->rx_pending < EFX_RXQ_MIN_ENT) {
netif_err(efx, drv, efx->net_dev,
- "TX and RX queues cannot be smaller than %ld\n",
- EFX_MIN_RING_SIZE);
+ "RX queues cannot be smaller than %u\n",
+ EFX_RXQ_MIN_ENT);
return -EINVAL;
}
- return efx_realloc_channels(efx, ring->rx_pending, ring->tx_pending);
+ txq_entries = max(ring->tx_pending, EFX_TXQ_MIN_ENT(efx));
+ if (txq_entries != ring->tx_pending)
+ netif_warn(efx, drv, efx->net_dev,
+ "increasing TX queue size to minimum of %u\n",
+ txq_entries);
+
+ return efx_realloc_channels(efx, ring->rx_pending, txq_entries);
}
static int efx_ethtool_set_pauseparam(struct net_device *net_dev,
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index 9b225a7..814bd17 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -15,6 +15,7 @@
#include <linux/ipv6.h>
#include <linux/slab.h>
#include <net/ipv6.h>
+#include <net/tcp.h>
#include <linux/if_ether.h>
#include <linux/highmem.h>
#include "net_driver.h"
@@ -119,6 +120,25 @@ efx_max_tx_len(struct efx_nic *efx, dma_addr_t dma_addr)
return len;
}
+unsigned int efx_tx_max_skb_descs(struct efx_nic *efx)
+{
+ /* Header and payload descriptor for each output segment, plus
+ * one for every input fragment boundary within a segment
+ */
+ unsigned int max_descs = TCP_MAX_GSO_SEGS * 2 + MAX_SKB_FRAGS;
+
+ /* Possibly one more per segment for the alignment workaround */
+ if (EFX_WORKAROUND_5391(efx))
+ max_descs += TCP_MAX_GSO_SEGS;
+
+ /* Possibly more for PCIe page boundaries within input fragments */
+ if (PAGE_SIZE > EFX_PAGE_SIZE)
+ max_descs += max_t(unsigned int, MAX_SKB_FRAGS,
+ DIV_ROUND_UP(GSO_MAX_SIZE, EFX_PAGE_SIZE));
+
+ return max_descs;
+}
+
/*
* Add a socket buffer to a TX queue
*
--
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 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
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