* Re: [PATCH][V2] Freescale Frame Manager Device Tree binding document
From: Scott Wood @ 2014-09-29 23:33 UTC (permalink / raw)
To: Bob Cochran
Cc: Igal.Liberman, netdev, linuxppc-dev, Emilian.Medve, devicetree
In-Reply-To: <5425962F.7090602@mindchasers.com>
On Fri, 2014-09-26 at 12:37 -0400, Bob Cochran wrote:
> On 09/17/2014 07:08 AM, Igal.Liberman wrote:
> > From: Igal Liberman <Igal.Liberman@freescale.com>
> >
> > The Frame Manager (FMan) combines the Ethernet network interfaces with packet
> > distribution logic to provide intelligent distribution and queuing decisions
> > for incoming traffic at line rate.
> >
> > This binding document describes Freescale's Frame Manager hardware attributes
> > that are used by the Frame Manager driver for its basic initialization and
> > configuration.
> >
> > Difference between [V1] and [V2]:
> > Addressed all comments recieved from Scott in [V1]
> >
> > Signed-off-by: Igal Liberman <Igal.Liberman@freescale.com>
>
>
> Is there a reference (FMAN) dts file I can compare this against? I was
> reviewing this against my (combined) dts file for my t1040rdb built with
> either FSL's SDK1.6 or Yocto Project meta-fsl-ppc master branch, and I'm
> not finding a good match between the nodes / properties in my dts file
> and what you have here.
>
> For example, I don't find the required "clocks" or
> "fsl,qman-channel-range" properties under my <fman0: fman@400000 ...>
> node.
>
> I'm wondering if this document is trailing or leading the source in the
> latest published FSL SDK (1.6).
The SDK's device tree binding was not considered suitable and we've
started from scratch for upstream.
-Scott
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: VRFs and the scalability of namespaces
From: David Ahern @ 2014-09-29 23:43 UTC (permalink / raw)
To: Ben Greear, Sowmini Varadhan
Cc: Hannes Frederic Sowa, Eric W. Biederman, Nicolas Dichtel, netdev
In-Reply-To: <54299019.3050604@candelatech.com>
On 9/29/14, 11:00 AM, Ben Greear wrote:
> On 09/29/2014 09:50 AM, Sowmini Varadhan wrote:
>> On Mon, Sep 29, 2014 at 12:40 PM, Ben Greear <greearb@candelatech.com> wrote:
>>> On 09/29/2014 06:06 AM, David Ahern wrote:
>>
>>>
>>> We have implemented support for at least most of this (excepting duplicate IPs)
>>> using routing tables, rules, and (optionally, xorp as the router).
>>>
>>
>> My undertanding of multiple routing-tables/rules was that they
>> are closer in semantics to switch/router ACLs than to VRFs, eg.,
>> one big difference is that an interface can belong to exactly one
>> VRF at a time, which is not mandated by multiple routing-tables/rules.
>>
>> Was I mistaken?
>
> You can effectively force an interface to belong to a particular virtual
> router (table). It is not trivial to do, and possibly I have still not
> covered every possible case. Some rules grow somewhat exponentially as
> interfaces are added to virtual routers (ie, preference 10 rules).
An interesting way of doing it; thanks for the reference point.
Fundamentally the design should be able to assign interfaces to a single
VRF, support duplicate IP addresses on different interfaces in different
VRFs and be able to scale to 10,000+ netdevices -- devices representing
physical ports as well as logical interfaces built on top of them (e.g.,
sub-interfaces).
David
^ permalink raw reply
* [PATCHv9 net-next 0/4] sunvnet: add jumbo frames support
From: David L Stevens @ 2014-09-29 23:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Sowmini Varadhan, Raghuram Kothahota
This patch set updates the sunvnet driver to version 1.6 of the VIO protocol
to support per-port exchange of MTU information and allow non-standard MTU
sizes, including jumbo frames.
Using large MTUs shows a nearly 5X throughput improvement Linux-Solaris
and > 10X throughput improvement Linux-Linux.
Changes from v8:
-add a short timeout to free pending skbs if a new transmit doesn't
do it first per Dave Miller <davem@davemloft.net>
Changes from v7:
-handle skb allocation failures in vnet_skb_shape()
per Dave Miller <davem@davemloft.net>
Changes from v6:
-made kernel transmit path zero-copy to remove memory n^2 scaling issue
raised by Raghuram Kothakota <Raghuram.Kothakota@oracle.com>
Changes from v5:
- fixed comment per Sowmini Varadhan <sowmini.varadhan@oracle.com>
Changes from v4:
- changed VNET_MAXPACKET per David Laight <David.Laight@ACULAB.COM>
- added cookies to support non-contiguous buffers of max size
Changes from v3:
- added version functions per Dave Miller <davem@davemloft.net>
- moved rmtu to vnet_port per Dave Miller <davem@davemloft.net>
- explicitly set options bits and capability flags to 0 per
Raghuram Kothakota <Raghuram.Kothakota@oracle.com>
Changes from v2:
- make checkpatch clean
Changes from v1:
- fix brace formatting per Dave Miller <davem@davemloft.net>
David L Stevens (4):
sunvnet: upgrade to VIO protocol version 1.6
sunvnet: make transmit path zero-copy in the kernel
sunvnet: allow admin to set sunvnet MTU
sunvnet: generate ICMP PTMUD messages for smaller port MTUs
arch/sparc/include/asm/vio.h | 44 +++++-
arch/sparc/kernel/ldc.c | 2 +-
arch/sparc/kernel/viohs.c | 14 ++-
drivers/net/ethernet/sun/sunvnet.c | 358 ++++++++++++++++++++++++++++++------
drivers/net/ethernet/sun/sunvnet.h | 16 ++-
5 files changed, 366 insertions(+), 68 deletions(-)
^ permalink raw reply
* [PATCHv9 net-next 1/4] sunvnet: upgrade to VIO protocol version 1.6
From: David L Stevens @ 2014-09-29 23:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Sowmini Varadhan, Raghuram Kothahota
This patch upgrades the sunvnet driver to support VIO protocol version 1.6.
In particular, it adds per-port MTU negotiation, allowing MTUs other than
ETH_FRAMELEN with ports using newer VIO protocol versions.
Signed-off-by: David L Stevens <david.stevens@oracle.com>
---
arch/sparc/include/asm/vio.h | 44 ++++++++++++++-
arch/sparc/kernel/viohs.c | 14 ++++-
drivers/net/ethernet/sun/sunvnet.c | 104 +++++++++++++++++++++++++++++------
drivers/net/ethernet/sun/sunvnet.h | 3 +
4 files changed, 143 insertions(+), 22 deletions(-)
diff --git a/arch/sparc/include/asm/vio.h b/arch/sparc/include/asm/vio.h
index 5c0ebe7..107d4a4 100644
--- a/arch/sparc/include/asm/vio.h
+++ b/arch/sparc/include/asm/vio.h
@@ -65,6 +65,7 @@ struct vio_dring_register {
u16 options;
#define VIO_TX_DRING 0x0001
#define VIO_RX_DRING 0x0002
+#define VIO_RX_DRING_DATA 0x0004
u16 resv;
u32 num_cookies;
struct ldc_trans_cookie cookies[0];
@@ -80,6 +81,8 @@ struct vio_dring_unregister {
#define VIO_PKT_MODE 0x01 /* Packet based transfer */
#define VIO_DESC_MODE 0x02 /* In-band descriptors */
#define VIO_DRING_MODE 0x03 /* Descriptor rings */
+/* in vers >= 1.2, VIO_DRING_MODE is 0x04 and transfer mode is a bitmask */
+#define VIO_NEW_DRING_MODE 0x04
struct vio_dring_data {
struct vio_msg_tag tag;
@@ -209,10 +212,20 @@ struct vio_net_attr_info {
u8 addr_type;
#define VNET_ADDR_ETHERMAC 0x01
u16 ack_freq;
- u32 resv1;
+ u8 plnk_updt;
+#define PHYSLINK_UPDATE_NONE 0x00
+#define PHYSLINK_UPDATE_STATE 0x01
+#define PHYSLINK_UPDATE_STATE_ACK 0x02
+#define PHYSLINK_UPDATE_STATE_NACK 0x03
+ u8 options;
+ u16 resv1;
u64 addr;
u64 mtu;
- u64 resv2[3];
+ u16 cflags;
+#define VNET_LSO_IPV4_CAPAB 0x0001
+ u16 ipv4_lso_maxlen;
+ u32 resv2;
+ u64 resv3[2];
};
#define VNET_NUM_MCAST 7
@@ -370,6 +383,33 @@ struct vio_driver_state {
struct vio_driver_ops *ops;
};
+static inline bool vio_version_before(struct vio_driver_state *vio,
+ u16 major, u16 minor)
+{
+ u32 have = (u32)vio->ver.major << 16 | vio->ver.minor;
+ u32 want = (u32)major << 16 | minor;
+
+ return have < want;
+}
+
+static inline bool vio_version_after(struct vio_driver_state *vio,
+ u16 major, u16 minor)
+{
+ u32 have = (u32)vio->ver.major << 16 | vio->ver.minor;
+ u32 want = (u32)major << 16 | minor;
+
+ return have > want;
+}
+
+static inline bool vio_version_after_eq(struct vio_driver_state *vio,
+ u16 major, u16 minor)
+{
+ u32 have = (u32)vio->ver.major << 16 | vio->ver.minor;
+ u32 want = (u32)major << 16 | minor;
+
+ return have >= want;
+}
+
#define viodbg(TYPE, f, a...) \
do { if (vio->debug & VIO_DEBUG_##TYPE) \
printk(KERN_INFO "vio: ID[%lu] " f, \
diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
index f8e7dd5..7ef081a 100644
--- a/arch/sparc/kernel/viohs.c
+++ b/arch/sparc/kernel/viohs.c
@@ -426,6 +426,13 @@ static int process_dreg_info(struct vio_driver_state *vio,
if (vio->dr_state & VIO_DR_STATE_RXREG)
goto send_nack;
+ /* v1.6 and higher, ACK with desired, supported mode, or NACK */
+ if (vio_version_after_eq(vio, 1, 6)) {
+ if (!(pkt->options & VIO_TX_DRING))
+ goto send_nack;
+ pkt->options = VIO_TX_DRING;
+ }
+
BUG_ON(vio->desc_buf);
vio->desc_buf = kzalloc(pkt->descr_size, GFP_ATOMIC);
@@ -453,8 +460,11 @@ static int process_dreg_info(struct vio_driver_state *vio,
pkt->tag.stype = VIO_SUBTYPE_ACK;
pkt->dring_ident = ++dr->ident;
- viodbg(HS, "SEND DRING_REG ACK ident[%llx]\n",
- (unsigned long long) pkt->dring_ident);
+ viodbg(HS, "SEND DRING_REG ACK ident[%llx] "
+ "ndesc[%u] dsz[%u] opt[0x%x] ncookies[%u]\n",
+ (unsigned long long) pkt->dring_ident,
+ pkt->num_descr, pkt->descr_size, pkt->options,
+ pkt->num_cookies);
len = (sizeof(*pkt) +
(dr->ncookies * sizeof(struct ldc_trans_cookie)));
diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index edb8609..b1abcad 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -15,6 +15,7 @@
#include <linux/ethtool.h>
#include <linux/etherdevice.h>
#include <linux/mutex.h>
+#include <linux/if_vlan.h>
#include <asm/vio.h>
#include <asm/ldc.h>
@@ -41,6 +42,7 @@ static int __vnet_tx_trigger(struct vnet_port *port, u32 start);
/* Ordered from largest major to lowest */
static struct vio_version vnet_versions[] = {
+ { .major = 1, .minor = 6 },
{ .major = 1, .minor = 0 },
};
@@ -67,6 +69,7 @@ static int vnet_send_attr(struct vio_driver_state *vio)
struct vnet_port *port = to_vnet_port(vio);
struct net_device *dev = port->vp->dev;
struct vio_net_attr_info pkt;
+ int framelen = ETH_FRAME_LEN;
int i;
memset(&pkt, 0, sizeof(pkt));
@@ -74,19 +77,41 @@ static int vnet_send_attr(struct vio_driver_state *vio)
pkt.tag.stype = VIO_SUBTYPE_INFO;
pkt.tag.stype_env = VIO_ATTR_INFO;
pkt.tag.sid = vio_send_sid(vio);
- pkt.xfer_mode = VIO_DRING_MODE;
+ if (vio_version_before(vio, 1, 2))
+ pkt.xfer_mode = VIO_DRING_MODE;
+ else
+ pkt.xfer_mode = VIO_NEW_DRING_MODE;
pkt.addr_type = VNET_ADDR_ETHERMAC;
pkt.ack_freq = 0;
for (i = 0; i < 6; i++)
pkt.addr |= (u64)dev->dev_addr[i] << ((5 - i) * 8);
- pkt.mtu = ETH_FRAME_LEN;
+ if (vio_version_after(vio, 1, 3)) {
+ if (port->rmtu) {
+ port->rmtu = min(VNET_MAXPACKET, port->rmtu);
+ pkt.mtu = port->rmtu;
+ } else {
+ port->rmtu = VNET_MAXPACKET;
+ pkt.mtu = port->rmtu;
+ }
+ if (vio_version_after_eq(vio, 1, 6))
+ pkt.options = VIO_TX_DRING;
+ } else if (vio_version_before(vio, 1, 3)) {
+ pkt.mtu = framelen;
+ } else { /* v1.3 */
+ pkt.mtu = framelen + VLAN_HLEN;
+ }
+
+ pkt.plnk_updt = PHYSLINK_UPDATE_NONE;
+ pkt.cflags = 0;
viodbg(HS, "SEND NET ATTR xmode[0x%x] atype[0x%x] addr[%llx] "
- "ackfreq[%u] mtu[%llu]\n",
+ "ackfreq[%u] plnk_updt[0x%02x] opts[0x%02x] mtu[%llu] "
+ "cflags[0x%04x] lso_max[%u]\n",
pkt.xfer_mode, pkt.addr_type,
- (unsigned long long) pkt.addr,
- pkt.ack_freq,
- (unsigned long long) pkt.mtu);
+ (unsigned long long)pkt.addr,
+ pkt.ack_freq, pkt.plnk_updt, pkt.options,
+ (unsigned long long)pkt.mtu, pkt.cflags, pkt.ipv4_lso_maxlen);
+
return vio_ldc_send(vio, &pkt, sizeof(pkt));
}
@@ -94,18 +119,52 @@ static int vnet_send_attr(struct vio_driver_state *vio)
static int handle_attr_info(struct vio_driver_state *vio,
struct vio_net_attr_info *pkt)
{
- viodbg(HS, "GOT NET ATTR INFO xmode[0x%x] atype[0x%x] addr[%llx] "
- "ackfreq[%u] mtu[%llu]\n",
+ struct vnet_port *port = to_vnet_port(vio);
+ u64 localmtu;
+ u8 xfer_mode;
+
+ viodbg(HS, "GOT NET ATTR xmode[0x%x] atype[0x%x] addr[%llx] "
+ "ackfreq[%u] plnk_updt[0x%02x] opts[0x%02x] mtu[%llu] "
+ " (rmtu[%llu]) cflags[0x%04x] lso_max[%u]\n",
pkt->xfer_mode, pkt->addr_type,
- (unsigned long long) pkt->addr,
- pkt->ack_freq,
- (unsigned long long) pkt->mtu);
+ (unsigned long long)pkt->addr,
+ pkt->ack_freq, pkt->plnk_updt, pkt->options,
+ (unsigned long long)pkt->mtu, port->rmtu, pkt->cflags,
+ pkt->ipv4_lso_maxlen);
pkt->tag.sid = vio_send_sid(vio);
- if (pkt->xfer_mode != VIO_DRING_MODE ||
+ xfer_mode = pkt->xfer_mode;
+ /* for version < 1.2, VIO_DRING_MODE = 0x3 and no bitmask */
+ if (vio_version_before(vio, 1, 2) && xfer_mode == VIO_DRING_MODE)
+ xfer_mode = VIO_NEW_DRING_MODE;
+
+ /* MTU negotiation:
+ * < v1.3 - ETH_FRAME_LEN exactly
+ * > v1.3 - MIN(pkt.mtu, VNET_MAXPACKET, port->rmtu) and change
+ * pkt->mtu for ACK
+ * = v1.3 - ETH_FRAME_LEN + VLAN_HLEN exactly
+ */
+ if (vio_version_before(vio, 1, 3)) {
+ localmtu = ETH_FRAME_LEN;
+ } else if (vio_version_after(vio, 1, 3)) {
+ localmtu = port->rmtu ? port->rmtu : VNET_MAXPACKET;
+ localmtu = min(pkt->mtu, localmtu);
+ pkt->mtu = localmtu;
+ } else { /* v1.3 */
+ localmtu = ETH_FRAME_LEN + VLAN_HLEN;
+ }
+ port->rmtu = localmtu;
+
+ /* for version >= 1.6, ACK packet mode we support */
+ if (vio_version_after_eq(vio, 1, 6)) {
+ pkt->xfer_mode = VIO_NEW_DRING_MODE;
+ pkt->options = VIO_TX_DRING;
+ }
+
+ if (!(xfer_mode | VIO_NEW_DRING_MODE) ||
pkt->addr_type != VNET_ADDR_ETHERMAC ||
- pkt->mtu != ETH_FRAME_LEN) {
+ pkt->mtu != localmtu) {
viodbg(HS, "SEND NET ATTR NACK\n");
pkt->tag.stype = VIO_SUBTYPE_NACK;
@@ -114,7 +173,14 @@ static int handle_attr_info(struct vio_driver_state *vio,
return -ECONNRESET;
} else {
- viodbg(HS, "SEND NET ATTR ACK\n");
+ viodbg(HS, "SEND NET ATTR ACK xmode[0x%x] atype[0x%x] "
+ "addr[%llx] ackfreq[%u] plnk_updt[0x%02x] opts[0x%02x] "
+ "mtu[%llu] (rmtu[%llu]) cflags[0x%04x] lso_max[%u]\n",
+ pkt->xfer_mode, pkt->addr_type,
+ (unsigned long long)pkt->addr,
+ pkt->ack_freq, pkt->plnk_updt, pkt->options,
+ (unsigned long long)pkt->mtu, port->rmtu, pkt->cflags,
+ pkt->ipv4_lso_maxlen);
pkt->tag.stype = VIO_SUBTYPE_ACK;
@@ -210,7 +276,7 @@ static int vnet_rx_one(struct vnet_port *port, unsigned int len,
int err;
err = -EMSGSIZE;
- if (unlikely(len < ETH_ZLEN || len > ETH_FRAME_LEN)) {
+ if (unlikely(len < ETH_ZLEN || len > port->rmtu)) {
dev->stats.rx_length_errors++;
goto out_dropped;
}
@@ -558,8 +624,10 @@ static void vnet_event(void *arg, int event)
vio_link_state_change(vio, event);
spin_unlock_irqrestore(&vio->lock, flags);
- if (event == LDC_EVENT_RESET)
+ if (event == LDC_EVENT_RESET) {
+ port->rmtu = 0;
vio_port_up(vio);
+ }
return;
}
@@ -1051,8 +1119,8 @@ static int vnet_port_alloc_tx_bufs(struct vnet_port *port)
void *dring;
for (i = 0; i < VNET_TX_RING_SIZE; i++) {
- void *buf = kzalloc(ETH_FRAME_LEN + 8, GFP_KERNEL);
- int map_len = (ETH_FRAME_LEN + 7) & ~7;
+ void *buf = kzalloc(VNET_MAXPACKET + 8, GFP_KERNEL);
+ int map_len = (VNET_MAXPACKET + 7) & ~7;
err = -ENOMEM;
if (!buf)
diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
index da49337..986e04b 100644
--- a/drivers/net/ethernet/sun/sunvnet.h
+++ b/drivers/net/ethernet/sun/sunvnet.h
@@ -11,6 +11,7 @@
*/
#define VNET_TX_TIMEOUT (5 * HZ)
+#define VNET_MAXPACKET 1518ULL /* ETH_FRAMELEN + VLAN_HDR */
#define VNET_TX_RING_SIZE 512
#define VNET_TX_WAKEUP_THRESH(dr) ((dr)->pending / 4)
@@ -44,6 +45,8 @@ struct vnet_port {
u32 stop_rx_idx;
bool stop_rx;
bool start_cons;
+
+ u64 rmtu;
};
static inline struct vnet_port *to_vnet_port(struct vio_driver_state *vio)
--
1.7.1
^ permalink raw reply related
* [PATCHv9 net-next 2/4] sunvnet: make transmit path zero-copy in the kernel
From: David L Stevens @ 2014-09-29 23:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Sowmini Varadhan, Raghuram Kothahota
This patch removes pre-allocated transmit buffers and instead directly maps
pending packets on demand. This saves O(n^2) maximum-sized transmit buffers,
for n hosts on a vswitch, as well as a copy to those buffers.
Single-stream TCP throughput linux-solaris dropped ~5% for 1500-byte MTU,
but linux-linux at 1500-bytes increased ~20%.
Signed-off-by: David L Stevens <david.stevens@oracle.com>
---
drivers/net/ethernet/sun/sunvnet.c | 218 ++++++++++++++++++++++++++++-------
drivers/net/ethernet/sun/sunvnet.h | 9 ++-
2 files changed, 182 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index b1abcad..8f5f4e3 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -780,6 +780,117 @@ struct vnet_port *tx_port_find(struct vnet *vp, struct sk_buff *skb)
return ret;
}
+static struct sk_buff *vnet_clean_tx_ring(struct vnet_port *port,
+ unsigned *pending)
+{
+ struct vio_dring_state *dr = &port->vio.drings[VIO_DRIVER_TX_RING];
+ struct sk_buff *skb = NULL;
+ int i, txi;
+
+ *pending = 0;
+
+ txi = dr->prod-1;
+ if (txi < 0)
+ txi = VNET_TX_RING_SIZE-1;
+
+ for (i = 0; i < VNET_TX_RING_SIZE; ++i) {
+ struct vio_net_desc *d;
+
+ d = vio_dring_entry(dr, txi);
+
+ if (d->hdr.state == VIO_DESC_DONE) {
+ if (port->tx_bufs[txi].skb) {
+ BUG_ON(port->tx_bufs[txi].skb->next);
+
+ port->tx_bufs[txi].skb->next = skb;
+ skb = port->tx_bufs[txi].skb;
+ port->tx_bufs[txi].skb = NULL;
+
+ ldc_unmap(port->vio.lp,
+ port->tx_bufs[txi].cookies,
+ port->tx_bufs[txi].ncookies);
+ }
+ d->hdr.state = VIO_DESC_FREE;
+ } else if (d->hdr.state == VIO_DESC_READY) {
+ (*pending)++;
+ } else if (d->hdr.state == VIO_DESC_FREE) {
+ break;
+ }
+ --txi;
+ if (txi < 0)
+ txi = VNET_TX_RING_SIZE-1;
+ }
+ return skb;
+}
+
+static inline void vnet_free_skbs(struct sk_buff *skb)
+{
+ struct sk_buff *next;
+
+ while (skb) {
+ next = skb->next;
+ skb->next = NULL;
+ dev_kfree_skb(skb);
+ skb = next;
+ }
+}
+
+static void vnet_clean_timer_expire(unsigned long port0)
+{
+ struct vnet_port *port = (struct vnet_port *)port0;
+ struct sk_buff *freeskbs;
+ unsigned pending;
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->vio.lock, flags);
+ freeskbs = vnet_clean_tx_ring(port, &pending);
+ spin_unlock_irqrestore(&port->vio.lock, flags);
+
+ vnet_free_skbs(freeskbs);
+
+ if (pending)
+ (void)mod_timer(&port->clean_timer,
+ jiffies + VNET_CLEAN_TIMEOUT);
+ else
+ del_timer(&port->clean_timer);
+}
+
+static inline struct sk_buff *vnet_skb_shape(struct sk_buff *skb, void **pstart,
+ int *plen)
+{
+ struct sk_buff *nskb;
+ int len, pad;
+
+ len = skb->len;
+ pad = 0;
+ if (len < ETH_ZLEN) {
+ pad += ETH_ZLEN - skb->len;
+ len += pad;
+ }
+ len += VNET_PACKET_SKIP;
+ pad += 8 - (len & 7);
+ len += 8 - (len & 7);
+
+ if (((unsigned long)skb->data & 7) != VNET_PACKET_SKIP ||
+ skb_tailroom(skb) < pad ||
+ skb_headroom(skb) < VNET_PACKET_SKIP) {
+ nskb = alloc_and_align_skb(skb->dev, skb->len);
+ skb_reserve(nskb, VNET_PACKET_SKIP);
+ if (skb_copy_bits(skb, 0, nskb->data, skb->len)) {
+ dev_kfree_skb(nskb);
+ dev_kfree_skb(skb);
+ return NULL;
+ }
+ (void)skb_put(nskb, skb->len);
+ dev_kfree_skb(skb);
+ skb = nskb;
+ }
+
+ *pstart = skb->data - VNET_PACKET_SKIP;
+ *plen = len;
+ return skb;
+}
+
static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct vnet *vp = netdev_priv(dev);
@@ -788,12 +899,20 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
struct vio_net_desc *d;
unsigned long flags;
unsigned int len;
- void *tx_buf;
- int i, err;
+ struct sk_buff *freeskbs = NULL;
+ int i, err, txi;
+ void *start = NULL;
+ int nlen = 0;
+ unsigned pending = 0;
if (unlikely(!port))
goto out_dropped;
+ skb = vnet_skb_shape(skb, &start, &nlen);
+
+ if (unlikely(!skb))
+ goto out_dropped;
+
spin_lock_irqsave(&port->vio.lock, flags);
dr = &port->vio.drings[VIO_DRIVER_TX_RING];
@@ -811,14 +930,27 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
d = vio_dring_cur(dr);
- tx_buf = port->tx_bufs[dr->prod].buf;
- skb_copy_from_linear_data(skb, tx_buf + VNET_PACKET_SKIP, skb->len);
+ txi = dr->prod;
+
+ freeskbs = vnet_clean_tx_ring(port, &pending);
+
+ BUG_ON(port->tx_bufs[txi].skb);
len = skb->len;
- if (len < ETH_ZLEN) {
+ if (len < ETH_ZLEN)
len = ETH_ZLEN;
- memset(tx_buf+VNET_PACKET_SKIP+skb->len, 0, len - skb->len);
+
+ port->tx_bufs[txi].skb = skb;
+ skb = NULL;
+
+ err = ldc_map_single(port->vio.lp, start, nlen,
+ port->tx_bufs[txi].cookies, 2,
+ (LDC_MAP_SHADOW | LDC_MAP_DIRECT | LDC_MAP_RW));
+ if (err < 0) {
+ netdev_info(dev, "tx buffer map error %d\n", err);
+ goto out_dropped_unlock;
}
+ port->tx_bufs[txi].ncookies = err;
/* We don't rely on the ACKs to free the skb in vnet_start_xmit(),
* thus it is safe to not set VIO_ACK_ENABLE for each transmission:
@@ -830,9 +962,9 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
*/
d->hdr.ack = VIO_ACK_DISABLE;
d->size = len;
- d->ncookies = port->tx_bufs[dr->prod].ncookies;
+ d->ncookies = port->tx_bufs[txi].ncookies;
for (i = 0; i < d->ncookies; i++)
- d->cookies[i] = port->tx_bufs[dr->prod].cookies[i];
+ d->cookies[i] = port->tx_bufs[txi].cookies[i];
/* This has to be a non-SMP write barrier because we are writing
* to memory which is shared with the peer LDOM.
@@ -876,7 +1008,7 @@ ldc_start_done:
port->start_cons = false;
dev->stats.tx_packets++;
- dev->stats.tx_bytes += skb->len;
+ dev->stats.tx_bytes += port->tx_bufs[txi].skb->len;
dr->prod = (dr->prod + 1) & (VNET_TX_RING_SIZE - 1);
if (unlikely(vnet_tx_dring_avail(dr) < 2)) {
@@ -887,7 +1019,9 @@ ldc_start_done:
spin_unlock_irqrestore(&port->vio.lock, flags);
- dev_kfree_skb(skb);
+ vnet_free_skbs(freeskbs);
+
+ (void)mod_timer(&port->clean_timer, jiffies + VNET_CLEAN_TIMEOUT);
return NETDEV_TX_OK;
@@ -895,7 +1029,14 @@ out_dropped_unlock:
spin_unlock_irqrestore(&port->vio.lock, flags);
out_dropped:
- dev_kfree_skb(skb);
+ if (skb)
+ dev_kfree_skb(skb);
+ vnet_free_skbs(freeskbs);
+ if (pending)
+ (void)mod_timer(&port->clean_timer,
+ jiffies + VNET_CLEAN_TIMEOUT);
+ else
+ del_timer(&port->clean_timer);
dev->stats.tx_dropped++;
return NETDEV_TX_OK;
}
@@ -1097,17 +1238,22 @@ static void vnet_port_free_tx_bufs(struct vnet_port *port)
}
for (i = 0; i < VNET_TX_RING_SIZE; i++) {
- void *buf = port->tx_bufs[i].buf;
+ struct vio_net_desc *d;
+ void *skb = port->tx_bufs[i].skb;
- if (!buf)
+ if (!skb)
continue;
+ d = vio_dring_entry(dr, i);
+ if (d->hdr.state == VIO_DESC_READY)
+ pr_warn("active transmit buffers freed\n");
+
ldc_unmap(port->vio.lp,
port->tx_bufs[i].cookies,
port->tx_bufs[i].ncookies);
-
- kfree(buf);
- port->tx_bufs[i].buf = NULL;
+ dev_kfree_skb(skb);
+ port->tx_bufs[i].skb = NULL;
+ d->hdr.state = VIO_DESC_FREE;
}
}
@@ -1118,34 +1264,6 @@ static int vnet_port_alloc_tx_bufs(struct vnet_port *port)
int i, err, ncookies;
void *dring;
- for (i = 0; i < VNET_TX_RING_SIZE; i++) {
- void *buf = kzalloc(VNET_MAXPACKET + 8, GFP_KERNEL);
- int map_len = (VNET_MAXPACKET + 7) & ~7;
-
- err = -ENOMEM;
- if (!buf)
- goto err_out;
-
- err = -EFAULT;
- if ((unsigned long)buf & (8UL - 1)) {
- pr_err("TX buffer misaligned\n");
- kfree(buf);
- goto err_out;
- }
-
- err = ldc_map_single(port->vio.lp, buf, map_len,
- port->tx_bufs[i].cookies, 2,
- (LDC_MAP_SHADOW |
- LDC_MAP_DIRECT |
- LDC_MAP_RW));
- if (err < 0) {
- kfree(buf);
- goto err_out;
- }
- port->tx_bufs[i].buf = buf;
- port->tx_bufs[i].ncookies = err;
- }
-
dr = &port->vio.drings[VIO_DRIVER_TX_RING];
len = (VNET_TX_RING_SIZE *
@@ -1172,6 +1290,12 @@ static int vnet_port_alloc_tx_bufs(struct vnet_port *port)
dr->pending = VNET_TX_RING_SIZE;
dr->ncookies = ncookies;
+ for (i = 0; i < VNET_TX_RING_SIZE; ++i) {
+ struct vio_net_desc *d;
+
+ d = vio_dring_entry(dr, i);
+ d->hdr.state = VIO_DESC_FREE;
+ }
return 0;
err_out:
@@ -1203,6 +1327,8 @@ static struct vnet *vnet_new(const u64 *local_mac)
dev = alloc_etherdev(sizeof(*vp));
if (!dev)
return ERR_PTR(-ENOMEM);
+ dev->needed_headroom = VNET_PACKET_SKIP + 8;
+ dev->needed_tailroom = 8;
for (i = 0; i < ETH_ALEN; i++)
dev->dev_addr[i] = (*local_mac >> (5 - i) * 8) & 0xff;
@@ -1397,6 +1523,9 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
pr_info("%s: PORT ( remote-mac %pM%s )\n",
vp->dev->name, port->raddr, switch_port ? " switch-port" : "");
+ setup_timer(&port->clean_timer, vnet_clean_timer_expire,
+ (unsigned long)port);
+
vio_port_up(&port->vio);
mdesc_release(hp);
@@ -1423,6 +1552,7 @@ static int vnet_port_remove(struct vio_dev *vdev)
unsigned long flags;
del_timer_sync(&port->vio.timer);
+ del_timer_sync(&port->clean_timer);
spin_lock_irqsave(&vp->lock, flags);
list_del(&port->list);
diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
index 986e04b..02f507d 100644
--- a/drivers/net/ethernet/sun/sunvnet.h
+++ b/drivers/net/ethernet/sun/sunvnet.h
@@ -11,6 +11,11 @@
*/
#define VNET_TX_TIMEOUT (5 * HZ)
+/* length of time (or less) we expect pending descriptors to be marked
+ * as VIO_DESC_DONE and skbs ready to be freed
+ */
+#define VNET_CLEAN_TIMEOUT ((HZ/100)+1)
+
#define VNET_MAXPACKET 1518ULL /* ETH_FRAMELEN + VLAN_HDR */
#define VNET_TX_RING_SIZE 512
#define VNET_TX_WAKEUP_THRESH(dr) ((dr)->pending / 4)
@@ -22,7 +27,7 @@
#define VNET_PACKET_SKIP 6
struct vnet_tx_entry {
- void *buf;
+ struct sk_buff *skb;
unsigned int ncookies;
struct ldc_trans_cookie cookies[2];
};
@@ -46,6 +51,8 @@ struct vnet_port {
bool stop_rx;
bool start_cons;
+ struct timer_list clean_timer;
+
u64 rmtu;
};
--
1.7.1
^ permalink raw reply related
* [PATCHv9 net-next 3/4] sunvnet: allow admin to set sunvnet MTU
From: David L Stevens @ 2014-09-29 23:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Sowmini Varadhan, Raghuram Kothahota
This patch allows an admin to set the MTU on a sunvnet device to arbitrary
values between the minimum (68) and maximum (65535) IPv4 packet sizes.
Signed-off-by: David L Stevens <david.stevens@oracle.com>
---
arch/sparc/kernel/ldc.c | 2 +-
drivers/net/ethernet/sun/sunvnet.c | 7 +++++--
drivers/net/ethernet/sun/sunvnet.h | 6 ++++--
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/arch/sparc/kernel/ldc.c b/arch/sparc/kernel/ldc.c
index 66dacd5..0af28b9 100644
--- a/arch/sparc/kernel/ldc.c
+++ b/arch/sparc/kernel/ldc.c
@@ -2159,7 +2159,7 @@ int ldc_map_single(struct ldc_channel *lp,
state.pte_idx = (base - iommu->page_table);
state.nc = 0;
fill_cookies(&state, (pa & PAGE_MASK), (pa & ~PAGE_MASK), len);
- BUG_ON(state.nc != 1);
+ BUG_ON(state.nc > ncookies);
return state.nc;
}
diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 8f5f4e3..3980471 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -913,6 +913,9 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
if (unlikely(!skb))
goto out_dropped;
+ if (skb->len > port->rmtu)
+ goto out_dropped;
+
spin_lock_irqsave(&port->vio.lock, flags);
dr = &port->vio.drings[VIO_DRIVER_TX_RING];
@@ -944,7 +947,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
skb = NULL;
err = ldc_map_single(port->vio.lp, start, nlen,
- port->tx_bufs[txi].cookies, 2,
+ port->tx_bufs[txi].cookies, VNET_MAXCOOKIES,
(LDC_MAP_SHADOW | LDC_MAP_DIRECT | LDC_MAP_RW));
if (err < 0) {
netdev_info(dev, "tx buffer map error %d\n", err);
@@ -1182,7 +1185,7 @@ static void vnet_set_rx_mode(struct net_device *dev)
static int vnet_change_mtu(struct net_device *dev, int new_mtu)
{
- if (new_mtu != ETH_DATA_LEN)
+ if (new_mtu < 68 || new_mtu > 65535)
return -EINVAL;
dev->mtu = new_mtu;
diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
index 02f507d..c911045 100644
--- a/drivers/net/ethernet/sun/sunvnet.h
+++ b/drivers/net/ethernet/sun/sunvnet.h
@@ -16,7 +16,7 @@
*/
#define VNET_CLEAN_TIMEOUT ((HZ/100)+1)
-#define VNET_MAXPACKET 1518ULL /* ETH_FRAMELEN + VLAN_HDR */
+#define VNET_MAXPACKET (65535ULL + ETH_HLEN + VLAN_HLEN)
#define VNET_TX_RING_SIZE 512
#define VNET_TX_WAKEUP_THRESH(dr) ((dr)->pending / 4)
@@ -26,10 +26,12 @@
*/
#define VNET_PACKET_SKIP 6
+#define VNET_MAXCOOKIES (VNET_MAXPACKET/PAGE_SIZE + 1)
+
struct vnet_tx_entry {
struct sk_buff *skb;
unsigned int ncookies;
- struct ldc_trans_cookie cookies[2];
+ struct ldc_trans_cookie cookies[VNET_MAXCOOKIES];
};
struct vnet;
--
1.7.1
^ permalink raw reply related
* [PATCHv9 net-next 4/4] sunvnet: generate ICMP PTMUD messages for smaller port MTUs
From: David L Stevens @ 2014-09-29 23:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Sowmini Varadhan, Raghuram Kothahota
This patch sends ICMP and ICMPv6 messages for Path MTU Discovery when a remote
port MTU is smaller than the device MTU. This allows mixing newer VIO protocol
devices that support MTU negotiation with older devices that do not on the
same vswitch. It also allows Linux-Linux LDOMs to use 64K-1 data packets even
though Solaris vswitch is limited to <16K MTU.
Signed-off-by: David L Stevens <david.stevens@oracle.com>
---
drivers/net/ethernet/sun/sunvnet.c | 37 +++++++++++++++++++++++++++++++++++-
1 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 3980471..1262697 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -17,6 +17,13 @@
#include <linux/mutex.h>
#include <linux/if_vlan.h>
+#if IS_ENABLED(CONFIG_IPV6)
+#include <linux/icmpv6.h>
+#endif
+
+#include <net/icmp.h>
+#include <net/route.h>
+
#include <asm/vio.h>
#include <asm/ldc.h>
@@ -913,8 +920,36 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
if (unlikely(!skb))
goto out_dropped;
- if (skb->len > port->rmtu)
+ if (skb->len > port->rmtu) {
+ unsigned long localmtu = port->rmtu - ETH_HLEN;
+
+ if (vio_version_after_eq(&port->vio, 1, 3))
+ localmtu -= VLAN_HLEN;
+
+ if (skb->protocol == htons(ETH_P_IP)) {
+ struct flowi4 fl4;
+ struct rtable *rt = NULL;
+
+ memset(&fl4, 0, sizeof(fl4));
+ fl4.flowi4_oif = dev->ifindex;
+ fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
+ fl4.daddr = ip_hdr(skb)->daddr;
+ fl4.saddr = ip_hdr(skb)->saddr;
+
+ rt = ip_route_output_key(dev_net(dev), &fl4);
+ if (!IS_ERR(rt)) {
+ skb_dst_set(skb, &rt->dst);
+ icmp_send(skb, ICMP_DEST_UNREACH,
+ ICMP_FRAG_NEEDED,
+ htonl(localmtu));
+ }
+ }
+#if IS_ENABLED(CONFIG_IPV6)
+ else if (skb->protocol == htons(ETH_P_IPV6))
+ icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, localmtu);
+#endif
goto out_dropped;
+ }
spin_lock_irqsave(&port->vio.lock, flags);
--
1.7.1
^ permalink raw reply related
* Re: VRFs and the scalability of namespaces
From: Hannes Frederic Sowa @ 2014-09-29 23:50 UTC (permalink / raw)
To: David Ahern, Ben Greear, Sowmini Varadhan
Cc: Eric W. Biederman, Nicolas Dichtel, netdev
In-Reply-To: <5429EEAF.9030702@gmail.com>
On Tue, Sep 30, 2014, at 01:43, David Ahern wrote:
> On 9/29/14, 11:00 AM, Ben Greear wrote:
> > On 09/29/2014 09:50 AM, Sowmini Varadhan wrote:
> >> On Mon, Sep 29, 2014 at 12:40 PM, Ben Greear <greearb@candelatech.com> wrote:
> >>> On 09/29/2014 06:06 AM, David Ahern wrote:
> >>
> >>>
> >>> We have implemented support for at least most of this (excepting duplicate IPs)
> >>> using routing tables, rules, and (optionally, xorp as the router).
> >>>
> >>
> >> My undertanding of multiple routing-tables/rules was that they
> >> are closer in semantics to switch/router ACLs than to VRFs, eg.,
> >> one big difference is that an interface can belong to exactly one
> >> VRF at a time, which is not mandated by multiple routing-tables/rules.
> >>
> >> Was I mistaken?
> >
> > You can effectively force an interface to belong to a particular virtual
> > router (table). It is not trivial to do, and possibly I have still not
> > covered every possible case. Some rules grow somewhat exponentially as
> > interfaces are added to virtual routers (ie, preference 10 rules).
>
> An interesting way of doing it; thanks for the reference point.
>
> Fundamentally the design should be able to assign interfaces to a single
> VRF, support duplicate IP addresses on different interfaces in different
> VRFs and be able to scale to 10,000+ netdevices -- devices representing
> physical ports as well as logical interfaces built on top of them (e.g.,
> sub-interfaces).
Duplicate IP addresses don't go well with current linux stack being a
soft end model by default. Current separation is done on arp level today
if some kind of strong end model is desired. This calls for some kind of
namespaces again. ;)
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH 00/34] pull request: netfilter/ipvs updates for net-next
From: Simon Horman @ 2014-09-30 0:22 UTC (permalink / raw)
To: David Miller; +Cc: pablo, netfilter-devel, netdev
In-Reply-To: <20140929.145424.1355091301129121293.davem@davemloft.net>
On Mon, Sep 29, 2014 at 02:54:24PM -0400, David Miller wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Mon, 29 Sep 2014 14:38:49 +0200
>
> > The following patchset contains Netfilter/IPVS updates for net-next,
> > most relevantly they are:
> ...
> > You can pull these changes from:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git
>
> Pulled, thanks Pablo.
>
> Please address Sergei's coding style comments.
Thanks Dave,
I will handle that.
^ permalink raw reply
* Re: [PATCH] net: Add ndo_gso_check
From: Jesse Gross @ 2014-09-30 0:33 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, netdev, Or Gerlitz
In-Reply-To: <CA+mtBx-MdxNrmE82Cqut_T8WuzoOVivPmWKjC0pT8iSrGxWBQg@mail.gmail.com>
On Mon, Sep 29, 2014 at 1:47 PM, Tom Herbert <therbert@google.com> wrote:
> On Mon, Sep 29, 2014 at 1:13 PM, Jesse Gross <jesse@nicira.com> wrote:
>> On Sun, Sep 28, 2014 at 8:50 PM, Tom Herbert <therbert@google.com> wrote:
>>> Add ndo_gso_check which a device can define to indicate whether is
>>> is capable of doing GSO on a packet. This funciton would be called from
>>> the stack to determine whether software GSO is needed to be done. A
>>> driver should populate this function if it advertises GSO types for
>>> which there are combinations that it wouldn't be able to handle. For
>>> instance a device that performs UDP tunneling might only implement
>>> support for transparent Ethernet bridging type of inner packets
>>> or might have limitations on lengths of inner headers.
>>>
>>> Signed-off-by: Tom Herbert <therbert@google.com>
>>
>> Offloads that might have limitations extend beyond GSO - checksum is
>> another possibility that could need something like a length check.
>>
> There are several examples of drivers that advertise checksum offload
> but then in TX path decide that they can't do it for various reasons--
> in this case they need to do software checksum calculation themselves.
> Applying this model to GSO, that is drivers do software GSO when they
> need to punt on packet, seems pretty hard to me, but maybe someone can
> look at it.
Yes, I definitely think that it's easier to have GSO done in the
stack. I agree with you that exposing various capabilities,
limitations, etc. for different features will get messy very quickly,
so I guess this model makes sense for now.
>> In addition, while I would also like to be optimistic about the
>> capabilities of existing NICs it's unlikely that any of them that are
>> advertising SKB_GSO_UDP_TUNNEL can actually do it with full
>> generality. So unless we can get the driver writers to chime in about
>> their capabilities (maybe we can, there's only a handful of them right
>> now), we probably need to provide a more conservative implementation
>> for those drivers. I guess I would probably do length equal to VXLAN
>> and perhaps containing Ethernet as a balance between the most
>> conservative and the most optimistic.
>
> Checking header length in ndo_gso_check would be trivial.
I agree - I just think that we need to provide some implementations in
existing drivers to do this. Otherwise, as we introduce new protocols
they will probably be broken in some situations.
^ permalink raw reply
* Re: VRFs and the scalability of namespaces
From: Ben Greear @ 2014-09-30 1:15 UTC (permalink / raw)
To: Hannes Frederic Sowa, David Ahern, Sowmini Varadhan
Cc: Eric W. Biederman, Nicolas Dichtel, netdev
In-Reply-To: <1412034624.2008259.173208057.03BE7CBA@webmail.messagingengine.com>
On 09/29/2014 04:50 PM, Hannes Frederic Sowa wrote:
> On Tue, Sep 30, 2014, at 01:43, David Ahern wrote:
>> On 9/29/14, 11:00 AM, Ben Greear wrote:
>>> On 09/29/2014 09:50 AM, Sowmini Varadhan wrote:
>>>> On Mon, Sep 29, 2014 at 12:40 PM, Ben Greear <greearb@candelatech.com> wrote:
>>>>> On 09/29/2014 06:06 AM, David Ahern wrote:
>>>>
>>>>>
>>>>> We have implemented support for at least most of this (excepting duplicate IPs)
>>>>> using routing tables, rules, and (optionally, xorp as the router).
>>>>>
>>>>
>>>> My undertanding of multiple routing-tables/rules was that they
>>>> are closer in semantics to switch/router ACLs than to VRFs, eg.,
>>>> one big difference is that an interface can belong to exactly one
>>>> VRF at a time, which is not mandated by multiple routing-tables/rules.
>>>>
>>>> Was I mistaken?
>>>
>>> You can effectively force an interface to belong to a particular virtual
>>> router (table). It is not trivial to do, and possibly I have still not
>>> covered every possible case. Some rules grow somewhat exponentially as
>>> interfaces are added to virtual routers (ie, preference 10 rules).
>>
>> An interesting way of doing it; thanks for the reference point.
>>
>> Fundamentally the design should be able to assign interfaces to a single
>> VRF, support duplicate IP addresses on different interfaces in different
>> VRFs and be able to scale to 10,000+ netdevices -- devices representing
>> physical ports as well as logical interfaces built on top of them (e.g.,
>> sub-interfaces).
>
> Duplicate IP addresses don't go well with current linux stack being a
> soft end model by default. Current separation is done on arp level today
> if some kind of strong end model is desired. This calls for some kind of
> namespaces again. ;)
arp is per interface as well if you set arp-filter properly, the main problem with duplicate IPs is that
you can't (easily?) set up routing rules that match them properly...
Thanks,
Ben
>
> Bye,
> Hannes
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH v2 net-next] net: cleanup and document skb fclone layout
From: Eric Dumazet @ 2014-09-30 1:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <1412022555.30721.40.camel@edumazet-glaptop2.roam.corp.google.com>
On Mon, 2014-09-29 at 13:29 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
> {
> - struct sk_buff *n;
> + struct sk_buff_fclones *fclones = container_of(skb,
> + struct sk_buff_fclones,
> + skb1);
> + struct sk_buff *n = &fclones->skb2;
>
> if (skb_orphan_frags(skb, gfp_mask))
> return NULL;
>
> - n = skb + 1;
> if (skb->fclone == SKB_FCLONE_ORIG &&
> n->fclone == SKB_FCLONE_UNAVAILABLE) {
> - atomic_t *fclone_ref = (atomic_t *) (n + 1);
> n->fclone = SKB_FCLONE_CLONE;
> - atomic_inc(fclone_ref);
> + atomic_inc(&fclones->fclone_ref);
> } else {
> if (skb_pfmemalloc(skb))
> gfp_mask |= __GFP_MEMALLOC;
Note :
The "atomic_inc(&fclones->fclone_ref)" here can be replaced by an
atomic_set(&fclones->fclone_ref, 2), to avoid a locked operation.
I'll cook a patch for this, after you merged this cleanup
Thanks
^ permalink raw reply
* RE: [PATCH v2 1/1] net: fec: implement rx_copybreak to improve rx performance
From: fugang.duan @ 2014-09-30 1:38 UTC (permalink / raw)
To: Zhi Li
Cc: Eric Dumazet, Frank.Li@freescale.com, davem@davemloft.net,
romieu@fr.zoreil.com, netdev@vger.kernel.org,
shawn.guo@linaro.org, bhutchings@solarflare.com
In-Reply-To: <CAHrpEqTL+9iv6SUM7V2Nn4SD4G-20+xAfNCfTqvLMGO0weuj1A@mail.gmail.com>
From: Zhi Li <lznuaa@gmail.com> Sent: Monday, September 29, 2014 9:10 PM
>To: Duan Fugang-B38611
>Cc: Eric Dumazet; Li Frank-B20596; davem@davemloft.net;
>romieu@fr.zoreil.com; netdev@vger.kernel.org; shawn.guo@linaro.org;
>bhutchings@solarflare.com
>Subject: Re: [PATCH v2 1/1] net: fec: implement rx_copybreak to improve rx
>performance
>
>On Mon, Sep 29, 2014 at 7:43 AM, fugang.duan@freescale.com
><fugang.duan@freescale.com> wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com> Sent: Monday, September
>> 29, 2014 8:12 PM
>>>To: Duan Fugang-B38611
>>>Cc: Li Frank-B20596; davem@davemloft.net; romieu@fr.zoreil.com;
>>>netdev@vger.kernel.org; shawn.guo@linaro.org;
>>>bhutchings@solarflare.com
>>>Subject: Re: [PATCH v2 1/1] net: fec: implement rx_copybreak to
>>>improve rx performance
>>>
>>>On Mon, 2014-09-29 at 15:47 +0800, Fugang Duan wrote:
>>>> - Copy short frames and keep the buffers mapped, re-allocate skb
>>>> instead
>>>of
>>>> memory copy for long frames.
>>>> - Add support for setting/getting rx_copybreak using generic ethtool
>>>> tunable
>>>>
>>>> Changes V2:
>>>> * Implements rx_copybreak
>>>
>>>>
>>>> +#define COPYBREAK_DEFAULT 256
>>>> +static unsigned int copybreak __read_mostly = COPYBREAK_DEFAULT;
>>>> +module_param(copybreak, uint, 0644); MODULE_PARM_DESC(copybreak,
>>>> + "Maximum packet size that is copied to a new buffer on
>>>receive");
>>>> +
>>>
>>>It is not the right way to handle this.
>>>
>>>Old drivers might still have a legacy module parameter.
>>>
>>>New implementations should provide the new ethtool support, and only
>this.
>>>No new module parameter is accepted.
>>>
>>>For details, check following commits :
>>>
>>>d4ad30b182305ecf97f145a5d4d1fd9e728c6d01 enic: Add tunable_ops support
>>>for rx_copybreak
>>>
>>>f0db9b073415848709dd59a6394969882f517da9 ethtool: Add generic options
>>>for tunables
>>>
>>>
>>>Or check the current tree :
>>>
>>>git grep -n rx_copybreak drivers/net/ethernet/cisco/enic
>>>
>> The patch implement ethtool interface, pls read the patch completely.
>
>I think Eric's means is that REMOVE module parameter and just keep ethtool
>API.
>
>best regards
>Frank Li
>
Ok, I will submit the next version to remove the module parameter.
Regards,
Andy
^ permalink raw reply
* [PATCH][net-next] tcp: Change tcp_slow_start function to return void
From: roy.qing.li @ 2014-09-30 1:49 UTC (permalink / raw)
To: netdev
From: Li RongQing <roy.qing.li@gmail.com>
No caller uses the return value, so make this function return void.
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
include/net/tcp.h | 2 +-
net/ipv4/tcp_cong.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 1f57c53..d4fef69 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -830,7 +830,7 @@ void tcp_get_available_congestion_control(char *buf, size_t len);
void tcp_get_allowed_congestion_control(char *buf, size_t len);
int tcp_set_allowed_congestion_control(char *allowed);
int tcp_set_congestion_control(struct sock *sk, const char *name);
-int tcp_slow_start(struct tcp_sock *tp, u32 acked);
+void tcp_slow_start(struct tcp_sock *tp, u32 acked);
void tcp_cong_avoid_ai(struct tcp_sock *tp, u32 w);
u32 tcp_reno_ssthresh(struct sock *sk);
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index a6c8a57..b1c5970 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -291,15 +291,13 @@ int tcp_set_congestion_control(struct sock *sk, const char *name)
* ABC caps N to 2. Slow start exits when cwnd grows over ssthresh and
* returns the leftover acks to adjust cwnd in congestion avoidance mode.
*/
-int tcp_slow_start(struct tcp_sock *tp, u32 acked)
+void tcp_slow_start(struct tcp_sock *tp, u32 acked)
{
u32 cwnd = tp->snd_cwnd + acked;
if (cwnd > tp->snd_ssthresh)
cwnd = tp->snd_ssthresh + 1;
- acked -= cwnd - tp->snd_cwnd;
tp->snd_cwnd = min(cwnd, tp->snd_cwnd_clamp);
- return acked;
}
EXPORT_SYMBOL_GPL(tcp_slow_start);
--
1.7.10.4
^ permalink raw reply related
* [PATCH net-next 0/2] bpf: add search pruning optimization and tests
From: Alexei Starovoitov @ 2014-09-30 1:50 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, netdev, linux-kernel
Hi All,
patch #1 commit log explains why eBPF verifier has to examine some
instructions multiple times and describes the search pruning optimization
that improves verification speed for branchy programs and allows more
complex programs to be verified successfully.
This patch completes the core verifier logic.
patch #2 adds more verifier tests related to branches and search pruning
I'm still working on Andy's 'bitmask for stack slots' suggestion. It will be
done on top of this patch.
The current verifier algorithm is brute force depth first search with
state pruning. If anyone can come up with another algorithm that demonstrates
better results, we'll replace the algorithm without affecting user space.
Note verifier doesn't guarantee that all possible valid programs are accepted.
Overly complex programs may still be rejected.
Verifier improvements/optimizations will guarantee that if a program
was passing verification in the past, it will still be passing.
Alexei Starovoitov (2):
bpf: add search pruning optimization to verifier
bpf: add tests to verifier testsuite
kernel/bpf/verifier.c | 146 +++++++++++++++++++++++++++++++++++++++++++
samples/bpf/test_verifier.c | 130 ++++++++++++++++++++++++++++++++++++++
2 files changed, 276 insertions(+)
--
1.7.9.5
^ permalink raw reply
* [PATCH net-next 2/2] bpf: add tests to verifier testsuite
From: Alexei Starovoitov @ 2014-09-30 1:50 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, netdev, linux-kernel
In-Reply-To: <1412041802-24858-1-git-send-email-ast@plumgrid.com>
add 4 extra tests to cover jump verification better
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
samples/bpf/test_verifier.c | 130 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 130 insertions(+)
diff --git a/samples/bpf/test_verifier.c b/samples/bpf/test_verifier.c
index d10992e2740e..f44ef11f65a7 100644
--- a/samples/bpf/test_verifier.c
+++ b/samples/bpf/test_verifier.c
@@ -461,6 +461,136 @@ static struct bpf_test tests[] = {
.errstr = "R0 invalid mem access",
.result = REJECT,
},
+ {
+ "jump test 1",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_STX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1, -8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -8, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -16, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 2, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -8, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 3, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -16, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 4, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -8, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 5, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -32, 5),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ },
+ {
+ "jump test 2",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 2),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -8, 0),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 14),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 2),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -16, 0),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 11),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 2, 2),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -32, 0),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 3, 2),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -40, 0),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 5),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 4, 2),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -48, 0),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 5, 1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -56, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ },
+ {
+ "jump test 3",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 3),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -8, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 19),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 3),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -16, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -16),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 15),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 2, 3),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -32, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -32),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 11),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 3, 3),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -40, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -40),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 7),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 4, 3),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -48, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -48),
+ BPF_JMP_IMM(BPF_JA, 0, 0, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 5, 0),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -56, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -56),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_unspec),
+ BPF_EXIT_INSN(),
+ },
+ .fixup = {24},
+ .result = ACCEPT,
+ },
+ {
+ "jump test 4",
+ .insns = {
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 1),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 2),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 3),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 4),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, BPF_REG_10, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ },
};
static int probe_filter_length(struct bpf_insn *fp)
--
1.7.9.5
^ permalink raw reply related
* [PATCH] ipvs: Clean up comment style in ip_vs.h
From: Simon Horman @ 2014-09-30 1:50 UTC (permalink / raw)
To: lvs-devel
Cc: netdev, netfilter-devel, Wensong Zhang, Julian Anastasov,
Sergei Shtylyov, David Miller, Pablo Neira Ayuso, Simon Horman
* Consistently use the multi-line comment style for networking code:
/* This
* That
* The other thing
*/
* Use single-line comment style for comments with only one line of text.
* In general follow the leading '*' of each line of a comment with a
single space and then text.
Reported-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/net/ip_vs.h | 183 ++++++++++++++++++----------------------------------
1 file changed, 61 insertions(+), 122 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 576d7f0..af69086 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1,6 +1,5 @@
-/*
- * IP Virtual Server
- * data structure and functionality definitions
+/* IP Virtual Server
+ * data structure and functionality definitions
*/
#ifndef _NET_IP_VS_H
@@ -30,15 +29,12 @@
#endif
#include <net/net_namespace.h> /* Netw namespace */
-/*
- * Generic access of ipvs struct
- */
+/* Generic access of ipvs struct */
static inline struct netns_ipvs *net_ipvs(struct net* net)
{
return net->ipvs;
}
-/*
- * Get net ptr from skb in traffic cases
+/* Get net ptr from skb in traffic cases
* use skb_sknet when call is from userland (ioctl or netlink)
*/
static inline struct net *skb_net(const struct sk_buff *skb)
@@ -90,8 +86,7 @@ static inline struct net *skb_sknet(const struct sk_buff *skb)
return &init_net;
#endif
}
-/*
- * This one needed for single_open_net since net is stored directly in
+/* This one needed for single_open_net since net is stored directly in
* private not as a struct i.e. seq_file_net can't be used.
*/
static inline struct net *seq_file_single_net(struct seq_file *seq)
@@ -108,7 +103,7 @@ extern int ip_vs_conn_tab_size;
struct ip_vs_iphdr {
__u32 len; /* IPv4 simply where L4 starts
- IPv6 where L4 Transport Header starts */
+ * IPv6 where L4 Transport Header starts */
__u16 fragoffs; /* IPv6 fragment offset, 0 if first frag (or not frag)*/
__s16 protocol;
__s32 flags;
@@ -305,15 +300,11 @@ static inline const char *ip_vs_dbg_addr(int af, char *buf, size_t buf_len,
#endif
-/*
- * The port number of FTP service (in network order).
- */
+/* The port number of FTP service (in network order). */
#define FTPPORT cpu_to_be16(21)
#define FTPDATA cpu_to_be16(20)
-/*
- * TCP State Values
- */
+/* TCP State Values */
enum {
IP_VS_TCP_S_NONE = 0,
IP_VS_TCP_S_ESTABLISHED,
@@ -329,25 +320,19 @@ enum {
IP_VS_TCP_S_LAST
};
-/*
- * UDP State Values
- */
+/* UDP State Values */
enum {
IP_VS_UDP_S_NORMAL,
IP_VS_UDP_S_LAST,
};
-/*
- * ICMP State Values
- */
+/* ICMP State Values */
enum {
IP_VS_ICMP_S_NORMAL,
IP_VS_ICMP_S_LAST,
};
-/*
- * SCTP State Values
- */
+/* SCTP State Values */
enum ip_vs_sctp_states {
IP_VS_SCTP_S_NONE,
IP_VS_SCTP_S_INIT1,
@@ -366,10 +351,9 @@ enum ip_vs_sctp_states {
IP_VS_SCTP_S_LAST
};
-/*
- * Delta sequence info structure
- * Each ip_vs_conn has 2 (output AND input seq. changes).
- * Only used in the VS/NAT.
+/* Delta sequence info structure
+ * Each ip_vs_conn has 2 (output AND input seq. changes).
+ * Only used in the VS/NAT.
*/
struct ip_vs_seq {
__u32 init_seq; /* Add delta from this seq */
@@ -378,9 +362,7 @@ struct ip_vs_seq {
before last resized pkt */
};
-/*
- * counters per cpu
- */
+/* counters per cpu */
struct ip_vs_counters {
__u32 conns; /* connections scheduled */
__u32 inpkts; /* incoming packets */
@@ -388,17 +370,13 @@ struct ip_vs_counters {
__u64 inbytes; /* incoming bytes */
__u64 outbytes; /* outgoing bytes */
};
-/*
- * Stats per cpu
- */
+/* Stats per cpu */
struct ip_vs_cpu_stats {
struct ip_vs_counters ustats;
struct u64_stats_sync syncp;
};
-/*
- * IPVS statistics objects
- */
+/* IPVS statistics objects */
struct ip_vs_estimator {
struct list_head list;
@@ -491,9 +469,7 @@ struct ip_vs_protocol {
void (*timeout_change)(struct ip_vs_proto_data *pd, int flags);
};
-/*
- * protocol data per netns
- */
+/* protocol data per netns */
struct ip_vs_proto_data {
struct ip_vs_proto_data *next;
struct ip_vs_protocol *pp;
@@ -520,9 +496,7 @@ struct ip_vs_conn_param {
__u8 pe_data_len;
};
-/*
- * IP_VS structure allocated for each dynamically scheduled connection
- */
+/* IP_VS structure allocated for each dynamically scheduled connection */
struct ip_vs_conn {
struct hlist_node c_list; /* hashed list heads */
/* Protocol, addresses and port numbers */
@@ -570,8 +544,9 @@ struct ip_vs_conn {
struct ip_vs_protocol *pp, struct ip_vs_iphdr *iph);
/* Note: we can group the following members into a structure,
- in order to save more space, and the following members are
- only used in VS/NAT anyway */
+ * in order to save more space, and the following members are
+ * only used in VS/NAT anyway
+ */
struct ip_vs_app *app; /* bound ip_vs_app object */
void *app_data; /* Application private data */
struct ip_vs_seq in_seq; /* incoming seq. struct */
@@ -584,9 +559,7 @@ struct ip_vs_conn {
struct rcu_head rcu_head;
};
-/*
- * To save some memory in conn table when name space is disabled.
- */
+/* To save some memory in conn table when name space is disabled. */
static inline struct net *ip_vs_conn_net(const struct ip_vs_conn *cp)
{
#ifdef CONFIG_NET_NS
@@ -612,13 +585,12 @@ static inline int ip_vs_conn_net_eq(const struct ip_vs_conn *cp,
#endif
}
-/*
- * Extended internal versions of struct ip_vs_service_user and
- * ip_vs_dest_user for IPv6 support.
+/* Extended internal versions of struct ip_vs_service_user and
+ * ip_vs_dest_user for IPv6 support.
*
- * We need these to conveniently pass around service and destination
- * options, but unfortunately, we also need to keep the old definitions to
- * maintain userspace backwards compatibility for the setsockopt interface.
+ * We need these to conveniently pass around service and destination
+ * options, but unfortunately, we also need to keep the old definitions to
+ * maintain userspace backwards compatibility for the setsockopt interface.
*/
struct ip_vs_service_user_kern {
/* virtual service addresses */
@@ -697,9 +669,8 @@ struct ip_vs_dest_dst {
struct rcu_head rcu_head;
};
-/*
- * The real server destination forwarding entry
- * with ip address, port number, and so on.
+/* The real server destination forwarding entry
+ * with ip address, port number, and so on.
*/
struct ip_vs_dest {
struct list_head n_list; /* for the dests in the service */
@@ -739,9 +710,7 @@ struct ip_vs_dest {
};
-/*
- * The scheduler object
- */
+/* The scheduler object */
struct ip_vs_scheduler {
struct list_head n_list; /* d-linked list head */
char *name; /* scheduler name */
@@ -781,9 +750,7 @@ struct ip_vs_pe {
int (*show_pe_data)(const struct ip_vs_conn *cp, char *buf);
};
-/*
- * The application module object (a.k.a. app incarnation)
- */
+/* The application module object (a.k.a. app incarnation) */
struct ip_vs_app {
struct list_head a_list; /* member in app list */
int type; /* IP_VS_APP_TYPE_xxx */
@@ -799,16 +766,14 @@ struct ip_vs_app {
atomic_t usecnt; /* usage counter */
struct rcu_head rcu_head;
- /*
- * output hook: Process packet in inout direction, diff set for TCP.
+ /* output hook: Process packet in inout direction, diff set for TCP.
* Return: 0=Error, 1=Payload Not Mangled/Mangled but checksum is ok,
* 2=Mangled but checksum was not updated
*/
int (*pkt_out)(struct ip_vs_app *, struct ip_vs_conn *,
struct sk_buff *, int *diff);
- /*
- * input hook: Process packet in outin direction, diff set for TCP.
+ /* input hook: Process packet in outin direction, diff set for TCP.
* Return: 0=Error, 1=Payload Not Mangled/Mangled but checksum is ok,
* 2=Mangled but checksum was not updated
*/
@@ -867,9 +832,7 @@ struct ipvs_master_sync_state {
struct netns_ipvs {
int gen; /* Generation */
int enable; /* enable like nf_hooks do */
- /*
- * Hash table: for real service lookups
- */
+ /* Hash table: for real service lookups */
#define IP_VS_RTAB_BITS 4
#define IP_VS_RTAB_SIZE (1 << IP_VS_RTAB_BITS)
#define IP_VS_RTAB_MASK (IP_VS_RTAB_SIZE - 1)
@@ -903,7 +866,7 @@ struct netns_ipvs {
struct list_head sctp_apps[SCTP_APP_TAB_SIZE];
#endif
/* ip_vs_conn */
- atomic_t conn_count; /* connection counter */
+ atomic_t conn_count; /* connection counter */
/* ip_vs_ctl */
struct ip_vs_stats tot_stats; /* Statistics & est. */
@@ -992,7 +955,8 @@ struct netns_ipvs {
struct net *net; /* Needed by timer routines */
/* Number of heterogeneous destinations, needed because
* heterogeneous are not supported when synchronization is
- * enabled */
+ * enabled
+ */
unsigned int mixed_address_family_dests;
};
@@ -1147,9 +1111,8 @@ static inline int sysctl_backup_only(struct netns_ipvs *ipvs)
#endif
-/*
- * IPVS core functions
- * (from ip_vs_core.c)
+/* IPVS core functions
+ * (from ip_vs_core.c)
*/
const char *ip_vs_proto_name(unsigned int proto);
void ip_vs_init_hash_table(struct list_head *table, int rows);
@@ -1157,11 +1120,9 @@ void ip_vs_init_hash_table(struct list_head *table, int rows);
#define IP_VS_APP_TYPE_FTP 1
-/*
- * ip_vs_conn handling functions
- * (from ip_vs_conn.c)
+/* ip_vs_conn handling functions
+ * (from ip_vs_conn.c)
*/
-
enum {
IP_VS_DIR_INPUT = 0,
IP_VS_DIR_OUTPUT,
@@ -1292,9 +1253,7 @@ ip_vs_control_add(struct ip_vs_conn *cp, struct ip_vs_conn *ctl_cp)
atomic_inc(&ctl_cp->n_control);
}
-/*
- * IPVS netns init & cleanup functions
- */
+/* IPVS netns init & cleanup functions */
int ip_vs_estimator_net_init(struct net *net);
int ip_vs_control_net_init(struct net *net);
int ip_vs_protocol_net_init(struct net *net);
@@ -1309,9 +1268,8 @@ void ip_vs_estimator_net_cleanup(struct net *net);
void ip_vs_sync_net_cleanup(struct net *net);
void ip_vs_service_net_cleanup(struct net *net);
-/*
- * IPVS application functions
- * (from ip_vs_app.c)
+/* IPVS application functions
+ * (from ip_vs_app.c)
*/
#define IP_VS_APP_MAX_PORTS 8
struct ip_vs_app *register_ip_vs_app(struct net *net, struct ip_vs_app *app);
@@ -1331,9 +1289,7 @@ int unregister_ip_vs_pe(struct ip_vs_pe *pe);
struct ip_vs_pe *ip_vs_pe_getbyname(const char *name);
struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name);
-/*
- * Use a #define to avoid all of module.h just for these trivial ops
- */
+/* Use a #define to avoid all of module.h just for these trivial ops */
#define ip_vs_pe_get(pe) \
if (pe && pe->module) \
__module_get(pe->module);
@@ -1342,9 +1298,7 @@ struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name);
if (pe && pe->module) \
module_put(pe->module);
-/*
- * IPVS protocol functions (from ip_vs_proto.c)
- */
+/* IPVS protocol functions (from ip_vs_proto.c) */
int ip_vs_protocol_init(void);
void ip_vs_protocol_cleanup(void);
void ip_vs_protocol_timeout_change(struct netns_ipvs *ipvs, int flags);
@@ -1362,9 +1316,8 @@ extern struct ip_vs_protocol ip_vs_protocol_esp;
extern struct ip_vs_protocol ip_vs_protocol_ah;
extern struct ip_vs_protocol ip_vs_protocol_sctp;
-/*
- * Registering/unregistering scheduler functions
- * (from ip_vs_sched.c)
+/* Registering/unregistering scheduler functions
+ * (from ip_vs_sched.c)
*/
int register_ip_vs_scheduler(struct ip_vs_scheduler *scheduler);
int unregister_ip_vs_scheduler(struct ip_vs_scheduler *scheduler);
@@ -1384,9 +1337,7 @@ int ip_vs_leave(struct ip_vs_service *svc, struct sk_buff *skb,
void ip_vs_scheduler_err(struct ip_vs_service *svc, const char *msg);
-/*
- * IPVS control data and functions (from ip_vs_ctl.c)
- */
+/* IPVS control data and functions (from ip_vs_ctl.c) */
extern struct ip_vs_stats ip_vs_stats;
extern int sysctl_ip_vs_sync_ver;
@@ -1427,26 +1378,21 @@ static inline void ip_vs_dest_put_and_free(struct ip_vs_dest *dest)
kfree(dest);
}
-/*
- * IPVS sync daemon data and function prototypes
- * (from ip_vs_sync.c)
+/* IPVS sync daemon data and function prototypes
+ * (from ip_vs_sync.c)
*/
int start_sync_thread(struct net *net, int state, char *mcast_ifn, __u8 syncid);
int stop_sync_thread(struct net *net, int state);
void ip_vs_sync_conn(struct net *net, struct ip_vs_conn *cp, int pkts);
-/*
- * IPVS rate estimator prototypes (from ip_vs_est.c)
- */
+/* IPVS rate estimator prototypes (from ip_vs_est.c) */
void ip_vs_start_estimator(struct net *net, struct ip_vs_stats *stats);
void ip_vs_stop_estimator(struct net *net, struct ip_vs_stats *stats);
void ip_vs_zero_estimator(struct ip_vs_stats *stats);
void ip_vs_read_estimator(struct ip_vs_stats_user *dst,
struct ip_vs_stats *stats);
-/*
- * Various IPVS packet transmitters (from ip_vs_xmit.c)
- */
+/* Various IPVS packet transmitters (from ip_vs_xmit.c) */
int ip_vs_null_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
struct ip_vs_protocol *pp, struct ip_vs_iphdr *iph);
int ip_vs_bypass_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
@@ -1477,10 +1423,9 @@ int ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
#endif
#ifdef CONFIG_SYSCTL
-/*
- * This is a simple mechanism to ignore packets when
- * we are loaded. Just set ip_vs_drop_rate to 'n' and
- * we start to drop 1/rate of the packets
+/* This is a simple mechanism to ignore packets when
+ * we are loaded. Just set ip_vs_drop_rate to 'n' and
+ * we start to drop 1/rate of the packets
*/
static inline int ip_vs_todrop(struct netns_ipvs *ipvs)
@@ -1496,9 +1441,7 @@ static inline int ip_vs_todrop(struct netns_ipvs *ipvs)
static inline int ip_vs_todrop(struct netns_ipvs *ipvs) { return 0; }
#endif
-/*
- * ip_vs_fwd_tag returns the forwarding tag of the connection
- */
+/* ip_vs_fwd_tag returns the forwarding tag of the connection */
#define IP_VS_FWD_METHOD(cp) (cp->flags & IP_VS_CONN_F_FWD_MASK)
static inline char ip_vs_fwd_tag(struct ip_vs_conn *cp)
@@ -1557,9 +1500,7 @@ static inline __wsum ip_vs_check_diff2(__be16 old, __be16 new, __wsum oldsum)
return csum_partial(diff, sizeof(diff), oldsum);
}
-/*
- * Forget current conntrack (unconfirmed) and attach notrack entry
- */
+/* Forget current conntrack (unconfirmed) and attach notrack entry */
static inline void ip_vs_notrack(struct sk_buff *skb)
{
#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
@@ -1576,8 +1517,7 @@ static inline void ip_vs_notrack(struct sk_buff *skb)
}
#ifdef CONFIG_IP_VS_NFCT
-/*
- * Netfilter connection tracking
+/* Netfilter connection tracking
* (from ip_vs_nfct.c)
*/
static inline int ip_vs_conntrack_enabled(struct netns_ipvs *ipvs)
@@ -1623,8 +1563,7 @@ static inline void ip_vs_conn_drop_conntrack(struct ip_vs_conn *cp)
static inline int
ip_vs_dest_conn_overhead(struct ip_vs_dest *dest)
{
- /*
- * We think the overhead of processing active connections is 256
+ /* We think the overhead of processing active connections is 256
* times higher than that of inactive connections in average. (This
* 256 times might not be accurate, we will change it later) We
* use the following formula to estimate the overhead now:
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 1/2] bpf: add search pruning optimization to verifier
From: Alexei Starovoitov @ 2014-09-30 1:50 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, netdev, linux-kernel
In-Reply-To: <1412041802-24858-1-git-send-email-ast@plumgrid.com>
consider C program represented in eBPF:
int filter(int arg)
{
int a, b, c, *ptr;
if (arg == 1)
ptr = &a;
else if (arg == 2)
ptr = &b;
else
ptr = &c;
*ptr = 0;
return 0;
}
eBPF verifier has to follow all possible paths through the program
to recognize that '*ptr = 0' instruction would be safe to execute
in all situations.
It's doing it by picking a path towards the end and observes changes
to registers and stack at every insn until it reaches bpf_exit.
Then it comes back to one of the previous branches and goes towards
the end again with potentially different values in registers.
When program has a lot of branches, the number of possible combinations
of branches is huge, so verifer has a hard limit of walking no more
than 32k instructions. This limit can be reached and complex (but valid)
programs could be rejected. Therefore it's important to recognize equivalent
verifier states to prune this depth first search.
Basic idea can be illustrated by the program (where .. are some eBPF insns):
1: ..
2: if (rX == rY) goto 4
3: ..
4: ..
5: ..
6: bpf_exit
In the first pass towards bpf_exit the verifier will walk insns: 1, 2, 3, 4, 5, 6
Since insn#2 is a branch the verifier will remember its state in verifier stack
to come back to it later.
Since insn#4 is marked as 'branch target', the verifier will remember its state
in explored_states[4] linked list.
Once it reaches insn#6 successfully it will pop the state recorded at insn#2 and
will continue.
Without search pruning optimization verifier would have to walk 4, 5, 6 again,
effectively simulating execution of insns 1, 2, 4, 5, 6
With search pruning it will check whether state at #4 after jumping from #2
is equivalent to one recorded in explored_states[4] during first pass.
If there is an equivalent state, verifier can prune the search at #4 and declare
this path to be safe as well.
In other words two states at #4 are equivalent if execution of 1, 2, 3, 4 insns
and 1, 2, 4 insns produces equivalent registers and stack.
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
kernel/bpf/verifier.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 146 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a086dd3210a8..801f5f3b9307 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -199,6 +199,7 @@ struct verifier_env {
struct verifier_stack_elem *head; /* stack of verifier states to be processed */
int stack_size; /* number of states to be processed */
struct verifier_state cur_state; /* current verifier state */
+ struct verifier_state_list **explored_states; /* search pruning optimization */
struct bpf_map *used_maps[MAX_USED_MAPS]; /* array of map's used by eBPF program */
u32 used_map_cnt; /* number of used maps */
};
@@ -1219,6 +1220,8 @@ enum {
BRANCH = 2,
};
+#define STATE_LIST_MARK ((struct verifier_state_list *) -1L)
+
static int *insn_stack; /* stack of insns to process */
static int cur_stack; /* current stack index */
static int *insn_state;
@@ -1241,6 +1244,10 @@ static int push_insn(int t, int w, int e, struct verifier_env *env)
return -EINVAL;
}
+ if (e == BRANCH)
+ /* mark branch target for state pruning */
+ env->explored_states[w] = STATE_LIST_MARK;
+
if (insn_state[w] == 0) {
/* tree-edge */
insn_state[t] = DISCOVERED | e;
@@ -1314,6 +1321,10 @@ peek_stack:
goto peek_stack;
else if (ret < 0)
goto err_free;
+ /* tell verifier to check for equivalent states
+ * after every call and jump
+ */
+ env->explored_states[t + 1] = STATE_LIST_MARK;
} else {
/* conditional jump with two edges */
ret = push_insn(t, t + 1, FALLTHROUGH, env);
@@ -1364,6 +1375,95 @@ err_free:
return ret;
}
+/* compare two verifier states
+ *
+ * all states stored in state_list are known to be valid, since
+ * verifier reached 'bpf_exit' instruction through them
+ *
+ * this function is called when verifier exploring different branches of
+ * execution popped from the state stack. If it sees an old state that has
+ * more strict register state and more strict stack state then this execution
+ * branch doesn't need to be explored further, since verifier already
+ * concluded that more strict state leads to valid finish.
+ *
+ * Therefore two states are equivalent if register state is more conservative
+ * and explored stack state is more conservative than the current one.
+ * Example:
+ * explored current
+ * (slot1=INV slot2=MISC) == (slot1=MISC slot2=MISC)
+ * (slot1=MISC slot2=MISC) != (slot1=INV slot2=MISC)
+ *
+ * In other words if current stack state (one being explored) has more
+ * valid slots than old one that already passed validation, it means
+ * the verifier can stop exploring and conclude that current state is valid too
+ *
+ * Similarly with registers. If explored state has register type as invalid
+ * whereas register type in current state is meaningful, it means that
+ * the current state will reach 'bpf_exit' instruction safely
+ */
+static bool states_equal(struct verifier_state *old, struct verifier_state *cur)
+{
+ int i;
+
+ for (i = 0; i < MAX_BPF_REG; i++) {
+ if (memcmp(&old->regs[i], &cur->regs[i],
+ sizeof(old->regs[0])) != 0) {
+ if (old->regs[i].type == NOT_INIT ||
+ old->regs[i].type == UNKNOWN_VALUE)
+ continue;
+ return false;
+ }
+ }
+
+ for (i = 0; i < MAX_BPF_STACK; i++) {
+ if (memcmp(&old->stack[i], &cur->stack[i],
+ sizeof(old->stack[0])) != 0) {
+ if (old->stack[i].stype == STACK_INVALID)
+ continue;
+ return false;
+ }
+ }
+ return true;
+}
+
+static int is_state_visited(struct verifier_env *env, int insn_idx)
+{
+ struct verifier_state_list *new_sl;
+ struct verifier_state_list *sl;
+
+ sl = env->explored_states[insn_idx];
+ if (!sl)
+ /* this 'insn_idx' instruction wasn't marked, so we will not
+ * be doing state search here
+ */
+ return 0;
+
+ while (sl != STATE_LIST_MARK) {
+ if (states_equal(&sl->state, &env->cur_state))
+ /* reached equivalent register/stack state,
+ * prune the search
+ */
+ return 1;
+ sl = sl->next;
+ }
+
+ /* there were no equivalent states, remember current one.
+ * technically the current state is not proven to be safe yet,
+ * but it will either reach bpf_exit (which means it's safe) or
+ * it will be rejected. Since there are no loops, we won't be
+ * seeing this 'insn_idx' instruction again on the way to bpf_exit
+ */
+ new_sl = kmalloc(sizeof(struct verifier_state_list), GFP_USER);
+ if (!new_sl)
+ return -ENOMEM;
+
+ /* add new state to the head of linked list */
+ memcpy(&new_sl->state, &env->cur_state, sizeof(env->cur_state));
+ new_sl->next = env->explored_states[insn_idx];
+ env->explored_states[insn_idx] = new_sl;
+ return 0;
+}
+
static int do_check(struct verifier_env *env)
{
struct verifier_state *state = &env->cur_state;
@@ -1396,6 +1496,21 @@ static int do_check(struct verifier_env *env)
return -E2BIG;
}
+ err = is_state_visited(env, insn_idx);
+ if (err < 0)
+ return err;
+ if (err == 1) {
+ /* found equivalent state, can prune the search */
+ if (log_level) {
+ if (do_print_state)
+ verbose("\nfrom %d to %d: safe\n",
+ prev_insn_idx, insn_idx);
+ else
+ verbose("%d: safe\n", insn_idx);
+ }
+ goto process_bpf_exit;
+ }
+
if (log_level && do_print_state) {
verbose("\nfrom %d to %d:", prev_insn_idx, insn_idx);
print_verifier_state(env);
@@ -1531,6 +1646,7 @@ static int do_check(struct verifier_env *env)
if (err)
return err;
+process_bpf_exit:
insn_idx = pop_stack(env, &prev_insn_idx);
if (insn_idx < 0) {
break;
@@ -1671,6 +1787,28 @@ static void convert_pseudo_ld_imm64(struct verifier_env *env)
insn->src_reg = 0;
}
+static void free_states(struct verifier_env *env)
+{
+ struct verifier_state_list *sl, *sln;
+ int i;
+
+ if (!env->explored_states)
+ return;
+
+ for (i = 0; i < env->prog->len; i++) {
+ sl = env->explored_states[i];
+
+ if (sl)
+ while (sl != STATE_LIST_MARK) {
+ sln = sl->next;
+ kfree(sl);
+ sl = sln;
+ }
+ }
+
+ kfree(env->explored_states);
+}
+
int bpf_check(struct bpf_prog *prog, union bpf_attr *attr)
{
char __user *log_ubuf = NULL;
@@ -1719,6 +1857,13 @@ int bpf_check(struct bpf_prog *prog, union bpf_attr *attr)
if (ret < 0)
goto skip_full_check;
+ env->explored_states = kcalloc(prog->len,
+ sizeof(struct verifier_state_list *),
+ GFP_USER);
+ ret = -ENOMEM;
+ if (!env->explored_states)
+ goto skip_full_check;
+
ret = check_cfg(env);
if (ret < 0)
goto skip_full_check;
@@ -1727,6 +1872,7 @@ int bpf_check(struct bpf_prog *prog, union bpf_attr *attr)
skip_full_check:
while (pop_stack(env, NULL) >= 0);
+ free_states(env);
if (log_level && log_len >= log_size - 1) {
BUG_ON(log_len >= log_size);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 1/1] net: fec: implement rx_copybreak to improve rx performance
From: Fugang Duan @ 2014-09-30 1:28 UTC (permalink / raw)
To: davem, b20596; +Cc: netdev, bhutchings, shawn.guo, romieu, eric.dumazet, b38611
- Copy short frames and keep the buffers mapped, re-allocate skb instead of
memory copy for long frames.
- Add support for setting/getting rx_copybreak using generic ethtool tunable
Changes V3:
* As Eric Dumazet's suggestion that removing the copybreak module parameter
and only keep the ethtool API support for rx_copybreak.
Changes V2:
* Implements rx_copybreak
* Rx_copybreak provides module parameter to change this value
* Add tunable_ops support for rx_copybreak
Signed-off-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
drivers/net/ethernet/freescale/fec.h | 2 +
drivers/net/ethernet/freescale/fec_main.c | 208 ++++++++++++++++++++---------
2 files changed, 145 insertions(+), 65 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 354a309..1d5e182 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -482,6 +482,8 @@ struct fec_enet_private {
unsigned int tx_pkts_itr;
unsigned int tx_time_itr;
unsigned int itr_clk_rate;
+
+ u32 rx_copybreak;
};
void fec_ptp_init(struct platform_device *pdev);
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 2c73434..1f07db8 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -236,6 +236,8 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
#define FEC_PAUSE_FLAG_AUTONEG 0x1
#define FEC_PAUSE_FLAG_ENABLE 0x2
+#define COPYBREAK_DEFAULT 256
+
#define TSO_HEADER_SIZE 128
/* Max number of allowed TCP segments for software TSO */
#define FEC_MAX_TSO_SEGS 100
@@ -1322,6 +1324,50 @@ fec_enet_tx(struct net_device *ndev)
return;
}
+static int
+fec_enet_new_rxbdp(struct net_device *ndev, struct bufdesc *bdp, struct sk_buff *skb)
+{
+ struct fec_enet_private *fep = netdev_priv(ndev);
+ int off;
+
+ off = ((unsigned long)skb->data) & fep->rx_align;
+ if (off)
+ skb_reserve(skb, fep->rx_align + 1 - off);
+
+ bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, skb->data,
+ FEC_ENET_RX_FRSIZE - fep->rx_align,
+ DMA_FROM_DEVICE);
+ if (dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr)) {
+ if (net_ratelimit())
+ netdev_err(ndev, "Rx DMA memory map failed\n");
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static bool fec_enet_copybreak(struct net_device *ndev, struct sk_buff **skb,
+ struct bufdesc *bdp, u32 length)
+{
+ struct fec_enet_private *fep = netdev_priv(ndev);
+ struct sk_buff *new_skb;
+
+ if (length > fep->rx_copybreak)
+ return false;
+
+ new_skb = netdev_alloc_skb(ndev, length);
+ if (!new_skb)
+ return false;
+
+ dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
+ FEC_ENET_RX_FRSIZE - fep->rx_align,
+ DMA_FROM_DEVICE);
+ memcpy(new_skb->data, (*skb)->data, length);
+ *skb = new_skb;
+
+ return true;
+}
+
/* During a receive, the cur_rx points to the current incoming buffer.
* When we update through the ring, if the next incoming buffer has
* not been given to the system, we just set the empty indicator,
@@ -1336,7 +1382,8 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
struct fec_enet_priv_rx_q *rxq;
struct bufdesc *bdp;
unsigned short status;
- struct sk_buff *skb;
+ struct sk_buff *skb_new = NULL;
+ struct sk_buff *skb;
ushort pkt_len;
__u8 *data;
int pkt_received = 0;
@@ -1344,6 +1391,7 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
bool vlan_packet_rcvd = false;
u16 vlan_tag;
int index = 0;
+ bool is_copybreak;
#ifdef CONFIG_M532x
flush_cache_all();
@@ -1401,11 +1449,27 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
ndev->stats.rx_bytes += pkt_len;
index = fec_enet_get_bd_index(rxq->rx_bd_base, bdp, fep);
- data = rxq->rx_skbuff[index]->data;
- dma_sync_single_for_cpu(&fep->pdev->dev, bdp->cbd_bufaddr,
- FEC_ENET_RX_FRSIZE - fep->rx_align,
- DMA_FROM_DEVICE);
+ skb = rxq->rx_skbuff[index];
+ /* The packet length includes FCS, but we don't want to
+ * include that when passing upstream as it messes up
+ * bridging applications.
+ */
+ is_copybreak = fec_enet_copybreak(ndev, &skb, bdp, pkt_len - 4);
+ if (!is_copybreak) {
+ skb_new = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE);
+ if (unlikely(!skb_new)) {
+ ndev->stats.rx_dropped++;
+ goto rx_processing_done;
+ }
+ dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
+ FEC_ENET_RX_FRSIZE - fep->rx_align,
+ DMA_FROM_DEVICE);
+ }
+
+ prefetch(skb->data - NET_IP_ALIGN);
+ skb_put(skb, pkt_len - 4);
+ data = skb->data;
if (id_entry->driver_data & FEC_QUIRK_SWAP_FRAME)
swap_buffer(data, pkt_len);
@@ -1422,62 +1486,48 @@ fec_enet_rx_queue(struct net_device *ndev, int budget, u16 queue_id)
struct vlan_hdr *vlan_header =
(struct vlan_hdr *) (data + ETH_HLEN);
vlan_tag = ntohs(vlan_header->h_vlan_TCI);
- pkt_len -= VLAN_HLEN;
vlan_packet_rcvd = true;
+
+ skb_copy_to_linear_data_offset(skb, VLAN_HLEN,
+ data, (2 * ETH_ALEN));
+ skb_pull(skb, VLAN_HLEN);
}
- /* This does 16 byte alignment, exactly what we need.
- * The packet length includes FCS, but we don't want to
- * include that when passing upstream as it messes up
- * bridging applications.
- */
- skb = netdev_alloc_skb(ndev, pkt_len - 4 + NET_IP_ALIGN);
+ skb->protocol = eth_type_trans(skb, ndev);
- if (unlikely(!skb)) {
- ndev->stats.rx_dropped++;
- } else {
- int payload_offset = (2 * ETH_ALEN);
- skb_reserve(skb, NET_IP_ALIGN);
- skb_put(skb, pkt_len - 4); /* Make room */
-
- /* Extract the frame data without the VLAN header. */
- skb_copy_to_linear_data(skb, data, (2 * ETH_ALEN));
- if (vlan_packet_rcvd)
- payload_offset = (2 * ETH_ALEN) + VLAN_HLEN;
- skb_copy_to_linear_data_offset(skb, (2 * ETH_ALEN),
- data + payload_offset,
- pkt_len - 4 - (2 * ETH_ALEN));
-
- skb->protocol = eth_type_trans(skb, ndev);
-
- /* Get receive timestamp from the skb */
- if (fep->hwts_rx_en && fep->bufdesc_ex)
- fec_enet_hwtstamp(fep, ebdp->ts,
- skb_hwtstamps(skb));
-
- if (fep->bufdesc_ex &&
- (fep->csum_flags & FLAG_RX_CSUM_ENABLED)) {
- if (!(ebdp->cbd_esc & FLAG_RX_CSUM_ERROR)) {
- /* don't check it */
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- } else {
- skb_checksum_none_assert(skb);
- }
+ /* Get receive timestamp from the skb */
+ if (fep->hwts_rx_en && fep->bufdesc_ex)
+ fec_enet_hwtstamp(fep, ebdp->ts,
+ skb_hwtstamps(skb));
+
+ if (fep->bufdesc_ex &&
+ (fep->csum_flags & FLAG_RX_CSUM_ENABLED)) {
+ if (!(ebdp->cbd_esc & FLAG_RX_CSUM_ERROR)) {
+ /* don't check it */
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ } else {
+ skb_checksum_none_assert(skb);
}
+ }
- /* Handle received VLAN packets */
- if (vlan_packet_rcvd)
- __vlan_hwaccel_put_tag(skb,
- htons(ETH_P_8021Q),
- vlan_tag);
+ /* Handle received VLAN packets */
+ if (vlan_packet_rcvd)
+ __vlan_hwaccel_put_tag(skb,
+ htons(ETH_P_8021Q),
+ vlan_tag);
- napi_gro_receive(&fep->napi, skb);
+ napi_gro_receive(&fep->napi, skb);
+
+ if (is_copybreak) {
+ dma_sync_single_for_device(&fep->pdev->dev, bdp->cbd_bufaddr,
+ FEC_ENET_RX_FRSIZE - fep->rx_align,
+ DMA_FROM_DEVICE);
+ } else {
+ rxq->rx_skbuff[index] = skb_new;
+ fec_enet_new_rxbdp(ndev, bdp, skb_new);
}
- dma_sync_single_for_device(&fep->pdev->dev, bdp->cbd_bufaddr,
- FEC_ENET_RX_FRSIZE - fep->rx_align,
- DMA_FROM_DEVICE);
rx_processing_done:
/* Clear the status flags for this buffer */
status &= ~BD_ENET_RX_STATS;
@@ -2392,6 +2442,44 @@ static void fec_enet_itr_coal_init(struct net_device *ndev)
fec_enet_set_coalesce(ndev, &ec);
}
+static int fec_enet_get_tunable(struct net_device *netdev,
+ const struct ethtool_tunable *tuna,
+ void *data)
+{
+ struct fec_enet_private *fep = netdev_priv(netdev);
+ int ret = 0;
+
+ switch (tuna->id) {
+ case ETHTOOL_RX_COPYBREAK:
+ *(u32 *)data = fep->rx_copybreak;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+static int fec_enet_set_tunable(struct net_device *netdev,
+ const struct ethtool_tunable *tuna,
+ const void *data)
+{
+ struct fec_enet_private *fep = netdev_priv(netdev);
+ int ret = 0;
+
+ switch (tuna->id) {
+ case ETHTOOL_RX_COPYBREAK:
+ fep->rx_copybreak = *(u32 *)data;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
static const struct ethtool_ops fec_enet_ethtool_ops = {
.get_settings = fec_enet_get_settings,
.set_settings = fec_enet_set_settings,
@@ -2408,6 +2496,8 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
.get_sset_count = fec_enet_get_sset_count,
#endif
.get_ts_info = fec_enet_get_ts_info,
+ .get_tunable = fec_enet_get_tunable,
+ .set_tunable = fec_enet_set_tunable,
};
static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
@@ -2553,33 +2643,20 @@ fec_enet_alloc_rxq_buffers(struct net_device *ndev, unsigned int queue)
struct sk_buff *skb;
struct bufdesc *bdp;
struct fec_enet_priv_rx_q *rxq;
- unsigned int off;
rxq = fep->rx_queue[queue];
bdp = rxq->rx_bd_base;
for (i = 0; i < rxq->rx_ring_size; i++) {
- dma_addr_t addr;
-
skb = netdev_alloc_skb(ndev, FEC_ENET_RX_FRSIZE);
if (!skb)
goto err_alloc;
- off = ((unsigned long)skb->data) & fep->rx_align;
- if (off)
- skb_reserve(skb, fep->rx_align + 1 - off);
-
- addr = dma_map_single(&fep->pdev->dev, skb->data,
- FEC_ENET_RX_FRSIZE - fep->rx_align, DMA_FROM_DEVICE);
-
- if (dma_mapping_error(&fep->pdev->dev, addr)) {
+ if (fec_enet_new_rxbdp(ndev, bdp, skb)) {
dev_kfree_skb(skb);
- if (net_ratelimit())
- netdev_err(ndev, "Rx DMA memory map failed\n");
goto err_alloc;
}
rxq->rx_skbuff[i] = skb;
- bdp->cbd_bufaddr = addr;
bdp->cbd_sc = BD_ENET_RX_EMPTY;
if (fep->bufdesc_ex) {
@@ -3240,6 +3317,7 @@ fec_probe(struct platform_device *pdev)
if (fep->bufdesc_ex && fep->ptp_clock)
netdev_info(ndev, "registered PHC device %d\n", fep->dev_id);
+ fep->rx_copybreak = COPYBREAK_DEFAULT;
INIT_WORK(&fep->tx_timeout_work, fec_enet_timeout_work);
return 0;
--
1.7.8
^ permalink raw reply related
* Re: [PATCH] net: Add ndo_gso_check
From: Tom Herbert @ 2014-09-30 1:59 UTC (permalink / raw)
To: Jesse Gross; +Cc: David Miller, netdev, Or Gerlitz
In-Reply-To: <CAEP_g=9gEhR3UKNe9aVJq8Prmg3awm6fDYHkZy_CumHKFkNNuA@mail.gmail.com>
On Mon, Sep 29, 2014 at 5:33 PM, Jesse Gross <jesse@nicira.com> wrote:
> On Mon, Sep 29, 2014 at 1:47 PM, Tom Herbert <therbert@google.com> wrote:
>> On Mon, Sep 29, 2014 at 1:13 PM, Jesse Gross <jesse@nicira.com> wrote:
>>> On Sun, Sep 28, 2014 at 8:50 PM, Tom Herbert <therbert@google.com> wrote:
>>>> Add ndo_gso_check which a device can define to indicate whether is
>>>> is capable of doing GSO on a packet. This funciton would be called from
>>>> the stack to determine whether software GSO is needed to be done. A
>>>> driver should populate this function if it advertises GSO types for
>>>> which there are combinations that it wouldn't be able to handle. For
>>>> instance a device that performs UDP tunneling might only implement
>>>> support for transparent Ethernet bridging type of inner packets
>>>> or might have limitations on lengths of inner headers.
>>>>
>>>> Signed-off-by: Tom Herbert <therbert@google.com>
>>>
>>> Offloads that might have limitations extend beyond GSO - checksum is
>>> another possibility that could need something like a length check.
>>>
>> There are several examples of drivers that advertise checksum offload
>> but then in TX path decide that they can't do it for various reasons--
>> in this case they need to do software checksum calculation themselves.
>> Applying this model to GSO, that is drivers do software GSO when they
>> need to punt on packet, seems pretty hard to me, but maybe someone can
>> look at it.
>
> Yes, I definitely think that it's easier to have GSO done in the
> stack. I agree with you that exposing various capabilities,
> limitations, etc. for different features will get messy very quickly,
> so I guess this model makes sense for now.
>
>>> In addition, while I would also like to be optimistic about the
>>> capabilities of existing NICs it's unlikely that any of them that are
>>> advertising SKB_GSO_UDP_TUNNEL can actually do it with full
>>> generality. So unless we can get the driver writers to chime in about
>>> their capabilities (maybe we can, there's only a handful of them right
>>> now), we probably need to provide a more conservative implementation
>>> for those drivers. I guess I would probably do length equal to VXLAN
>>> and perhaps containing Ethernet as a balance between the most
>>> conservative and the most optimistic.
>>
>> Checking header length in ndo_gso_check would be trivial.
>
> I agree - I just think that we need to provide some implementations in
> existing drivers to do this. Otherwise, as we introduce new protocols
> they will probably be broken in some situations.
A large part of the problem is that potential driver interfaces
(around GSO, checksum) are not clearly specified and especially with
encapsulation the meanings may not be intuitively obvious. For
instance, there's not a single comment in skbuff.h about what any of
the GSO constants mean. I think this is something that we should
address!
^ permalink raw reply
* [PATCH] bridge: Fix redefined references
From: Tom Herbert @ 2014-09-30 2:16 UTC (permalink / raw)
To: davem, pablo, netdev
Commit 34666d467cbf1e2e3 ("netfilter: bridge: move br_netfilter out of
the core") cause compiler errors when CONFIG_BRIDGE_NETFILTER is not
defined.
net/bridge/br_nf_core.c:77:1: error: expected identifier or '(' before '{' token
net/bridge/br_nf_core.c:88:115: error: redefinition of 'br_nf_core_init'
In file included from net/bridge/br_nf_core.c:23:0:
net/bridge/br_private.h:762:59: note: previous definition of 'br_nf_core_init' was here
net/bridge/br_nf_core.c:93:6: error: redefinition of 'br_nf_core_fini'
In file included from net/bridge/br_nf_core.c:23:0:
net/bridge/br_private.h:763:60: note: previous definition of 'br_nf_core_fini' was here
Add #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) around definitions of
the redefined functions.
Signed-off-by: Tom Herbert <therbert@google.com>
---
net/bridge/br_nf_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bridge/br_nf_core.c b/net/bridge/br_nf_core.c
index 387cb3b..ac4bea1 100644
--- a/net/bridge/br_nf_core.c
+++ b/net/bridge/br_nf_core.c
@@ -73,6 +73,7 @@ static const u32 br_dst_default_metrics[RTAX_MAX] = {
[RTAX_MTU - 1] = 1500,
};
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
void br_netfilter_rtable_init(struct net_bridge *br)
{
struct rtable *rt = &br->fake_rtable;
@@ -94,3 +95,4 @@ void br_nf_core_fini(void)
{
dst_entries_destroy(&fake_dst_ops);
}
+#endif
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [PATCH 18/34] ipvs: prevent mixing heterogeneous pools and synchronization
From: Simon Horman @ 2014-09-30 2:21 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Pablo Neira Ayuso, netfilter-devel, davem, netdev
In-Reply-To: <54298616.7080307@cogentembedded.com>
On Mon, Sep 29, 2014 at 08:17:26PM +0400, Sergei Shtylyov wrote:
> On 09/29/2014 04:39 PM, Pablo Neira Ayuso wrote:
>
> >From: Alex Gartrell <agartrell@fb.com>
>
> >The synchronization protocol is not compatible with heterogeneous pools, so
> >we need to verify that we're not turning both on at the same time.
>
> >Signed-off-by: Alex Gartrell <agartrell@fb.com>
> >Acked-by: Julian Anastasov <ja@ssi.bg>
> >Signed-off-by: Simon Horman <horms@verge.net.au>
> >---
> > include/net/ip_vs.h | 4 ++++
> > net/netfilter/ipvs/ip_vs_ctl.c | 15 +++++++++++++++
> > 2 files changed, 19 insertions(+)
>
> >diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> >index 7600dbe..576d7f0 100644
> >--- a/include/net/ip_vs.h
> >+++ b/include/net/ip_vs.h
> >@@ -990,6 +990,10 @@ struct netns_ipvs {
> > char backup_mcast_ifn[IP_VS_IFNAME_MAXLEN];
> > /* net name space ptr */
> > struct net *net; /* Needed by timer routines */
> >+ /* Number of heterogeneous destinations, needed because
> >+ * heterogeneous are not supported when synchronization is
> >+ * enabled */
>
> Multi-line comment style in the networking code is:
>
> /* bla
> * bla
> */
Thanks, I have sent a cleanup patch.
^ permalink raw reply
* Re: [PATCH v1 5/5] driver-core: add driver asynchronous probe support
From: Luis R. Rodriguez @ 2014-09-30 2:27 UTC (permalink / raw)
To: Tom Gundersen
Cc: Luis R. Rodriguez, Michal Hocko, Greg KH, Dmitry Torokhov,
Takashi Iwai, Tejun Heo, Arjan van de Ven, Robert Milasan, werner,
Oleg Nesterov, hare, Benjamin Poirier, Santosh Rastapur, pmladek,
dbueso, LKML, Tetsuo Handa, Joseph Salisbury, Kay Sievers,
One Thousand Gnomes, Tim Gardner, Pierre Fersing, Andrew Morton,
Nagalakshmi Nandigama
In-Reply-To: <CAG-2HqWmVODJ6Q46t96mS9ALbeFjRcQ18ypwejY=ug0wCUVv-Q@mail.gmail.com>
On Sun, Sep 28, 2014 at 07:07:24PM +0200, Tom Gundersen wrote:
> On Fri, Sep 26, 2014 at 11:57 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> > Systemd has a general timeout for all workers currently set to 180
> > seconds after which it will send a sigkill signal. Systemd now has a
> > warning which is issued once it reaches 1/3 of the timeout. The original
> > motivation for the systemd timeout was to help track device drivers
> > which do not use asynch firmware loading on init() and the timeout was
> > originally set to 30 seconds.
>
> Please note that the motivation for the timeout in systemd had nothing
> to do with async firmware loading (that was just the case where
> problems cropped up).
*Part *of the original kill logic, according to the commit log, was actually
due to the assumption that the issues observed *were* synchronous firmware
loading on module init():
commit e64fae5573e566ce4fd9b23c68ac8f3096603314
Author: Kay Sievers <kay.sievers@vrfy.org>
Date: Wed Jan 18 05:06:18 2012 +0100
udevd: kill hanging event processes after 30 seconds
Some broken kernel drivers load firmware synchronously in the module init
path and block modprobe until the firmware request is fulfilled.
<...>
My point here is not to point fingers but to explain why we went on with
this and how we failed to realize only until later that the driver core
ran probe together with init. When a few folks pointed out the issues
with the kill the issue was punted back to kernel developers and the
assumption even among some kernel maintainers was that it was init paths
with sync behaviour that was causing some delays and they were broken
drivers. It is important to highlight these assumptions ended up setting
us off on the wrong path for a while in a hunt to try to fix this issue
either in driver or elsewhere.
> The motivation was to not allow udev-workers to
> stay around indefinitely, and hence put an upper-bound on
> their duration (initially 180 s). At some point the bound was reduced
> to 30 seconds to make sure module-loading would bail out before the
> kernel's firmware loading timeout would bail out (60s I believe).
Sure, part of it was that, but folks beat on driver developer about
the kill insisting it was drivers that were broken. It was only until
Chelsie folks called bloody murder becuase their delays were on probe
that we realized there was a bit more to this than what was being pushed
back on to driver developers.
> That
> is no longer relevant, which is why it was safe to reset the timeout
> to 180 s.
Indeed :D
> > Since systemd + kernel are heavily tied in for the purposes of this
> > patch it is assumed you have merged on systemd the following
> > commits:
> >
> > 671174136525ddf208cdbe75d6d6bd159afa961f udev: timeout - warn after a third of the timeout before killing
> > b5338a19864ac3f5632aee48069a669479621dca udev: timeout - increase timeout
> > 2e92633dbae52f5ac9b7b2e068935990d475d2cd udev: bump event timeout to 60 seconds
> > be2ea723b1d023b3d385d3b791ee4607cbfb20ca udev: remove userspace firmware loading support
> > 9f20a8a376f924c8eb5423cfc1f98644fc1e2d1a udev: fixup commit
> > dd5eddd28a74a49607a8fffcaf960040dba98479 udev: unify event timeout handling
> > 9719859c07aa13539ed2cd4b31972cd30f678543 udevd: add --event-timeout commandline option
> >
> > Since we bundle together serially driver init() and probe()
> > on module initialiation systemd's imposed timeout put a limit on the
> > amount of time a driver init() and probe routines can take. There's a
> > few overlooked issues with this and the timeout in general:
> >
> > 0) Not all drivers are killed, the signal is just sent and
> > the kill will only be acted upoon if the driver you loaded
> > happens to have some code path that either uses kthreads (which
> > as of 786235ee are now killable), or uses some code which checks for
> > fatal_signal_pending() on the kernel somewhere -- i.e: pci_read_vpd().
>
> Shouldn't this be seen as something to be fixed in the kernel?
That's a great question. In practice now after CVE-2012-4398 and its series of
patches added which enabled OOM to kill things followed by 786235ee to also
handle OOM on kthreads it seems imperative we strive towards this, in practive
however if you're getting OOMs on boot you have far more serious issue to be
concerned over than handling CVE-2012-4398. Another issue is that even if we
wanted to address this a critical right now on module loading driver error
paths tend to be pretty buggy and we'd probably end up causing more issues than
fixing anything if the sigkill that triggered this was an arbitrary timeout,
specially if the timeout is not properly justified. Addressing sigkill due
to OOM is important, but as noted if you're running out of memory at load
time you have a bit other problems to be concerned over.
So extending the kill onto more drivers *because* of the timeout is probably
not a good reason as it would probably create more issue than fix anything
right now.
> I mean,
> do we not want userspace to have the possibility to kill udev/modprobe
> even disregarding the worker timeouts (say at shutdown, or before
> switching from the initrd)?
That's a good point and I think the merit to handle a kill due to the
other reasons (shutdown, switching from the initrd) should be addressed
separately. I mean that validating addressing the kill for the other
reasons does not validate the existing kill on timeout for synchronous
probing.
If its important to handle the kill on shutdown / switching initrd
that should be dealt with orthogonally.
> > 1) Since systemd is the only one logging the sigkill debugging that
> > drivers are not loaded or in the worst case *failed to boot* because
> > of a sigkill has proven hard to debug.
>
> Care to clarify this a bit? Are the udev logs somehow unclear?
Sure, so the problem is that folks debugging were not aware of what systemd was
doing. Let me be clear that the original 30 second sigkill timeout thing was
passed down onto driver maintainers as a non-documented new kernel policy
slap-in-the-face-you-must-obviously-be-doing-something-wrong (TM) approach.
This was a policy decision passed down as a *reactive* measure, not many folks
were aware of it and of what systemd was doing. What made the situation even worse
was that as noted on 1) even though the sigkill was being sent since commit
e64fae55 (January 2012) on systemd the sigkill was not being picked up on many
drivers. To be clear the sigkill was being picked up if you had a driver that
by chance had some code on init / probe that by chance checked for
fatal_signal_pending(), and even when that triggered folks debugging were in no
way shape or form expecting a sigkill from userspace on modprobe as it was not well
known that this was part of the policy they should follow. Shit started to hit
the fan a bit more widely when kernel commit 786235ee (Nov 2013) was merged
upstream which allowed kthreads to be killed, and more drivers started failing.
An example of an ancient bug that no one handled until recently:
https://bugzilla.kernel.org/show_bug.cgi?id=59581
There is a proper fix to this now but the kill was what was causing this
in the first place. The kill was justified as theese drivers *should*
be using async probe but by no means does that mean the kill was
justified for all subsystems / drivers. The bug also really also sent
people on the wrong track and it was only until Alexander poked me
about the issue we were seeing on cxbg4 likely being related that we
started to really zeroe in on the real issue.
The first driver reported / studied due to the kill from system was
mptsas:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1276705
A full bisect was done to even try to understand what the issue was..
Then there was the tug of war between either reverting the patch that
allowed the kthread to be killed or if this was systemd issue which
required increasing the timeout. This was still a storage driver,
and increasing the timeout arbitrarily really would not have helped
address the root cause of the issue.
The next non-storage driver bug that was reported and heavily
debugged was cxgb4 and it wasn't easy to debug:
https://bugzilla.suse.com/show_bug.cgi?id=877622
Conclusion then is that folks were simply not aware of this new de-facto
policy, it was obviously incorrect but well intentioned, and no one
really was paying attention to systemd-udevd logs. If we want chatty
behaviour that people will pick up we probably instead want a WARN()
on the kernel specially before we kill a driver and even then I'm sure
this can irritate some folks.
> If you think we can improve the logging from udev, please ping me about that
> and I'll sort it out.
I think the logging done on systemd is fine, there are a few issues with the
way things trickled down and what we now need to do. First and foremost there
was general communication issue about this new timing policy and obviously it
would have helped if this also had more design / review from others. Its no
one's fault, but we should learn from it. Design policies on systemd that can
affect the kernel / drivers could likely use some bit more review from a wider
audience and probably include folks who are probably going to be more critical
than those who likely would typically be favorable. Without wider review we
could fail to end up with something like a filter bubble [0] but applied to
engineering, a design filter bubble, if you will. So apart from addressing
logging its important to reflect on this issue and try to aim for having
something like a Red Team [1] on design involving systemd and kernel. This is
specially true if we are to really marry these two together more and more.
The more critical people can be the better, but of course those need to
provide constructive criticism, not just rants.
In terms of logging:
Do we know if distributions / users are reviewing systemd-udevd logs for
certain types of issues with as much dilligence as they put to kernel logs when
systemd makes decision affecting the kernel? If not we should consider a way so
that that happens. In this case the fact that drivers were being killed while
being loaded was missed since it was unexpected that would happen so folks
didn't know to look for that option, but apart from that the *reason* for the
kill probably could have helped too. To help both of these we have to consider if
we are going to keep the sigkill on systemd on module loading due to a timeout.
As you clarified the goal of the timeout is to avoid having udev workers stay
around indefinitely, but I think we need to give kmod workers a bit more
consideration. The point of this patch set was partly to give systemd what it
assumed was there, but clearly we can't assume all drivers can be loaded
asynchronously without issues right now. That means that even with this
functionality merged systemd will have to cope with the fact that some drivers
will be loaded with synchronous probe. A general timeout and specially with a
sigkill is probably not a good idea then, unless of course:
0) those device drivers / subsystem maintainer want a timeout
1) the above decision can distinguish between sync probe / async probe
being done
To address 0) perhaps one solution is that if subsystem maintainers
feel this is needed they can express this on data structure somewhere,
perhaps on the bus and/or have a driver value override, for example.
For 1) we could expose what we end up doing through sysfs.
Of course userspace could also simply want to put in place some
requirements but in terms of a timeout / kill it would have to also
accept that it cannot get what it might want. For instance we now know
it may be that an async probe is not possible on some drivers.
Perhaps its best to think about this differently and address now a
way to do that efficiently instead of reactively. Apart form having
the ability to let systemd ask for async probe, what else do we want
to accomplish?
[0] http://en.wikipedia.org/wiki/Filter_bubble
[1] http://en.wikipedia.org/wiki/Red_team
> > 2) When and if the signal is received by the driver somehow
> > the driver may fail at different points in its initialization
> > and unless all error paths on the driver are implemented
> > perfectly this could mean leaving a device in a half
> > initialized state.
> >
> > 3) The timeout is penalizing device drivers that take long on
> > probe(), this wasn't the original motivation. Systemd seems
> > to have been under assumption that probe was asynchronous,
> > this perhaps is true as an *objective* and goal for *some
> > subsystems* but by no means is it true that we've been on a wide
> > crusade to ensure this for all device drivers. It may be a good
> > idea for *many* device drivers but penalizing them with a kill
> > for taking long on probe is simply unacceptable specially
> > when the timeout is completely arbitrary.
>
> The point is really not to "penalize" anything, we just need to make
> sure we put some sort of restrictions on our workers so they don't
> hang around forever.
Thanks for clarifying this, can you explain what issues could arise
from making an exception to allowing kmod workers to hang around
completing init + probe over a certain defined amount of time without
being killed?
Luis
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2014-09-30 2:51 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-next, linux-kernel, hayeswang
[-- Attachment #1: Type: text/plain, Size: 1567 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
drivers/net/usb/r8152.c between commit 445f7f4d6262 ("r8152: fix the
carrier off when autoresuming") from the net tree and commit
b209af9981ee ("r8152: check code with checkpatch.pl") from the net-next
tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc drivers/net/usb/r8152.c
index e0394427e372,a4d4c4a1354f..000000000000
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@@ -2067,10 -2080,11 +2097,10 @@@ static void rtl_disable(struct r8152 *t
for (i = 0; i < 1000; i++) {
if (ocp_read_word(tp, MCU_TYPE_PLA, PLA_TCR0) & TCR0_TX_EMPTY)
break;
- mdelay(1);
+ usleep_range(1000, 2000);
}
- for (i = 0; i < RTL8152_MAX_RX; i++)
- usb_kill_urb(tp->rx_info[i].urb);
+ rtl_stop_rx(tp);
rtl8152_nic_reset(tp);
}
@@@ -3131,12 -3211,13 +3229,13 @@@ static int rtl8152_resume(struct usb_in
} else {
tp->rtl_ops.up(tp);
rtl8152_set_speed(tp, AUTONEG_ENABLE,
- tp->mii.supports_gmii ? SPEED_1000 : SPEED_100,
- DUPLEX_FULL);
+ tp->mii.supports_gmii ?
+ SPEED_1000 : SPEED_100,
+ DUPLEX_FULL);
+ tp->speed = 0;
+ netif_carrier_off(tp->netdev);
+ set_bit(WORK_ENABLE, &tp->flags);
}
- tp->speed = 0;
- netif_carrier_off(tp->netdev);
- set_bit(WORK_ENABLE, &tp->flags);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
}
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2014-09-30 2:54 UTC (permalink / raw)
To: David Miller, netdev; +Cc: linux-next, linux-kernel, Pablo Neira Ayuso
[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in
net/netfilter/nfnetlink.c between commit cbb8125eb40b ("netfilter:
nfnetlink: deliver netlink errors on batch completion") from the net
tree and commit fc04733a1a71 ("netfilter: nfnetlink: use original
skbuff when committing/aborting") from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary (no action
is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc net/netfilter/nfnetlink.c
index f37f0716a9fc,f77d3f7f22b5..000000000000
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@@ -380,8 -333,7 +380,8 @@@ replay
* original skb.
*/
if (err == -EAGAIN) {
+ nfnl_err_reset(&err_list);
- ss->abort(skb);
+ ss->abort(oskb);
nfnl_unlock(subsys_id);
kfree_skb(nskb);
goto replay;
@@@ -418,11 -357,10 +418,11 @@@ ack
}
done:
if (success && done)
- ss->commit(skb);
+ ss->commit(oskb);
else
- ss->abort(skb);
+ ss->abort(oskb);
+ nfnl_err_deliver(&err_list, oskb);
nfnl_unlock(subsys_id);
kfree_skb(nskb);
}
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ 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