* Re: [PATCH net-next iproute2 v2] iproute2: allow to change slave options via type_slave
From: Jiri Pirko @ 2014-09-03 16:04 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, stephen, davem
In-Reply-To: <1409759850-15812-1-git-send-email-nikolay@redhat.com>
Wed, Sep 03, 2014 at 05:57:30PM CEST, nikolay@redhat.com wrote:
>This patch adds the necessary changes to allow altering a slave device's
>options via ip link set <device> type <master type>_slave specific-option.
>It also adds support to set the bonding slaves' queue_id.
>
>Example:
> ip link set eth0 type bond_slave queue_id 10
>
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
Acked-by: Jiri Pirko <jiri@resnulli.us>
^ permalink raw reply
* [PATCH 2/2] net-timestamp: Make the clone operation stand-alone from phy timestamping
From: Alexander Duyck @ 2014-09-03 15:53 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <20140903155022.1896.59030.stgit@ahduyck-bv4.jf.intel.com>
The phy timestamping takes a different path than the regular timestamping
does in that it will create a clone first so that the packets needing to be
timestamped can be placed in a queue, or the context block could be used.
In order to support these use cases I am pulling the core of the code out
so it can be used in other drivers beyond just phy devices.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
include/linux/skbuff.h | 2 ++
net/core/skbuff.c | 20 ++++++++++++++++++++
net/core/timestamping.c | 14 +++-----------
3 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 02529fc..19eb787 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2690,6 +2690,8 @@ static inline ktime_t net_invalid_timestamp(void)
return ktime_set(0, 0);
}
+struct sk_buff *__skb_clone_tx_timestamp(struct sk_buff *skb);
+
#ifdef CONFIG_NETWORK_PHY_TIMESTAMPING
void skb_clone_tx_timestamp(struct sk_buff *skb);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 488d902..3f5cf29 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3511,6 +3511,26 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
}
EXPORT_SYMBOL(sock_dequeue_err_skb);
+struct sk_buff *__skb_clone_tx_timestamp(struct sk_buff *skb)
+{
+ struct sock *sk = skb->sk;
+ struct sk_buff *clone;
+
+ if (!sk || !atomic_inc_not_zero(&sk->sk_refcnt))
+ return NULL;
+
+ clone = skb_clone(skb, GFP_ATOMIC);
+ if (!clone) {
+ sock_put(sk);
+ return NULL;
+ }
+
+ clone->sk = sk;
+
+ return clone;
+}
+EXPORT_SYMBOL(__skb_clone_tx_timestamp);
+
static void __skb_complete_tx_timestamp(struct sk_buff *skb,
struct sock *sk,
int tstype)
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index f48a59f..458f5cb 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -36,10 +36,9 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
{
struct phy_device *phydev;
struct sk_buff *clone;
- struct sock *sk = skb->sk;
unsigned int type;
- if (!sk)
+ if (!skb->sk)
return;
type = classify(skb);
@@ -48,16 +47,9 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
phydev = skb->dev->phydev;
if (likely(phydev->drv->txtstamp)) {
- if (!atomic_inc_not_zero(&sk->sk_refcnt))
+ clone = __skb_clone_tx_timestamp(skb);
+ if (!clone)
return;
-
- clone = skb_clone(skb, GFP_ATOMIC);
- if (!clone) {
- sock_put(sk);
- return;
- }
-
- clone->sk = sk;
phydev->drv->txtstamp(phydev, clone, type);
}
}
^ permalink raw reply related
* [PATCH 1/2] net-timestamp: Merge shared code between phy and regular timestamping
From: Alexander Duyck @ 2014-09-03 15:53 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
In-Reply-To: <20140903155022.1896.59030.stgit@ahduyck-bv4.jf.intel.com>
This change merges the shared bits that exist between skb_tx_tstamp and
skb_complete_tx_timestamp. By doing this we can avoid the two diverging as
there were already changes pushed into skb_tx_tstamp that hadn't made it
into the other function.
In addition this resolves issues with the fact that
skb_complete_tx_timestamp was included in linux/skbuff.h even though it was
only compiled in if phy timestamping was enabled.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
net/core/skbuff.c | 70 ++++++++++++++++++++++++++++++++---------------
net/core/timestamping.c | 29 -------------------
2 files changed, 47 insertions(+), 52 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 53ce536..488d902 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3511,33 +3511,13 @@ struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
}
EXPORT_SYMBOL(sock_dequeue_err_skb);
-void __skb_tstamp_tx(struct sk_buff *orig_skb,
- struct skb_shared_hwtstamps *hwtstamps,
- struct sock *sk, int tstype)
+static void __skb_complete_tx_timestamp(struct sk_buff *skb,
+ struct sock *sk,
+ int tstype)
{
struct sock_exterr_skb *serr;
- struct sk_buff *skb;
int err;
- if (!sk)
- return;
-
- if (hwtstamps) {
- *skb_hwtstamps(orig_skb) =
- *hwtstamps;
- } else {
- /*
- * no hardware time stamps available,
- * so keep the shared tx_flags and only
- * store software time stamp
- */
- orig_skb->tstamp = ktime_get_real();
- }
-
- skb = skb_clone(orig_skb, GFP_ATOMIC);
- if (!skb)
- return;
-
serr = SKB_EXT_ERR(skb);
memset(serr, 0, sizeof(*serr));
serr->ee.ee_errno = ENOMSG;
@@ -3554,6 +3534,50 @@ void __skb_tstamp_tx(struct sk_buff *orig_skb,
if (err)
kfree_skb(skb);
}
+
+void skb_complete_tx_timestamp(struct sk_buff *skb,
+ struct skb_shared_hwtstamps *hwtstamps)
+{
+ struct sock *sk = skb->sk;
+
+ skb->sk = NULL;
+
+ if (hwtstamps) {
+ *skb_hwtstamps(skb) = *hwtstamps;
+ __skb_complete_tx_timestamp(skb, sk, SCM_TSTAMP_SND);
+ } else {
+ kfree_skb(skb);
+ }
+
+ sock_put(sk);
+}
+EXPORT_SYMBOL_GPL(skb_complete_tx_timestamp);
+
+void __skb_tstamp_tx(struct sk_buff *orig_skb,
+ struct skb_shared_hwtstamps *hwtstamps,
+ struct sock *sk, int tstype)
+{
+ struct sk_buff *skb;
+
+ if (!sk)
+ return;
+
+ /*
+ * no hardware time stamps available,
+ * so keep the shared tx_flags and only
+ * store software time stamp
+ */
+ if (!hwtstamps)
+ orig_skb->tstamp = ktime_get_real();
+ else
+ *skb_hwtstamps(orig_skb) = *hwtstamps;
+
+ skb = skb_clone(orig_skb, GFP_ATOMIC);
+ if (!skb)
+ return;
+
+ __skb_complete_tx_timestamp(skb, sk, tstype);
+}
EXPORT_SYMBOL_GPL(__skb_tstamp_tx);
void skb_tstamp_tx(struct sk_buff *orig_skb,
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index a877039..f48a59f 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -63,35 +63,6 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(skb_clone_tx_timestamp);
-void skb_complete_tx_timestamp(struct sk_buff *skb,
- struct skb_shared_hwtstamps *hwtstamps)
-{
- struct sock *sk = skb->sk;
- struct sock_exterr_skb *serr;
- int err;
-
- if (!hwtstamps) {
- sock_put(sk);
- kfree_skb(skb);
- return;
- }
-
- *skb_hwtstamps(skb) = *hwtstamps;
-
- serr = SKB_EXT_ERR(skb);
- memset(serr, 0, sizeof(*serr));
- serr->ee.ee_errno = ENOMSG;
- serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
- skb->sk = NULL;
-
- err = sock_queue_err_skb(sk, skb);
-
- sock_put(sk);
- if (err)
- kfree_skb(skb);
-}
-EXPORT_SYMBOL_GPL(skb_complete_tx_timestamp);
-
bool skb_defer_rx_timestamp(struct sk_buff *skb)
{
struct phy_device *phydev;
^ permalink raw reply related
* [PATCH net] net-timestamp: only report sw timestamp if reporting bit is set
From: Willem de Bruijn @ 2014-09-03 16:01 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
The timestamping API has separate bits for generating and reporting
timestamps. A software timestamp should only be reported for a packet
when the packet has the relevant generation flag (SKBTX_..) set
and the socket has reporting bit SOF_TIMESTAMPING_SOFTWARE set.
The second check was accidentally removed. Reinstitute the original
behavior.
Tested:
Without this patch, Documentation/networking/txtimestamp reports
timestamps regardless of whether SOF_TIMESTAMPING_SOFTWARE is set.
After the patch, it only reports them when the flag is set.
Fixes: f24b9be5957b ("net-timestamp: extend SCM_TIMESTAMPING ancillary data struct")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
include/net/sock.h | 4 +---
net/socket.c | 3 +--
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 7f2ab72..b9a5bd0 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2165,9 +2165,7 @@ sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
*/
if (sock_flag(sk, SOCK_RCVTSTAMP) ||
(sk->sk_tsflags & SOF_TIMESTAMPING_RX_SOFTWARE) ||
- (kt.tv64 &&
- (sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE ||
- skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP)) ||
+ (kt.tv64 && sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE) ||
(hwtstamps->hwtstamp.tv64 &&
(sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE)))
__sock_recv_timestamp(msg, sk, skb);
diff --git a/net/socket.c b/net/socket.c
index 95ee7d8..4eb09b3 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -734,8 +734,7 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
}
memset(&tss, 0, sizeof(tss));
- if ((sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE ||
- skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP) &&
+ if ((sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE) &&
ktime_to_timespec_cond(skb->tstamp, tss.ts + 0))
empty = 0;
if (shhwtstamps &&
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH 0/2] Combine standard and phy timestamping logic
From: Alexander Duyck @ 2014-09-03 15:53 UTC (permalink / raw)
To: netdev; +Cc: richardcochran, davem, willemb
This change makes it so that the core path for the phy timestamping logic
is shared between skb_tx_tstamp and skb_complete_tx_timestamp. In addition
it provides a means of using the same skb clone type path in non phy
timestamping drivers.
The main motivation for this is to enable non-phy drivers to be able to
manipulate tx timestamp skbs for such things as putting them in lists or
setting aside buffer in the context block.
---
Alexander Duyck (2):
net-timestamp: Merge shared code between phy and regular timestamping
net-timestamp: Make the clone operation stand-alone from phy timestamping
include/linux/skbuff.h | 2 +
net/core/skbuff.c | 86 ++++++++++++++++++++++++++++++++++++-----------
net/core/timestamping.c | 43 ++----------------------
3 files changed, 70 insertions(+), 61 deletions(-)
--
^ permalink raw reply
* [PATCH net-next iproute2 v2] iproute2: allow to change slave options via type_slave
From: Nikolay Aleksandrov @ 2014-09-03 15:57 UTC (permalink / raw)
To: netdev; +Cc: jiri, stephen, davem, Nikolay Aleksandrov
In-Reply-To: <20140903150215.GA1843@nanopsycho.lan>
This patch adds the necessary changes to allow altering a slave device's
options via ip link set <device> type <master type>_slave specific-option.
It also adds support to set the bonding slaves' queue_id.
Example:
ip link set eth0 type bond_slave queue_id 10
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
v2: change the usage from <master type>_slave to bond_slave as Jiri suggested
ip/iplink.c | 22 +++++++++++++++++++---
ip/iplink_bond_slave.c | 19 +++++++++++++++++++
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/ip/iplink.c b/ip/iplink.c
index 1a907d998a87..0992923c83bc 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -88,7 +88,8 @@ void iplink_usage(void)
fprintf(stderr, "\n");
fprintf(stderr, "TYPE := { vlan | veth | vcan | dummy | ifb | macvlan | macvtap |\n");
fprintf(stderr, " bridge | bond | ipoib | ip6tnl | ipip | sit | vxlan |\n");
- fprintf(stderr, " gre | gretap | ip6gre | ip6gretap | vti | nlmon }\n");
+ fprintf(stderr, " gre | gretap | ip6gre | ip6gretap | vti | nlmon |\n");
+ fprintf(stderr, " bond_slave }\n");
}
exit(-1);
}
@@ -697,14 +698,29 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
if (type) {
struct rtattr *linkinfo = NLMSG_TAIL(&req.n);
+ char slavebuf[128], *ulinep = strchr(type, '_');
+ int iflatype;
+
addattr_l(&req.n, sizeof(req), IFLA_LINKINFO, NULL, 0);
addattr_l(&req.n, sizeof(req), IFLA_INFO_KIND, type,
strlen(type));
- lu = get_link_kind(type);
+ if (ulinep && !strcmp(ulinep, "_slave")) {
+ strncpy(slavebuf, type, sizeof(slavebuf));
+ slavebuf[sizeof(slavebuf) - 1] = '\0';
+ ulinep = strchr(slavebuf, '_');
+ /* check in case it was after sizeof(slavebuf) - 1*/
+ if (ulinep)
+ *ulinep = '\0';
+ lu = get_link_slave_kind(slavebuf);
+ iflatype = IFLA_INFO_SLAVE_DATA;
+ } else {
+ lu = get_link_kind(type);
+ iflatype = IFLA_INFO_DATA;
+ }
if (lu && argc) {
struct rtattr * data = NLMSG_TAIL(&req.n);
- addattr_l(&req.n, sizeof(req), IFLA_INFO_DATA, NULL, 0);
+ addattr_l(&req.n, sizeof(req), iflatype, NULL, 0);
if (lu->parse_opt &&
lu->parse_opt(lu, argc, argv, &req.n))
diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c
index 8f3fc6cec6fa..aacba14aef9c 100644
--- a/ip/iplink_bond_slave.c
+++ b/ip/iplink_bond_slave.c
@@ -80,10 +80,29 @@ static void bond_slave_print_opt(struct link_util *lu, FILE *f, struct rtattr *t
rta_getattr_u16(tb[IFLA_BOND_SLAVE_AD_AGGREGATOR_ID]));
}
+static int bond_slave_parse_opt(struct link_util *lu, int argc, char **argv,
+ struct nlmsghdr *n)
+{
+ __u16 queue_id;
+
+ while (argc > 0) {
+ if (matches(*argv, "queue_id") == 0) {
+ NEXT_ARG();
+ if (get_u16(&queue_id, *argv, 0))
+ invarg("queue_id is invalid", *argv);
+ addattr16(n, 1024, IFLA_BOND_SLAVE_QUEUE_ID, queue_id);
+ }
+ argc--, argv++;
+ }
+
+ return 0;
+}
+
struct link_util bond_slave_link_util = {
.id = "bond",
.maxattr = IFLA_BOND_SLAVE_MAX,
.print_opt = bond_slave_print_opt,
+ .parse_opt = bond_slave_parse_opt,
.slave = true,
};
--
1.9.3
^ permalink raw reply related
* [net-next PATCH V3] qdisc: validate frames going through the direct_xmit path
From: Jesper Dangaard Brouer @ 2014-09-03 15:56 UTC (permalink / raw)
To: Jesper Dangaard Brouer, David S. Miller, Alexander Duyck, netdev
Cc: Eric Dumazet
In-Reply-To: <20140903114841.19969.22671.stgit@dragon>
In commit 50cbe9ab5f8d ("net: Validate xmit SKBs right when we
pull them out of the qdisc") the validation code was moved out of
dev_hard_start_xmit and into dequeue_skb.
However this overlooked the fact that we do not always enqueue
the skb onto a qdisc. First situation is if qdisc have flag
TCQ_F_CAN_BYPASS and qdisc is empty. Second situation is if
there is no qdisc on the device, which is a common case for
software devices.
Originally spotted and inital patch by Alexander Duyck.
As a result Alex was seeing issues trying to connect to a
vhost_net interface after commit 50cbe9ab5f8d was applied.
Added a call to validate_xmit_skb() in __dev_xmit_skb(), in the
code path for qdiscs with TCQ_F_CAN_BYPASS flag, and in
__dev_queue_xmit() when no qdisc.
Also handle the error situation where dev_hard_start_xmit() could
return a skb list, and does not return dev_xmit_complete(rc) and
falls through to the kfree_skb(), in that situation it should
call kfree_skb_list().
Fixes: 50cbe9ab5f8d ("net: Validate xmit SKBs right when we pull them out of the qdisc")
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
V2:
- Also handle no qdisc on the device situation
V3:
- In __dev_queue_xmit() move validate_xmit_skb() up before
HARD_TX_LOCK and record tx_dropped if the validation failed.
- Use kfree_skb_list() instead of kfree_skb() in fallthrough err case.
net/core/dev.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 3774afc..2f3dbd6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2739,7 +2739,8 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
qdisc_bstats_update(q, skb);
- if (sch_direct_xmit(skb, q, dev, txq, root_lock)) {
+ skb = validate_xmit_skb(skb, dev);
+ if (skb && sch_direct_xmit(skb, q, dev, txq, root_lock)) {
if (unlikely(contended)) {
spin_unlock(&q->busylock);
contended = false;
@@ -2879,6 +2880,10 @@ static int __dev_queue_xmit(struct sk_buff *skb, void *accel_priv)
if (__this_cpu_read(xmit_recursion) > RECURSION_LIMIT)
goto recursion_alert;
+ skb = validate_xmit_skb(skb, dev);
+ if (!skb)
+ goto drop;
+
HARD_TX_LOCK(dev, txq, cpu);
if (!netif_xmit_stopped(txq)) {
@@ -2904,10 +2909,11 @@ recursion_alert:
}
rc = -ENETDOWN;
+drop:
rcu_read_unlock_bh();
atomic_long_inc(&dev->tx_dropped);
- kfree_skb(skb);
+ kfree_skb_list(skb);
return rc;
out:
rcu_read_unlock_bh();
^ permalink raw reply related
* Re: [PATCH net] l2tp: fix race while getting PMTU on PPP pseudo-wire
From: Guillaume Nault @ 2014-09-03 15:47 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Dmitry Petukhov, netdev, James Chapman, David S. Miller
In-Reply-To: <1409752526.26422.36.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Sep 03, 2014 at 06:55:26AM -0700, Eric Dumazet wrote:
> On Wed, 2014-09-03 at 14:12 +0200, Guillaume Nault wrote:
> > Use dst_entry held by sk_dst_get() to retrieve tunnel's PMTU.
> >
> > The dst_mtu(__sk_dst_get(tunnel->sock)) call was racy. __sk_dst_get()
> > could return NULL if tunnel->sock->sk_dst_cache was reset just before the
> > call, thus making dst_mtu() dereference a NULL pointer:
> >
> > [ 1937.661598] BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
> > [ 1937.664005] IP: [<ffffffffa049db88>] pppol2tp_connect+0x33d/0x41e [l2tp_ppp]
> > [ 1937.664005] PGD daf0c067 PUD d9f93067 PMD 0
> > [ 1937.664005] Oops: 0000 [#1] SMP
> > [ 1937.664005] Modules linked in: l2tp_ppp l2tp_netlink l2tp_core ip6table_filter ip6_tables iptable_filter ip_tables ebtable_nat ebtables x_tables udp_tunnel pppoe pppox ppp_generic slhc deflate ctr twofish_generic twofish_x86_64_3way xts lrw gf128mul glue_helper twofish_x86_64 twofish_common blowfish_generic blowfish_x86_64 blowfish_common des_generic cbc xcbc rmd160 sha512_generic hmac crypto_null af_key xfrm_algo 8021q garp bridge stp llc tun atmtcp clip atm ext3 mbcache jbd iTCO_wdt coretemp kvm_intel iTCO_vendor_support kvm pcspkr evdev ehci_pci lpc_ich mfd_core i5400_edac edac_core i5k_amb shpchp button processor thermal_sys xfs crc32c_generic libcrc32c dm_mod usbhid sg hid sr_mod sd_mod cdrom crc_t10dif crct10dif_common ata_generic ahci ata_piix tg3 libahci libata uhci_hcd ptp
ehci_hcd pps_core usbcore scsi_mod libphy usb_common [last unloaded: l2tp_core]
> > [ 1937.664005] CPU: 0 PID: 10022 Comm: l2tpstress Tainted: G O 3.17.0-rc1 #1
> > [ 1937.664005] Hardware name: HP ProLiant DL160 G5, BIOS O12 08/22/2008
> > [ 1937.664005] task: ffff8800d8fda790 ti: ffff8800c43c4000 task.ti: ffff8800c43c4000
> > [ 1937.664005] RIP: 0010:[<ffffffffa049db88>] [<ffffffffa049db88>] pppol2tp_connect+0x33d/0x41e [l2tp_ppp]
> > [ 1937.664005] RSP: 0018:ffff8800c43c7de8 EFLAGS: 00010282
> > [ 1937.664005] RAX: ffff8800da8a7240 RBX: ffff8800d8c64600 RCX: 000001c325a137b5
> > [ 1937.664005] RDX: 8c6318c6318c6320 RSI: 000000000000010c RDI: 0000000000000000
> > [ 1937.664005] RBP: ffff8800c43c7ea8 R08: 0000000000000000 R09: 0000000000000000
> > [ 1937.664005] R10: ffffffffa048e2c0 R11: ffff8800d8c64600 R12: ffff8800ca7a5000
> > [ 1937.664005] R13: ffff8800c439bf40 R14: 000000000000000c R15: 0000000000000009
> > [ 1937.664005] FS: 00007fd7f610f700(0000) GS:ffff88011a600000(0000) knlGS:0000000000000000
> > [ 1937.664005] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > [ 1937.664005] CR2: 0000000000000020 CR3: 00000000d9d75000 CR4: 00000000000027e0
> > [ 1937.664005] Stack:
> > [ 1937.664005] ffffffffa049da80 ffff8800d8fda790 000000000000005b ffff880000000009
> > [ 1937.664005] ffff8800daf3f200 0000000000000003 ffff8800c43c7e48 ffffffff81109b57
> > [ 1937.664005] ffffffff81109b0e ffffffff8114c566 0000000000000000 0000000000000000
> > [ 1937.664005] Call Trace:
> > [ 1937.664005] [<ffffffffa049da80>] ? pppol2tp_connect+0x235/0x41e [l2tp_ppp]
> > [ 1937.664005] [<ffffffff81109b57>] ? might_fault+0x9e/0xa5
> > [ 1937.664005] [<ffffffff81109b0e>] ? might_fault+0x55/0xa5
> > [ 1937.664005] [<ffffffff8114c566>] ? rcu_read_unlock+0x1c/0x26
> > [ 1937.664005] [<ffffffff81309196>] SYSC_connect+0x87/0xb1
> > [ 1937.664005] [<ffffffff813e56f7>] ? sysret_check+0x1b/0x56
> > [ 1937.664005] [<ffffffff8107590d>] ? trace_hardirqs_on_caller+0x145/0x1a1
> > [ 1937.664005] [<ffffffff81213dee>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> > [ 1937.664005] [<ffffffff8114c262>] ? spin_lock+0x9/0xb
> > [ 1937.664005] [<ffffffff813092b4>] SyS_connect+0x9/0xb
> > [ 1937.664005] [<ffffffff813e56d2>] system_call_fastpath+0x16/0x1b
> > [ 1937.664005] Code: 10 2a 84 81 e8 65 76 bd e0 65 ff 0c 25 10 bb 00 00 4d 85 ed 74 37 48 8b 85 60 ff ff ff 48 8b 80 88 01 00 00 48 8b b8 10 02 00 00 <48> 8b 47 20 ff 50 20 85 c0 74 0f 83 e8 28 89 83 10 01 00 00 89
> > [ 1937.664005] RIP [<ffffffffa049db88>] pppol2tp_connect+0x33d/0x41e [l2tp_ppp]
> > [ 1937.664005] RSP <ffff8800c43c7de8>
> > [ 1937.664005] CR2: 0000000000000020
> > [ 1939.559375] ---[ end trace 82d44500f28f8708 ]---
> >
> > Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
> > ---
> > net/l2tp/l2tp_ppp.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
> > index 13752d9..b704a93 100644
> > --- a/net/l2tp/l2tp_ppp.c
> > +++ b/net/l2tp/l2tp_ppp.c
> > @@ -755,7 +755,8 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> > /* If PMTU discovery was enabled, use the MTU that was discovered */
> > dst = sk_dst_get(tunnel->sock);
> > if (dst != NULL) {
> > - u32 pmtu = dst_mtu(__sk_dst_get(tunnel->sock));
> > + u32 pmtu = dst_mtu(dst);
> > +
> > if (pmtu != 0)
> > session->mtu = session->mru = pmtu -
> > PPPOL2TP_HEADER_OVERHEAD;
>
>
> Probably introduced in commit f34c4a35d879 ("l2tp: take PMTU from tunnel
> UDP socket")
>
Indeed, I forgot to mention it. Thanks for clarification.
> CC Dmitry
>
> Before this patch, the bug could not happen.
>
> Acked-by: Eric Dumazet <edumazet@google.com>
> Fixes: f34c4a35d879 ("l2tp: take PMTU from tunnel UDP socket")
>
>
^ permalink raw reply
* Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: Daniel Borkmann @ 2014-09-03 15:46 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409714246-31054-3-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On 09/03/2014 05:17 AM, Alexei Starovoitov wrote:
> allow user space to generate eBPF programs
>
> uapi/linux/bpf.h: eBPF instruction set definition
>
> linux/filter.h: the rest
>
> This patch only moves macro definitions, but practically it freezes existing
> eBPF instruction set, though new instructions can still be added in the future.
>
> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
> may conflict with existing applications.
>
> Full eBPF ISA description is in Documentation/networking/filter.txt
>
> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Ok, given you post the remaining two RFCs later on this window as
you indicate, I have no objections:
Acked-by: Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply
* Re: [patch net-next 03/13] net: introduce generic switch devices support
From: John Fastabend @ 2014-09-03 15:46 UTC (permalink / raw)
To: Jiri Pirko
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
jasowang-H+wXaHxf7aLQT0dZR+AlfA,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
buytenh-OLH4Qvv75CYX/NnBR394Jw,
roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
netdev-u79uwXL29TY76Z2rM5mHXA,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1409736300-12303-4-git-send-email-jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
On 09/03/2014 02:24 AM, Jiri Pirko wrote:
> The goal of this is to provide a possibility to suport various switch
> chips. Drivers should implement relevant ndos to do so. Now there is a
> couple of ndos defines:
> - for getting physical switch id is in place.
> - for work with flows.
>
> Note that user can use random port netdevice to access the switch.
>
> Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
> ---
[...]
> struct netpoll_info;
> @@ -997,6 +999,24 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
> * Callback to use for xmit over the accelerated station. This
> * is used in place of ndo_start_xmit on accelerated net
> * devices.
> + *
> + * int (*ndo_swdev_get_id)(struct net_device *dev,
> + * struct netdev_phys_item_id *psid);
> + * Called to get an ID of the switch chip this port is part of.
> + * If driver implements this, it indicates that it represents a port
> + * of a switch chip.
> + *
> + * int (*ndo_swdev_flow_insert)(struct net_device *dev,
> + * const struct sw_flow *flow);
> + * Called to insert a flow into switch device. If driver does
> + * not implement this, it is assumed that the hw does not have
> + * a capability to work with flows.
> + *
> + * int (*ndo_swdev_flow_remove)(struct net_device *dev,
> + * const struct sw_flow *flow);
> + * Called to remove a flow from switch device. If driver does
> + * not implement this, it is assumed that the hw does not have
> + * a capability to work with flows.
> */
> struct net_device_ops {
> int (*ndo_init)(struct net_device *dev);
> @@ -1146,6 +1166,14 @@ struct net_device_ops {
> struct net_device *dev,
> void *priv);
> int (*ndo_get_lock_subclass)(struct net_device *dev);
> +#ifdef CONFIG_NET_SWITCHDEV
> + int (*ndo_swdev_get_id)(struct net_device *dev,
> + struct netdev_phys_item_id *psid);
> + int (*ndo_swdev_flow_insert)(struct net_device *dev,
> + const struct sw_flow *flow);
> + int (*ndo_swdev_flow_remove)(struct net_device *dev,
> + const struct sw_flow *flow);
Not really a critique of your patch but I'll need to extend this
with a ndo_swdev_flow_dump() to get the fields. Without this if
your user space side ever restarts, gets out of sync there is no
way to get back in sync.
Also with hardware that has multiple flow tables we need to indicate
the table to insert the flow into. One concrete reason to do this
is to create atomic updates of multiple ACLs. The idea is to create
a new ACL table build the table up and then link it in. This can be
added when its needed my opensource drivers don't support this yet
either but maybe adding multiple tables to rocker switch will help
flush this out.
Finally we need some way to drive capabilities out of the swdev.
Even rocker switch needs this to indicate it doesn't support matching
on all the sw_flow fields. Without this its not clear to me how to
manage the device from user space. I tried writing user space daemon
for the simpler flow director interface and the try and see model
breaks quickly.
> +#endif
> };
>
> /**
> diff --git a/include/net/sw_flow.h b/include/net/sw_flow.h
> index 21724f1..3af7758 100644
> --- a/include/net/sw_flow.h
> +++ b/include/net/sw_flow.h
> @@ -81,7 +81,21 @@ struct sw_flow_mask {
> struct sw_flow_key key;
> };
>
> +enum sw_flow_action_type {
> + SW_FLOW_ACTION_TYPE_OUTPUT,
> + SW_FLOW_ACTION_TYPE_VLAN_PUSH,
> + SW_FLOW_ACTION_TYPE_VLAN_POP,
> +};
> +
OK my previous comment about having another patch to create
generic actions seems to be resolved here. I'm not sure how
important it is but if we abstract the flow types away from
OVS is there any reason not to reuse and relabel the action
types as well? I guess we can't break userspace API but maybe
a 1:1 mapping would be better?
> struct sw_flow_action {
> + enum sw_flow_action_type type;
> + union {
> + u32 out_port_ifindex;
> + struct {
> + __be16 vlan_proto;
> + u16 vlan_tci;
> + } vlan;
> + };
> };
[...]
I think my comments could be addressed with additional patches
if you want. I could help but it will be another week or so
before I have some time. The biggest issue IMO is the lack of
capabilities queries.
Thanks,
John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH 4/4] ARM: dts: Enable emac node on the rk3188-radxarock boards
From: Naoki FUKAUMI @ 2014-09-03 15:29 UTC (permalink / raw)
To: Romain Perier
Cc: heiko, linux-rockchip, devicetree, arnd, linux-arm-kernel, netdev
In-Reply-To: <1409740036-8117-4-git-send-email-romain.perier@gmail.com>
On Wed, Sep 3, 2014 at 7:27 PM, Romain Perier <romain.perier@gmail.com> wrote:
> This enables EMAC Rockchip support on radxa rock boards.
>
> Signed-off-by: Romain Perier <romain.perier@gmail.com>
> ---
> arch/arm/boot/dts/rk3188-radxarock.dts | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3188-radxarock.dts b/arch/arm/boot/dts/rk3188-radxarock.dts
> index 5e4e3c23..e1358d3 100644
> --- a/arch/arm/boot/dts/rk3188-radxarock.dts
> +++ b/arch/arm/boot/dts/rk3188-radxarock.dts
> @@ -76,6 +76,24 @@
> };
> };
>
> +&emac {
> + status = "okay";
> + compatible = "rockchip,rk3188-emac";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&emac_xfer>, <&emac_mdio>, <&phy_int>;
> +
> + mac-address = [ c6 ef 91 8e 60 4b ];
is "mac-address" required?
^ permalink raw reply
* [PATCH] drivers/net/fddi/skfp/h/skfbi.h: Remove useless PCI_BASE_2ND macros
From: Chen Gang @ 2014-09-03 15:26 UTC (permalink / raw)
To: David S. Miller
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
michal.simek, Michal Simek
They are use less, and may generate compiling warnings, so remove them
(microblaze, arc, arm64, and unicore32 have already defined PCI_IOBASE).
The related warnings (with allmodconfig under microblaze):
CC [M] drivers/net/fddi/skfp/skfddi.o
In file included from drivers/net/fddi/skfp/skfddi.c:95:0:
drivers/net/fddi/skfp/h/skfbi.h:151:0: warning: "PCI_IOBASE" redefined
#define PCI_IOBASE 0xffffff00L /* Bit 31..8: I/O Base address */
^
In file included from include/linux/io.h:22:0,
from include/linux/pci.h:31,
from drivers/net/fddi/skfp/skfddi.c:82:
./arch/microblaze/include/asm/io.h:33:0: note: this is the location of the previous definition
#define PCI_IOBASE ((void __iomem *)_IO_BASE)
^
Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
drivers/net/fddi/skfp/h/skfbi.h | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/fddi/skfp/h/skfbi.h b/drivers/net/fddi/skfp/h/skfbi.h
index c1ba26c..3de2f0d 100644
--- a/drivers/net/fddi/skfp/h/skfbi.h
+++ b/drivers/net/fddi/skfp/h/skfbi.h
@@ -147,11 +147,6 @@
#define PCI_MEM64BIT (2<<1) /* Base addr anywhere in 64 Bit range */
#define PCI_MEMSPACE 0x00000001L /* Bit 0: Memory Space Indic. */
-/* PCI_BASE_2ND 32 bit 2nd Base address */
-#define PCI_IOBASE 0xffffff00L /* Bit 31..8: I/O Base address */
-#define PCI_IOSIZE 0x000000fcL /* Bit 7..2: I/O Size Requirements */
-#define PCI_IOSPACE 0x00000001L /* Bit 0: I/O Space Indicator */
-
/* PCI_SUB_VID 16 bit Subsystem Vendor ID */
/* PCI_SUB_ID 16 bit Subsystem ID */
--
1.9.3
^ permalink raw reply related
* [net-next PATCH V2] qdisc: validate frames going through the direct_xmit path
From: Jesper Dangaard Brouer @ 2014-09-03 15:24 UTC (permalink / raw)
To: Jesper Dangaard Brouer, David S. Miller, Alexander Duyck, netdev
Cc: Eric Dumazet
In-Reply-To: <20140903114841.19969.22671.stgit@dragon>
In commit 50cbe9ab5f8d ("net: Validate xmit SKBs right when we
pull them out of the qdisc") the validation code was moved out of
dev_hard_start_xmit and into dequeue_skb.
However this overlooked the fact that we do not always enqueue
the skb onto a qdisc. First situation is if qdisc have flag
TCQ_F_CAN_BYPASS and qdisc is empty. Second situation is if
there is no qdisc on the device, which is a common case for
software devices.
Originally spotted and inital patch by Alexander Duyck.
As a result Alex was seeing issues trying to connect to a
vhost_net interface after commit 50cbe9ab5f8d was applied.
Added a call to validate_xmit_skb() in __dev_xmit_skb(), in the
code path for qdiscs with TCQ_F_CAN_BYPASS flag, and in
__dev_queue_xmit() when no qdisc.
Fixes: 50cbe9ab5f8d ("net: Validate xmit SKBs right when we pull them out of the qdisc")
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
V2:
- Also handle no qdisc on the device situation
Posting a V2 quickly, if DaveM wants this fixed quickly.
net/core/dev.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 3774afc..8d5af9c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2739,7 +2739,8 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
qdisc_bstats_update(q, skb);
- if (sch_direct_xmit(skb, q, dev, txq, root_lock)) {
+ skb = validate_xmit_skb(skb, dev);
+ if (skb && sch_direct_xmit(skb, q, dev, txq, root_lock)) {
if (unlikely(contended)) {
spin_unlock(&q->busylock);
contended = false;
@@ -2882,6 +2883,7 @@ static int __dev_queue_xmit(struct sk_buff *skb, void *accel_priv)
HARD_TX_LOCK(dev, txq, cpu);
if (!netif_xmit_stopped(txq)) {
+ skb = validate_xmit_skb(skb, dev);
__this_cpu_inc(xmit_recursion);
skb = dev_hard_start_xmit(skb, dev, txq, &rc);
__this_cpu_dec(xmit_recursion);
^ permalink raw reply related
* Re: [net-next PATCH] qdisc: validate frames going through the direct_xmit path
From: Eric Dumazet @ 2014-09-03 15:22 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: David S. Miller, Alexander Duyck, netdev
In-Reply-To: <20140903162628.3d05f113@redhat.com>
On Wed, 2014-09-03 at 16:26 +0200, Jesper Dangaard Brouer wrote:
> On Wed, 3 Sep 2014 15:52:13 +0200
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
> > On Wed, 03 Sep 2014 06:43:25 -0700
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > >
> > > Jesper, you missed another spot, when there is no qdisc on the device.
> > >
> > > __dev_queue_xmit() calls dev_hard_start_xmit() around line 2886
> > >
> > > Could we try to not add a myriad of small patches ?
> > >
> > > Some of us will need to backport all of them.
> >
> > I'm in the same backport situation ;-)
> >
> > I'll send a V2 of this patch, with missed spot...
>
> (to avoid doing too many patches)
>
> In the code below (from __dev_queue_xmit()), I've added validate_xmit_skb().
> (dev_hard_start_xmit() can handle if it is a NULL ptr)
>
> HARD_TX_LOCK(dev, txq, cpu);
> if (!netif_xmit_stopped(txq)) {
> skb = validate_xmit_skb(skb, dev);
> __this_cpu_inc(xmit_recursion);
> skb = dev_hard_start_xmit(skb, dev, txq, &rc);
> __this_cpu_dec(xmit_recursion);
> if (dev_xmit_complete(rc)) {
> HARD_TX_UNLOCK(dev, txq);
> goto out;
> }
> }
> HARD_TX_UNLOCK(dev, txq);
> net_crit_ratelimited("Virtual device %s asks to queue packet!\n",
> dev->name);
> BUT shouldn't we also handle if dev_hard_start_xmit() returns an skb pointer,
> which means that there were skb's left on the skb list. Due to the rc
> value "returned" in this case, we should see the net_crit_ratelimited()
> msg, but we don't handle freeing these SKBs.
>
> Any recommentations?
>
First I would do the validate_xmit_skb() before HARD_TX_LOCK(dev, txq,
cpu)
Then, if we have skbs that could not be transmitted, we need to free
them (and possibly account the drops on the device tx_dropped). Logging
a one time message would be enough.
Thanks
^ permalink raw reply
* Re: [patch net-next 01/13] openvswitch: split flow structures into ovs specific and generic ones
From: John Fastabend @ 2014-09-03 15:20 UTC (permalink / raw)
To: Jiri Pirko
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
jasowang-H+wXaHxf7aLQT0dZR+AlfA,
john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
buytenh-OLH4Qvv75CYX/NnBR394Jw,
roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
netdev-u79uwXL29TY76Z2rM5mHXA,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1409736300-12303-2-git-send-email-jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
On 09/03/2014 02:24 AM, Jiri Pirko wrote:
> After this, flow related structures can be used in other code.
>
> Signed-off-by: Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
> ---
Hi Jiri,
As I indicated before I'm looking into integrating this with some
hardware here. Progress is a bit slow but starting to look at it.The
i40e/ixgbe driver being one open source example with very limited
support for tables, flow matches, etc. And then a closed source driver
with much more flexibility. What I don't have is a middle of the road
switch to work with something better then a host nic but not as
flexible as a TOR.
Couple questions my assumption here is I can extend the flow_key
as needed to support additional match criteria my hardware has.
I scanned the ./net/openvswitch source and I didn't catch any
place that would break but might need to take a closer look.
Similarly the actions set will need to be extended. For example
if I want to use this with i40e a OVS_ACTION_ATTR_QUEUE could
be used to steer packets to the queue. With this in mind we
will want a follow up patch to rename OVS_ACTION_ATTR_* to
FLOW_ACTION_ATTR_*
Also I have some filters that can match on offset/length/mask
tuples. As far as I can tell this is going to have to be yet
another interface? Or would it be worth the effort to define
the flow key more generically. My initial guess is I'll just
write a separate interface. I think this is what Jamal referred
to as another "classifier".
Thanks,
John
[...]
> +
> +struct sw_flow_key_ipv4_tunnel {
> + __be64 tun_id;
> + __be32 ipv4_src;
> + __be32 ipv4_dst;
> + __be16 tun_flags;
> + u8 ipv4_tos;
> + u8 ipv4_ttl;
> +};
> +
> +struct sw_flow_key {
> + struct sw_flow_key_ipv4_tunnel tun_key; /* Encapsulating tunnel key. */
> + struct {
> + u32 priority; /* Packet QoS priority. */
> + u32 skb_mark; /* SKB mark. */
> + u16 in_port; /* Input switch port (or DP_MAX_PORTS). */
> + } __packed phy; /* Safe when right after 'tun_key'. */
> + struct {
> + u8 src[ETH_ALEN]; /* Ethernet source address. */
> + u8 dst[ETH_ALEN]; /* Ethernet destination address. */
> + __be16 tci; /* 0 if no VLAN, VLAN_TAG_PRESENT set otherwise. */
> + __be16 type; /* Ethernet frame type. */
> + } eth;
> + struct {
> + u8 proto; /* IP protocol or lower 8 bits of ARP opcode. */
> + u8 tos; /* IP ToS. */
> + u8 ttl; /* IP TTL/hop limit. */
> + u8 frag; /* One of OVS_FRAG_TYPE_*. */
> + } ip;
> + struct {
> + __be16 src; /* TCP/UDP/SCTP source port. */
> + __be16 dst; /* TCP/UDP/SCTP destination port. */
> + __be16 flags; /* TCP flags. */
> + } tp;
> + union {
> + struct {
> + struct {
> + __be32 src; /* IP source address. */
> + __be32 dst; /* IP destination address. */
> + } addr;
> + struct {
> + u8 sha[ETH_ALEN]; /* ARP source hardware address. */
> + u8 tha[ETH_ALEN]; /* ARP target hardware address. */
> + } arp;
> + } ipv4;
> + struct {
> + struct {
> + struct in6_addr src; /* IPv6 source address. */
> + struct in6_addr dst; /* IPv6 destination address. */
> + } addr;
> + __be32 label; /* IPv6 flow label. */
> + struct {
> + struct in6_addr target; /* ND target address. */
> + u8 sll[ETH_ALEN]; /* ND source link layer address. */
> + u8 tll[ETH_ALEN]; /* ND target link layer address. */
> + } nd;
> + } ipv6;
> + };
> +} __aligned(BITS_PER_LONG/8); /* Ensure that we can do comparisons as longs. */
> +
> +struct sw_flow_key_range {
> + unsigned short int start;
> + unsigned short int end;
> +};
> +
> +struct sw_flow_mask {
> + struct sw_flow_key_range range;
> + struct sw_flow_key key;
> +};
> +
> +struct sw_flow_action {
> +};
> +
> +struct sw_flow_actions {
> + unsigned count;
> + struct sw_flow_action actions[0];
> +};
> +
> +struct sw_flow {
> + struct sw_flow_key key;
> + struct sw_flow_key unmasked_key;
> + struct sw_flow_mask *mask;
> + struct sw_flow_actions *actions;
> +};
> +
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: High perf top ip_idents_reserve doing netperf UDP_STREAM
From: Eric Dumazet @ 2014-09-03 15:17 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev@vger.kernel.org, Daniel Borkmann, Hannes Frederic Sowa,
Florian Westphal
In-Reply-To: <20140903165943.372b897b@redhat.com>
On Wed, 2014-09-03 at 16:59 +0200, Jesper Dangaard Brouer wrote:
> Hi Eric,
>
> When doing:
> super_netperf 120 -H 192.168.8.2 -t UDP_STREAM -l 100 -- -m 256
>
> I'm seeing function ip_idents_reserve() consuming most CPU. Could you
> help, explain what is going on, and how I can avoid this?
>
> Perf top:
> 11.67% [kernel] [k] ip_idents_reserve
> 8.37% [kernel] [k] fib_table_lookup
> 4.46% [kernel] [k] _raw_spin_lock
> 3.21% [kernel] [k] copy_user_enhanced_fast_string
> 2.92% [kernel] [k] sock_alloc_send_pskb
> 2.88% [kernel] [k] udp_sendmsg
>
Because you use a single destination, all flows compete on a single
atomic to get their next IP identifier.
You can try to use netperf options (-- -N -n) so that netperf uses
connected UDP sockets.
In this case, the IP identifier generator is held in each socket.
^ permalink raw reply
* Re: [PATCH net-next iproute2] iproute2: allow to change slave options via type_slave
From: Jiri Pirko @ 2014-09-03 15:02 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, stephen, davem
In-Reply-To: <1409755408-13929-1-git-send-email-nikolay@redhat.com>
Wed, Sep 03, 2014 at 04:43:28PM CEST, nikolay@redhat.com wrote:
>This patch adds the necessary changes to allow altering a slave device's
>options via ip link set <device> type <master type>_slave specific-option.
>It also adds support to set the bonding slaves' queue_id.
>
>Example:
> ip link set eth0 type bond_slave queue_id 10
>
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>---
> ip/iplink.c | 22 +++++++++++++++++++---
> ip/iplink_bond_slave.c | 19 +++++++++++++++++++
> 2 files changed, 38 insertions(+), 3 deletions(-)
>
>diff --git a/ip/iplink.c b/ip/iplink.c
>index 1a907d998a87..5f2db92c981e 100644
>--- a/ip/iplink.c
>+++ b/ip/iplink.c
>@@ -88,7 +88,8 @@ void iplink_usage(void)
> fprintf(stderr, "\n");
> fprintf(stderr, "TYPE := { vlan | veth | vcan | dummy | ifb | macvlan | macvtap |\n");
> fprintf(stderr, " bridge | bond | ipoib | ip6tnl | ipip | sit | vxlan |\n");
>- fprintf(stderr, " gre | gretap | ip6gre | ip6gretap | vti | nlmon }\n");
>+ fprintf(stderr, " gre | gretap | ip6gre | ip6gretap | vti | nlmon |\n");
>+ fprintf(stderr, " <master type>_slave }\n");
I think you can put "bond_slave" here. Once any other slave type
implements parse_opt, it would be added here as well.
> }
> exit(-1);
> }
>@@ -697,14 +698,29 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
>
> if (type) {
> struct rtattr *linkinfo = NLMSG_TAIL(&req.n);
>+ char slavebuf[128], *ulinep = strchr(type, '_');
>+ int iflatype;
>+
> addattr_l(&req.n, sizeof(req), IFLA_LINKINFO, NULL, 0);
> addattr_l(&req.n, sizeof(req), IFLA_INFO_KIND, type,
> strlen(type));
>
>- lu = get_link_kind(type);
>+ if (ulinep && !strcmp(ulinep, "_slave")) {
>+ strncpy(slavebuf, type, sizeof(slavebuf));
>+ slavebuf[sizeof(slavebuf) - 1] = '\0';
>+ ulinep = strchr(slavebuf, '_');
>+ /* check in case it was after sizeof(slavebuf) - 1*/
>+ if (ulinep)
>+ *ulinep = '\0';
>+ lu = get_link_slave_kind(slavebuf);
>+ iflatype = IFLA_INFO_SLAVE_DATA;
>+ } else {
>+ lu = get_link_kind(type);
>+ iflatype = IFLA_INFO_DATA;
>+ }
> if (lu && argc) {
> struct rtattr * data = NLMSG_TAIL(&req.n);
>- addattr_l(&req.n, sizeof(req), IFLA_INFO_DATA, NULL, 0);
>+ addattr_l(&req.n, sizeof(req), iflatype, NULL, 0);
>
> if (lu->parse_opt &&
> lu->parse_opt(lu, argc, argv, &req.n))
>diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c
>index 8f3fc6cec6fa..aacba14aef9c 100644
>--- a/ip/iplink_bond_slave.c
>+++ b/ip/iplink_bond_slave.c
>@@ -80,10 +80,29 @@ static void bond_slave_print_opt(struct link_util *lu, FILE *f, struct rtattr *t
> rta_getattr_u16(tb[IFLA_BOND_SLAVE_AD_AGGREGATOR_ID]));
> }
>
>+static int bond_slave_parse_opt(struct link_util *lu, int argc, char **argv,
>+ struct nlmsghdr *n)
>+{
>+ __u16 queue_id;
>+
>+ while (argc > 0) {
>+ if (matches(*argv, "queue_id") == 0) {
>+ NEXT_ARG();
>+ if (get_u16(&queue_id, *argv, 0))
>+ invarg("queue_id is invalid", *argv);
>+ addattr16(n, 1024, IFLA_BOND_SLAVE_QUEUE_ID, queue_id);
>+ }
>+ argc--, argv++;
>+ }
>+
>+ return 0;
>+}
>+
> struct link_util bond_slave_link_util = {
> .id = "bond",
> .maxattr = IFLA_BOND_SLAVE_MAX,
> .print_opt = bond_slave_print_opt,
>+ .parse_opt = bond_slave_parse_opt,
> .slave = true,
> };
>
>--
>1.9.3
>
^ permalink raw reply
* High perf top ip_idents_reserve doing netperf UDP_STREAM
From: Jesper Dangaard Brouer @ 2014-09-03 14:59 UTC (permalink / raw)
To: Eric Dumazet
Cc: brouer, netdev@vger.kernel.org, Daniel Borkmann,
Hannes Frederic Sowa, Florian Westphal
Hi Eric,
When doing:
super_netperf 120 -H 192.168.8.2 -t UDP_STREAM -l 100 -- -m 256
I'm seeing function ip_idents_reserve() consuming most CPU. Could you
help, explain what is going on, and how I can avoid this?
Perf top:
11.67% [kernel] [k] ip_idents_reserve
8.37% [kernel] [k] fib_table_lookup
4.46% [kernel] [k] _raw_spin_lock
3.21% [kernel] [k] copy_user_enhanced_fast_string
2.92% [kernel] [k] sock_alloc_send_pskb
2.88% [kernel] [k] udp_sendmsg
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net-next iproute2] iproute2: allow to change slave options via type_slave
From: Nikolay Aleksandrov @ 2014-09-03 14:43 UTC (permalink / raw)
To: netdev; +Cc: jiri, stephen, davem, Nikolay Aleksandrov
This patch adds the necessary changes to allow altering a slave device's
options via ip link set <device> type <master type>_slave specific-option.
It also adds support to set the bonding slaves' queue_id.
Example:
ip link set eth0 type bond_slave queue_id 10
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
ip/iplink.c | 22 +++++++++++++++++++---
ip/iplink_bond_slave.c | 19 +++++++++++++++++++
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/ip/iplink.c b/ip/iplink.c
index 1a907d998a87..5f2db92c981e 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -88,7 +88,8 @@ void iplink_usage(void)
fprintf(stderr, "\n");
fprintf(stderr, "TYPE := { vlan | veth | vcan | dummy | ifb | macvlan | macvtap |\n");
fprintf(stderr, " bridge | bond | ipoib | ip6tnl | ipip | sit | vxlan |\n");
- fprintf(stderr, " gre | gretap | ip6gre | ip6gretap | vti | nlmon }\n");
+ fprintf(stderr, " gre | gretap | ip6gre | ip6gretap | vti | nlmon |\n");
+ fprintf(stderr, " <master type>_slave }\n");
}
exit(-1);
}
@@ -697,14 +698,29 @@ static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
if (type) {
struct rtattr *linkinfo = NLMSG_TAIL(&req.n);
+ char slavebuf[128], *ulinep = strchr(type, '_');
+ int iflatype;
+
addattr_l(&req.n, sizeof(req), IFLA_LINKINFO, NULL, 0);
addattr_l(&req.n, sizeof(req), IFLA_INFO_KIND, type,
strlen(type));
- lu = get_link_kind(type);
+ if (ulinep && !strcmp(ulinep, "_slave")) {
+ strncpy(slavebuf, type, sizeof(slavebuf));
+ slavebuf[sizeof(slavebuf) - 1] = '\0';
+ ulinep = strchr(slavebuf, '_');
+ /* check in case it was after sizeof(slavebuf) - 1*/
+ if (ulinep)
+ *ulinep = '\0';
+ lu = get_link_slave_kind(slavebuf);
+ iflatype = IFLA_INFO_SLAVE_DATA;
+ } else {
+ lu = get_link_kind(type);
+ iflatype = IFLA_INFO_DATA;
+ }
if (lu && argc) {
struct rtattr * data = NLMSG_TAIL(&req.n);
- addattr_l(&req.n, sizeof(req), IFLA_INFO_DATA, NULL, 0);
+ addattr_l(&req.n, sizeof(req), iflatype, NULL, 0);
if (lu->parse_opt &&
lu->parse_opt(lu, argc, argv, &req.n))
diff --git a/ip/iplink_bond_slave.c b/ip/iplink_bond_slave.c
index 8f3fc6cec6fa..aacba14aef9c 100644
--- a/ip/iplink_bond_slave.c
+++ b/ip/iplink_bond_slave.c
@@ -80,10 +80,29 @@ static void bond_slave_print_opt(struct link_util *lu, FILE *f, struct rtattr *t
rta_getattr_u16(tb[IFLA_BOND_SLAVE_AD_AGGREGATOR_ID]));
}
+static int bond_slave_parse_opt(struct link_util *lu, int argc, char **argv,
+ struct nlmsghdr *n)
+{
+ __u16 queue_id;
+
+ while (argc > 0) {
+ if (matches(*argv, "queue_id") == 0) {
+ NEXT_ARG();
+ if (get_u16(&queue_id, *argv, 0))
+ invarg("queue_id is invalid", *argv);
+ addattr16(n, 1024, IFLA_BOND_SLAVE_QUEUE_ID, queue_id);
+ }
+ argc--, argv++;
+ }
+
+ return 0;
+}
+
struct link_util bond_slave_link_util = {
.id = "bond",
.maxattr = IFLA_BOND_SLAVE_MAX,
.print_opt = bond_slave_print_opt,
+ .parse_opt = bond_slave_parse_opt,
.slave = true,
};
--
1.9.3
^ permalink raw reply related
* Re: [net-next PATCH] qdisc: validate frames going through the direct_xmit path
From: Jesper Dangaard Brouer @ 2014-09-03 14:26 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Eric Dumazet, David S. Miller, Alexander Duyck, netdev
In-Reply-To: <20140903155213.62ff4eff@redhat.com>
On Wed, 3 Sep 2014 15:52:13 +0200
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> On Wed, 03 Sep 2014 06:43:25 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> >
> > Jesper, you missed another spot, when there is no qdisc on the device.
> >
> > __dev_queue_xmit() calls dev_hard_start_xmit() around line 2886
> >
> > Could we try to not add a myriad of small patches ?
> >
> > Some of us will need to backport all of them.
>
> I'm in the same backport situation ;-)
>
> I'll send a V2 of this patch, with missed spot...
(to avoid doing too many patches)
In the code below (from __dev_queue_xmit()), I've added validate_xmit_skb().
(dev_hard_start_xmit() can handle if it is a NULL ptr)
HARD_TX_LOCK(dev, txq, cpu);
if (!netif_xmit_stopped(txq)) {
skb = validate_xmit_skb(skb, dev);
__this_cpu_inc(xmit_recursion);
skb = dev_hard_start_xmit(skb, dev, txq, &rc);
__this_cpu_dec(xmit_recursion);
if (dev_xmit_complete(rc)) {
HARD_TX_UNLOCK(dev, txq);
goto out;
}
}
HARD_TX_UNLOCK(dev, txq);
net_crit_ratelimited("Virtual device %s asks to queue packet!\n",
dev->name);
BUT shouldn't we also handle if dev_hard_start_xmit() returns an skb pointer,
which means that there were skb's left on the skb list. Due to the rc
value "returned" in this case, we should see the net_crit_ratelimited()
msg, but we don't handle freeing these SKBs.
Any recommentations?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: Document EMAC Rockchip
From: Heiko Stübner @ 2014-09-03 14:26 UTC (permalink / raw)
To: Romain Perier; +Cc: linux-rockchip, linux-arm-kernel, netdev, devicetree, arnd
In-Reply-To: <1409740036-8117-2-git-send-email-romain.perier@gmail.com>
Am Mittwoch, 3. September 2014, 10:27:14 schrieb Romain Perier:
> This adds the necessary binding documentation for the EMAC Rockchip platform
> driver found in RK3066 and RK3188 SoCs.
>
> Signed-off-by: Romain Perier <romain.perier@gmail.com>
> ---
> .../devicetree/bindings/net/emac_rockchip.txt | 53
> ++++++++++++++++++++++ 1 file changed, 53 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/emac_rockchip.txt
>
> diff --git a/Documentation/devicetree/bindings/net/emac_rockchip.txt
> b/Documentation/devicetree/bindings/net/emac_rockchip.txt new file mode
> 100644
> index 0000000..d3242e4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/emac_rockchip.txt
> @@ -0,0 +1,53 @@
> +* ARC EMAC 10/100 Ethernet platform driver for Rockchip Rk3066/RK3188 SoCs
> +
> +Required properties:
> +- compatible: Should be "rockchip,rk3066-emac" or "rockchip,rk3188-emac"
> + according to the target SoC.
> +- reg: Address and length of the register set for the device
> +- interrupts: Should contain the EMAC interrupts
> +- pinctrl-names: pin control state name ("default").
> +- pinctrl-0: pin control group list of phandle. It must contain
> + pin control groups for communication with the phy and for the mii bus.
pinctrl is normally not supposed to be part of the binding doc, as it is a
property of the board and not the controller IP itself
> +- rockchip,grf: phandle to the syscon grf used to control speed and mode
> + for emac.
> +- phy: see ethernet.txt file in the same directory.
> +- phy-mode: see ethernet.txt file in the same directory.
> +
> +Optional properties:
> +- phy-supply: phandle to a regulator if the PHY needs one
> +
> +Clock handling:
> +- The host clock is needed to calculate and set polling period of EMAC.
> + It must be provided by the clock "hclk".
> +- The reference clock is needed to get/set data from phy at the right
> frequency,
> + according to its "phy-mode". It must be provided by the clock
> "macref".
I think a more often used scheme is something like
- clocks: Must contain an entry for each entry in clock-names.
- clock-names: Shall be "hclk" for the host clock needed to calculate and
set polling period of EMAC and "macref" for the reference
clock needed to transfer data to and from the phy.
> +
> +Child nodes of the driver are the individual PHY devices connected to the
> +MDIO bus. They must have a "reg" property given the PHY address on the MDIO
> bus. +
> +Examples:
> +
> +ethernet@10204000 {
> + compatible = "rockchip,rk3188-emac";
> + reg = <0xc0fc2000 0x3c>;
> + interrupts = <6>;
> + mac-address = [ 00 11 22 33 44 55 ];
> +
> + clocks = <&cru HCLK_EMAC>, <&cru SCLK_MAC>;
> + clock-names = "hclk", "macref";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&emac_xfer>, <&emac_mdio>, <&phy_int>;
> +
> + rockchip,grf = <&grf>;
> +
> + phy = <&phy0>;
> + phy-mode = "rmii";
> + phy-supply = <&vcc_rmii>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + phy0: ethernet-phy@0 {
> + reg = <1>;
> + };
> +};
^ permalink raw reply
* Re: [PATCH net] l2tp: fix race while getting PMTU on PPP pseudo-wire
From: Eric Dumazet @ 2014-09-03 13:55 UTC (permalink / raw)
To: Guillaume Nault, Dmitry Petukhov; +Cc: netdev, James Chapman, David S. Miller
In-Reply-To: <3a4673ddb2556634d0cbeae9b31ebc386e720335.1409746289.git.g.nault@alphalink.fr>
On Wed, 2014-09-03 at 14:12 +0200, Guillaume Nault wrote:
> Use dst_entry held by sk_dst_get() to retrieve tunnel's PMTU.
>
> The dst_mtu(__sk_dst_get(tunnel->sock)) call was racy. __sk_dst_get()
> could return NULL if tunnel->sock->sk_dst_cache was reset just before the
> call, thus making dst_mtu() dereference a NULL pointer:
>
> [ 1937.661598] BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
> [ 1937.664005] IP: [<ffffffffa049db88>] pppol2tp_connect+0x33d/0x41e [l2tp_ppp]
> [ 1937.664005] PGD daf0c067 PUD d9f93067 PMD 0
> [ 1937.664005] Oops: 0000 [#1] SMP
> [ 1937.664005] Modules linked in: l2tp_ppp l2tp_netlink l2tp_core ip6table_filter ip6_tables iptable_filter ip_tables ebtable_nat ebtables x_tables udp_tunnel pppoe pppox ppp_generic slhc deflate ctr twofish_generic twofish_x86_64_3way xts lrw gf128mul glue_helper twofish_x86_64 twofish_common blowfish_generic blowfish_x86_64 blowfish_common des_generic cbc xcbc rmd160 sha512_generic hmac crypto_null af_key xfrm_algo 8021q garp bridge stp llc tun atmtcp clip atm ext3 mbcache jbd iTCO_wdt coretemp kvm_intel iTCO_vendor_support kvm pcspkr evdev ehci_pci lpc_ich mfd_core i5400_edac edac_core i5k_amb shpchp button processor thermal_sys xfs crc32c_generic libcrc32c dm_mod usbhid sg hid sr_mod sd_mod cdrom crc_t10dif crct10dif_common ata_generic ahci ata_piix tg3 libahci libata uhci_hcd ptp eh
ci_hcd pps_core usbcore scsi_mod libphy usb_common [last unloaded: l2tp_core]
> [ 1937.664005] CPU: 0 PID: 10022 Comm: l2tpstress Tainted: G O 3.17.0-rc1 #1
> [ 1937.664005] Hardware name: HP ProLiant DL160 G5, BIOS O12 08/22/2008
> [ 1937.664005] task: ffff8800d8fda790 ti: ffff8800c43c4000 task.ti: ffff8800c43c4000
> [ 1937.664005] RIP: 0010:[<ffffffffa049db88>] [<ffffffffa049db88>] pppol2tp_connect+0x33d/0x41e [l2tp_ppp]
> [ 1937.664005] RSP: 0018:ffff8800c43c7de8 EFLAGS: 00010282
> [ 1937.664005] RAX: ffff8800da8a7240 RBX: ffff8800d8c64600 RCX: 000001c325a137b5
> [ 1937.664005] RDX: 8c6318c6318c6320 RSI: 000000000000010c RDI: 0000000000000000
> [ 1937.664005] RBP: ffff8800c43c7ea8 R08: 0000000000000000 R09: 0000000000000000
> [ 1937.664005] R10: ffffffffa048e2c0 R11: ffff8800d8c64600 R12: ffff8800ca7a5000
> [ 1937.664005] R13: ffff8800c439bf40 R14: 000000000000000c R15: 0000000000000009
> [ 1937.664005] FS: 00007fd7f610f700(0000) GS:ffff88011a600000(0000) knlGS:0000000000000000
> [ 1937.664005] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 1937.664005] CR2: 0000000000000020 CR3: 00000000d9d75000 CR4: 00000000000027e0
> [ 1937.664005] Stack:
> [ 1937.664005] ffffffffa049da80 ffff8800d8fda790 000000000000005b ffff880000000009
> [ 1937.664005] ffff8800daf3f200 0000000000000003 ffff8800c43c7e48 ffffffff81109b57
> [ 1937.664005] ffffffff81109b0e ffffffff8114c566 0000000000000000 0000000000000000
> [ 1937.664005] Call Trace:
> [ 1937.664005] [<ffffffffa049da80>] ? pppol2tp_connect+0x235/0x41e [l2tp_ppp]
> [ 1937.664005] [<ffffffff81109b57>] ? might_fault+0x9e/0xa5
> [ 1937.664005] [<ffffffff81109b0e>] ? might_fault+0x55/0xa5
> [ 1937.664005] [<ffffffff8114c566>] ? rcu_read_unlock+0x1c/0x26
> [ 1937.664005] [<ffffffff81309196>] SYSC_connect+0x87/0xb1
> [ 1937.664005] [<ffffffff813e56f7>] ? sysret_check+0x1b/0x56
> [ 1937.664005] [<ffffffff8107590d>] ? trace_hardirqs_on_caller+0x145/0x1a1
> [ 1937.664005] [<ffffffff81213dee>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [ 1937.664005] [<ffffffff8114c262>] ? spin_lock+0x9/0xb
> [ 1937.664005] [<ffffffff813092b4>] SyS_connect+0x9/0xb
> [ 1937.664005] [<ffffffff813e56d2>] system_call_fastpath+0x16/0x1b
> [ 1937.664005] Code: 10 2a 84 81 e8 65 76 bd e0 65 ff 0c 25 10 bb 00 00 4d 85 ed 74 37 48 8b 85 60 ff ff ff 48 8b 80 88 01 00 00 48 8b b8 10 02 00 00 <48> 8b 47 20 ff 50 20 85 c0 74 0f 83 e8 28 89 83 10 01 00 00 89
> [ 1937.664005] RIP [<ffffffffa049db88>] pppol2tp_connect+0x33d/0x41e [l2tp_ppp]
> [ 1937.664005] RSP <ffff8800c43c7de8>
> [ 1937.664005] CR2: 0000000000000020
> [ 1939.559375] ---[ end trace 82d44500f28f8708 ]---
>
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
> ---
> net/l2tp/l2tp_ppp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
> index 13752d9..b704a93 100644
> --- a/net/l2tp/l2tp_ppp.c
> +++ b/net/l2tp/l2tp_ppp.c
> @@ -755,7 +755,8 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
> /* If PMTU discovery was enabled, use the MTU that was discovered */
> dst = sk_dst_get(tunnel->sock);
> if (dst != NULL) {
> - u32 pmtu = dst_mtu(__sk_dst_get(tunnel->sock));
> + u32 pmtu = dst_mtu(dst);
> +
> if (pmtu != 0)
> session->mtu = session->mru = pmtu -
> PPPOL2TP_HEADER_OVERHEAD;
Probably introduced in commit f34c4a35d879 ("l2tp: take PMTU from tunnel
UDP socket")
CC Dmitry
Before this patch, the bug could not happen.
Acked-by: Eric Dumazet <edumazet@google.com>
Fixes: f34c4a35d879 ("l2tp: take PMTU from tunnel UDP socket")
^ permalink raw reply
* Re: [net-next PATCH] qdisc: validate frames going through the direct_xmit path
From: Jesper Dangaard Brouer @ 2014-09-03 13:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Alexander Duyck, netdev, brouer
In-Reply-To: <1409751805.26422.32.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, 03 Sep 2014 06:43:25 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2014-09-03 at 13:48 +0200, Jesper Dangaard Brouer wrote:
> > In commit 50cbe9ab5f8d ("net: Validate xmit SKBs right when we pull them
> > out of the qdisc") the validation code was moved out of dev_hard_start_xmit
> > and into dequeue_skb. However this overlooked the fact that we do not
> > always enqueue the skb onto a qdisc, if qdisc have flag TCQ_F_CAN_BYPASS.
> >
> > As a result Alex was seeing issues trying to connect to a vhost_net interface
> > after this patch was applied.
> >
> > Added a call to validate_xmit_skb in __dev_xmit_skb(), in the code path
> > for qdiscs with TCQ_F_CAN_BYPASS flag.
> >
> > Fixes: 50cbe9ab5f8d ("net: Validate xmit SKBs right when we pull them out of the qdisc")
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > ---
>
> Jesper, you missed another spot, when there is no qdisc on the device.
>
> __dev_queue_xmit() calls dev_hard_start_xmit() around line 2886
>
> Could we try to not add a myriad of small patches ?
>
> Some of us will need to backport all of them.
I'm in the same backport situation ;-)
I'll send a V2 of this patch, with missed spot...
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v2 net-next] net: Functions to report space available in device TX queues
From: Jesper Dangaard Brouer @ 2014-09-03 13:49 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Tom Herbert, davem, netdev, brouer
In-Reply-To: <1409751717.26422.31.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, 03 Sep 2014 06:41:57 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2014-09-03 at 15:20 +0200, Jesper Dangaard Brouer wrote:
> > On Mon, 25 Aug 2014 08:27:56 -0700 (PDT)
> > Tom Herbert <therbert@google.com> wrote:
> >
> > > This patch adds netdev_tx_avail_queue and netdev_avail_queue which are
> > > used to report number of bytes available in transmit queues per BQL. The
> > > functions call dql_avail which returns BQL limit minus number of
> > > inflight bytes. These functions can be called without txlock, for
> > > instance to ascertain how much data should be dequeued from a qdisc in
> > > a batch. When called without the tx_lock, the result is technically a
> > > hint, subsequently when the tx_lock is done for a transmit it is
> > > possible the availability has changed (for example a transmit
> > > completion may have freed up more space in the queue or changed the
> > > limit).
> > >
> > > Signed-off-by: Tom Herbert <therbert@google.com>
> > > ---
> > > include/linux/netdevice.h | 28 ++++++++++++++++++++++++++--
> > > 1 file changed, 26 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index 0fac884..bdf6c85 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -2544,6 +2544,30 @@ static inline void netdev_completed_queue(struct net_device *dev,
> > > netdev_tx_completed_queue(netdev_get_tx_queue(dev, 0), pkts, bytes);
> > > }
> > >
> > > +static inline int netdev_tx_avail_queue(struct netdev_queue *dev_queue)
> > > +{
> > > +#ifdef CONFIG_BQL
> > > + return dql_avail(&dev_queue->dql);
> > > +#else
> > > + return DQL_MAX_LIMIT;
> > > +#endif
> > > +}
> > > +
> > > +/**
> > > + * netdev_avail_queue - report how much space is availble for xmit
> > > + * @dev: network device
> > > + *
> > > + * Report the amount of space available in the TX queue in terms of
> > > + * number of bytes. This returns the number of bytes avaiable per
> > > + * DQL. This function may be called without taking the txlock on
> > > + * the device, however in that case the result should be taken as
> > > + * a (strong) hint.
> > > + */
> > > +static inline int netdev_avail_queue(struct net_device *dev)
> > > +{
> > > + return netdev_tx_avail_queue(netdev_get_tx_queue(dev, 0));
> > > +}
> > > +
> >
> > How can this work, when you are always passing 0 to: netdev_get_tx_queue(dev, 0)
> > ???
> >
>
> This is the helper for drivers having a single TX queue.
Okay, so I should use:
bytelimit = netdev_tx_avail_queue(txq);
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [net-next PATCH] qdisc: validate frames going through the direct_xmit path
From: Eric Dumazet @ 2014-09-03 13:43 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: David S. Miller, Alexander Duyck, netdev
In-Reply-To: <20140903114841.19969.22671.stgit@dragon>
On Wed, 2014-09-03 at 13:48 +0200, Jesper Dangaard Brouer wrote:
> In commit 50cbe9ab5f8d ("net: Validate xmit SKBs right when we pull them
> out of the qdisc") the validation code was moved out of dev_hard_start_xmit
> and into dequeue_skb. However this overlooked the fact that we do not
> always enqueue the skb onto a qdisc, if qdisc have flag TCQ_F_CAN_BYPASS.
>
> As a result Alex was seeing issues trying to connect to a vhost_net interface
> after this patch was applied.
>
> Added a call to validate_xmit_skb in __dev_xmit_skb(), in the code path
> for qdiscs with TCQ_F_CAN_BYPASS flag.
>
> Fixes: 50cbe9ab5f8d ("net: Validate xmit SKBs right when we pull them out of the qdisc")
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
Jesper, you missed another spot, when there is no qdisc on the device.
__dev_queue_xmit() calls dev_hard_start_xmit() around line 2886
Could we try to not add a myriad of small patches ?
Some of us will need to backport all of them.
Thanks.
^ 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