* [PATCH net 0/4] ipv4: various pmtu discovery fixes
From: Steffen Klassert @ 2011-10-11 11:08 UTC (permalink / raw)
To: David Miller; +Cc: netdev
This patchset addresses some issues I found during investigating
pmtu discovery. These issues were introduced with
git commit 2c8cec5 (ipv4: Cache learned PMTU information in inetpeer).
The patchset is based on current net/master.
Steffen
^ permalink raw reply
* Re: [net-next PATCH] net: allow vlan traffic to be received under bond
From: Hans Schillstrom @ 2011-10-11 11:08 UTC (permalink / raw)
To: Jesse Gross
Cc: John Fastabend, Jiri Pirko, davem@davemloft.net,
netdev@vger.kernel.org, fubar@us.ibm.com
In-Reply-To: <CAEP_g=83tCQMVPOTT82GBw6GamodpqU-SAeYyx9noWnzbGfUpg@mail.gmail.com>
Hello
On Tuesday 11 October 2011 04:43:03 Jesse Gross wrote:
> On Mon, Oct 10, 2011 at 7:07 PM, John Fastabend
> <john.r.fastabend@intel.com> wrote:
> > On 10/10/2011 3:37 PM, Jiri Pirko wrote:
> >> Mon, Oct 10, 2011 at 09:16:41PM CEST, john.r.fastabend@intel.com wrote:
> >>> The following configuration used to work as I expected. At least
> >>> we could use the fcoe interfaces to do MPIO and the bond0 iface
> >>> to do load balancing or failover.
> >>>
> >>> ---eth2.228-fcoe
> >>> |
> >>> eth2 -----|
> >>> |
> >>> |---- bond0
> >>> |
> >>> eth3 -----|
> >>> |
> >>> ---eth3.228-fcoe
> >>>
> >>> This worked because of a change we added to allow inactive slaves
> >>> to rx 'exact' matches. This functionality was kept intact with the
> >>> rx_handler mechanism. However now the vlan interface attached to the
> >>> active slave never receives traffic because the bonding rx_handler
> >>> updates the skb->dev and goto's another_round. Previously, the
> >>> vlan_do_receive() logic was called before the bonding rx_handler.
> >>>
> >>> Now by the time vlan_do_receive calls vlan_find_dev() the
> >>> skb->dev is set to bond0 and it is clear no vlan is attached
> >>> to this iface. The vlan lookup fails.
> >>>
> >>> This patch moves the VLAN check above the rx_handler. A VLAN
> >>> tagged frame is now routed to the eth2.228-fcoe iface in the
> >>> above schematic. Untagged frames continue to the bond0 as
> >>> normal. This case also remains intact,
> >>>
> >>> eth2 --> bond0 --> vlan.228
> >>>
> >>> Here the skb is VLAN tagged but the vlan lookup fails on eth2
> >>> causing the bonding rx_handler to be called. On the second
> >>> pass the vlan lookup is on the bond0 iface and completes as
> >>> expected.
> >>>
> >>> Putting a VLAN.228 on both the bond0 and eth2 device will
> >>> result in eth2.228 receiving the skb. I don't think this is
> >>> completely unexpected and was the result prior to the rx_handler
> >>> result.
I think this OK, but I do have a question
if bond0 is in Active/Backup mode, eth2 and eth3 got the same MAC.addr,
what about the VLAN:s ?
(or is just one of thme working ??)
> >>>
> >>> Note, the same setup is also used for other storage traffic that
> >>> MPIO is used with eg. iSCSI and similar setups can be contrived
> >>> without storage protocols.
> >>>
> >>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> >>> ---
> >>>
> >>> net/core/dev.c | 22 +++++++++++-----------
> >>> 1 files changed, 11 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/net/core/dev.c b/net/core/dev.c
> >>> index 70ecb86..8b6118a 100644
> >>> --- a/net/core/dev.c
> >>> +++ b/net/core/dev.c
> >>> @@ -3231,6 +3231,17 @@ another_round:
> >>> ncls:
> >>> #endif
> >>>
> >>> + if (vlan_tx_tag_present(skb)) {
> >>> + if (pt_prev) {
> >>> + ret = deliver_skb(skb, pt_prev, orig_dev);
> >>> + pt_prev = NULL;
> >>> + }
> >>> + if (vlan_do_receive(&skb))
> >>> + goto another_round;
> >>> + else if (unlikely(!skb))
> >>> + goto out;
> >>> + }
> >>> +
> >>> rx_handler = rcu_dereference(skb->dev->rx_handler);
> >>> if (rx_handler) {
> >>> if (pt_prev) {
> >>> @@ -3251,17 +3262,6 @@ ncls:
> >>> }
> >>> }
> >>>
> >>> - if (vlan_tx_tag_present(skb)) {
> >>> - if (pt_prev) {
> >>> - ret = deliver_skb(skb, pt_prev, orig_dev);
> >>> - pt_prev = NULL;
> >>> - }
> >>> - if (vlan_do_receive(&skb))
> >>> - goto another_round;
> >>> - else if (unlikely(!skb))
> >>> - goto out;
> >>> - }
> >>> -
> >>> /* deliver only exact match when indicated */
> >>> null_or_dev = deliver_exact ? skb->dev : NULL;
> >>>
> >>>
> >>
> >> Hmm, I must look at this again tomorrow but I have strong feeling this
> >> will break some some scenario including vlan-bridge-macvlan.
> >
> > Yes please review... I tested cases with vlan, bridge, and macvlan
> > components and believe this works unless I missed something.
> >
> > Maybe Jesse, can comment though on why this commit that moved (and
> > cleaned up) the vlan tag handling put the vlan_do_receive below the
> > rx_handler rather than above it. Was this intended to fix something?
>
> The original reason was to ensure packets received from NICs that do
> stripping behaved the same as those that don't. At the time, the
> packets with inline vlan tags were handled by the same code that
> handled upper layer protocols so it was important that code for vlan
> stripped tags be immediately before that. Otherwise, packets might be
> taken either by the bridge hook or vlan code depending the the type of
> device.
>
> However, that's no longer an issue because we now emulate vlan
> acceleration by untagging packets at the beginning of
> __netif_receive_skb(), so the code path will always be the same.
> Furthermore, based on feedback received since that patch it seems
> pretty clear that people prefer the behavior where vlan devices take
> traffic first, so now that we can have both that and consistent
> behavior it seems to be the way to go.
>
> This looks correct to me:
> Acked-by: Jesse Gross <jesse@nicira.com>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply
* [PATCH 1/4] ipv4: Fix pmtu propagating
From: Steffen Klassert @ 2011-10-11 11:09 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20111011110842.GC1830@secunet.com>
Since commit 2c8cec5c (ipv4: Cache learned PMTU information in inetpeer)
we cache the learned pmtu informations in inetpeer and propagate these
informations with the dst_ops->check() functions. However, these functions
might not be called for udp and raw packets. As a consequence, we don't
use the learned pmtu informations in these cases. With this patch we
call dst_check() from ip_setup_cork() to propagate the pmtu informations.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/ip_output.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 8c65633..f682437 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1063,6 +1063,11 @@ static int ip_setup_cork(struct sock *sk, struct inet_cork *cork,
rt = *rtp;
if (unlikely(!rt))
return -EFAULT;
+
+ rt = (struct rtable *)dst_check(&rt->dst, 0);
+ if (!rt)
+ return -EHOSTUNREACH;
+
/*
* We steal reference to this route, caller should not release it
*/
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/4] ipv4: Update pmtu informations on inetpeer only for output routes
From: Steffen Klassert @ 2011-10-11 11:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20111011110842.GC1830@secunet.com>
The pmtu informations on the inetpeer are visible for output and
input routes. On packet forwarding, we might propagate a learned
pmtu to the sender. As we update the pmtu informations of the
inetpeer on demand, the original sender of the forwarded packets
might never notice when the pmtu to that inetpeer increases.
So propagate the nexthop mtu instead of the pmtu to the final
destination.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/route.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 075212e..9a6623e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1579,9 +1579,10 @@ unsigned short ip_rt_frag_needed(struct net *net, const struct iphdr *iph,
static void check_peer_pmtu(struct dst_entry *dst, struct inet_peer *peer)
{
+ struct rtable *rt = (struct rtable *) dst;
unsigned long expires = ACCESS_ONCE(peer->pmtu_expires);
- if (!expires)
+ if (rt_is_input_route(rt) || !expires)
return;
if (time_before(jiffies, expires)) {
u32 orig_dst_mtu = dst_mtu(dst);
@@ -1803,6 +1804,7 @@ static void rt_init_metrics(struct rtable *rt, const struct flowi4 *fl4,
struct fib_info *fi)
{
struct inet_peer *peer;
+ struct dst_entry *dst = &rt->dst;
int create = 0;
/* If a peer entry exists for this destination, we must hook
@@ -1817,9 +1819,14 @@ static void rt_init_metrics(struct rtable *rt, const struct flowi4 *fl4,
if (inet_metrics_new(peer))
memcpy(peer->metrics, fi->fib_metrics,
sizeof(u32) * RTAX_MAX);
- dst_init_metrics(&rt->dst, peer->metrics, false);
- check_peer_pmtu(&rt->dst, peer);
+ dst_init_metrics(dst, peer->metrics, false);
+ check_peer_pmtu(dst, peer);
+
+ if (rt_is_input_route(rt))
+ dst_metric_set(dst, RTAX_MTU,
+ dst->ops->default_mtu(dst));
+
if (peer->redirect_learned.a4 &&
peer->redirect_learned.a4 != rt->rt_gateway) {
rt->rt_gateway = peer->redirect_learned.a4;
@@ -1830,7 +1837,7 @@ static void rt_init_metrics(struct rtable *rt, const struct flowi4 *fl4,
rt->fi = fi;
atomic_inc(&fi->fib_clntref);
}
- dst_init_metrics(&rt->dst, fi->fib_metrics, true);
+ dst_init_metrics(dst, fi->fib_metrics, true);
}
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/4] ipv4: Fix inetpeer expiration handling
From: Steffen Klassert @ 2011-10-11 11:11 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20111011110842.GC1830@secunet.com>
We leak a trigger to check if the learned pmtu information has
expired, so the learned pmtu informations never expire. This patch
add such a trigger to ipv4_dst_check().
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/route.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 9a6623e..cda370c 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1660,6 +1660,10 @@ static struct dst_entry *ipv4_dst_check(struct dst_entry *dst, u32 cookie)
if (rt_is_expired(rt))
return NULL;
+
+ if (rt->peer && peer_pmtu_expired(rt->peer))
+ dst_metric_set(dst, RTAX_MTU, rt->peer->pmtu_orig);
+
if (rt->rt_peer_genid != rt_peer_genid()) {
struct inet_peer *peer;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 4/4] ipv4: Fix inetpeer expire time information
From: Steffen Klassert @ 2011-10-11 11:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20111011110842.GC1830@secunet.com>
As we update the learned pmtu informations on demand, we might
report a nagative expiration time value to userspace if the
pmtu informations are already expired and we have not send a
packet to that inetpeer after expiration. With this patch we
send a expire time of null to userspace after expiration
until the next packet is send to that inetpeer.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/route.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index cda370c..501a3c0 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2832,7 +2832,7 @@ static int rt_fill_info(struct net *net,
struct rtable *rt = skb_rtable(skb);
struct rtmsg *r;
struct nlmsghdr *nlh;
- long expires = 0;
+ unsigned long expires = 0;
const struct inet_peer *peer = rt->peer;
u32 id = 0, ts = 0, tsage = 0, error;
@@ -2889,8 +2889,12 @@ static int rt_fill_info(struct net *net,
tsage = get_seconds() - peer->tcp_ts_stamp;
}
expires = ACCESS_ONCE(peer->pmtu_expires);
- if (expires)
- expires -= jiffies;
+ if (expires) {
+ if (time_before(jiffies, expires))
+ expires -= jiffies;
+ else
+ expires = 0;
+ }
}
if (rt_is_input_route(rt)) {
--
1.7.0.4
^ permalink raw reply related
* Re: e100 + VLANs?
From: Eric Dumazet @ 2011-10-11 11:25 UTC (permalink / raw)
To: Michael Tokarev; +Cc: David Lamparter, jeffrey.t.kirsher, netdev
In-Reply-To: <4E941198.9000307@msgid.tls.msk.ru>
[-- Attachment #1: Type: text/plain, Size: 1976 bytes --]
> So, is that a hardware limitation?
Its a driver bug
This comes from fact that sizeof(struct rfd) = 16
and VLAN_ETH_FRAME_LEN is 1518
driver mixes VLAN_ETH_FRAME_LEN and 1500+4+sizeof(struct rfd) (1520, not 1518)
It therefore misses 2 bytes for large frames (VLAN tagged)
Fix is to remove VLAN_ETH_FRAME_LEN references for good...
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index c1352c6..3287d31 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -435,6 +435,7 @@ struct rfd {
__le16 actual_size;
__le16 size;
};
+#define RFD_BUF_LEN (sizeof(struct rfd) + ETH_DATA_LEN + VLAN_HLEN)
struct rx {
struct rx *next, *prev;
@@ -1075,7 +1076,7 @@ static void e100_get_defaults(struct nic *nic)
/* Template for a freshly allocated RFD */
nic->blank_rfd.command = 0;
nic->blank_rfd.rbd = cpu_to_le32(0xFFFFFFFF);
- nic->blank_rfd.size = cpu_to_le16(VLAN_ETH_FRAME_LEN);
+ nic->blank_rfd.size = cpu_to_le16(RFD_BUF_LEN);
/* MII setup */
nic->mii.phy_id_mask = 0x1F;
@@ -1881,7 +1882,6 @@ static inline void e100_start_receiver(struct
nic *nic, struct rx *rx)
}
}
-#define RFD_BUF_LEN (sizeof(struct rfd) + VLAN_ETH_FRAME_LEN)
static int e100_rx_alloc_skb(struct nic *nic, struct rx *rx)
{
if (!(rx->skb = netdev_alloc_skb_ip_align(nic->netdev, RFD_BUF_LEN)))
@@ -2058,7 +2058,7 @@ static void e100_rx_clean(struct nic *nic,
unsigned int *work_done,
pci_dma_sync_single_for_device(nic->pdev,
old_before_last_rx->dma_addr, sizeof(struct rfd),
PCI_DMA_BIDIRECTIONAL);
- old_before_last_rfd->size = cpu_to_le16(VLAN_ETH_FRAME_LEN);
+ old_before_last_rfd->size = cpu_to_le16(RFD_BUF_LEN);
pci_dma_sync_single_for_device(nic->pdev,
old_before_last_rx->dma_addr, sizeof(struct rfd),
PCI_DMA_BIDIRECTIONAL);
[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 1434 bytes --]
diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index c1352c6..3287d31 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -435,6 +435,7 @@ struct rfd {
__le16 actual_size;
__le16 size;
};
+#define RFD_BUF_LEN (sizeof(struct rfd) + ETH_DATA_LEN + VLAN_HLEN)
struct rx {
struct rx *next, *prev;
@@ -1075,7 +1076,7 @@ static void e100_get_defaults(struct nic *nic)
/* Template for a freshly allocated RFD */
nic->blank_rfd.command = 0;
nic->blank_rfd.rbd = cpu_to_le32(0xFFFFFFFF);
- nic->blank_rfd.size = cpu_to_le16(VLAN_ETH_FRAME_LEN);
+ nic->blank_rfd.size = cpu_to_le16(RFD_BUF_LEN);
/* MII setup */
nic->mii.phy_id_mask = 0x1F;
@@ -1881,7 +1882,6 @@ static inline void e100_start_receiver(struct nic *nic, struct rx *rx)
}
}
-#define RFD_BUF_LEN (sizeof(struct rfd) + VLAN_ETH_FRAME_LEN)
static int e100_rx_alloc_skb(struct nic *nic, struct rx *rx)
{
if (!(rx->skb = netdev_alloc_skb_ip_align(nic->netdev, RFD_BUF_LEN)))
@@ -2058,7 +2058,7 @@ static void e100_rx_clean(struct nic *nic, unsigned int *work_done,
pci_dma_sync_single_for_device(nic->pdev,
old_before_last_rx->dma_addr, sizeof(struct rfd),
PCI_DMA_BIDIRECTIONAL);
- old_before_last_rfd->size = cpu_to_le16(VLAN_ETH_FRAME_LEN);
+ old_before_last_rfd->size = cpu_to_le16(RFD_BUF_LEN);
pci_dma_sync_single_for_device(nic->pdev,
old_before_last_rx->dma_addr, sizeof(struct rfd),
PCI_DMA_BIDIRECTIONAL);
^ permalink raw reply related
* [PATCH net-next 0/2] ipv6: IPsec pmtu/fragmentation fixes
From: Steffen Klassert @ 2011-10-11 11:42 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, netdev
This patchset fixes two issues for IPsec on ipv6. These issues
are arround, basically forever.
The patchset is based on current net-next/master.
1) Fix for a IPv6 IPsec slowpath fragmentation problem.
Already fixed for IPv4 with
git commit 353e5c9 (ipv4: Fix IPsec slowpath fragmentation problem).
2) Fix an incorrect update of the path mtu on IPv6 IPsec.
Already fixed for IPv4 with
git commit b00897b (xfrm4: Don't call icmp_send on local error).
^ permalink raw reply
* [PATCH 1/2] ipv6: Fix IPsec slowpath fragmentation problem
From: Steffen Klassert @ 2011-10-11 11:43 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, netdev
In-Reply-To: <20111011114251.GH1830@secunet.com>
ip6_append_data() builds packets based on the mtu from dst_mtu(rt->dst.path).
On IPsec the effective mtu is lower because we need to add the protocol
headers and trailers later when we do the IPsec transformations. So after
the IPsec transformations the packet might be too big, which leads to a
slowpath fragmentation then. This patch fixes this by building the packets
based on the lower IPsec mtu from dst_mtu(&rt->dst) and adapts the exthdr
handling to this.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/ip6_output.c | 18 ++++++++++++------
net/ipv6/raw.c | 3 +--
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 835c04b..1e20b64 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1193,6 +1193,7 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
struct sk_buff *skb;
unsigned int maxfraglen, fragheaderlen;
int exthdrlen;
+ int dst_exthdrlen;
int hh_len;
int mtu;
int copy;
@@ -1248,7 +1249,7 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
np->cork.hop_limit = hlimit;
np->cork.tclass = tclass;
mtu = np->pmtudisc == IPV6_PMTUDISC_PROBE ?
- rt->dst.dev->mtu : dst_mtu(rt->dst.path);
+ rt->dst.dev->mtu : dst_mtu(&rt->dst);
if (np->frag_size < mtu) {
if (np->frag_size)
mtu = np->frag_size;
@@ -1259,16 +1260,17 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
cork->length = 0;
sk->sk_sndmsg_page = NULL;
sk->sk_sndmsg_off = 0;
- exthdrlen = rt->dst.header_len + (opt ? opt->opt_flen : 0) -
- rt->rt6i_nfheader_len;
+ exthdrlen = (opt ? opt->opt_flen : 0) - rt->rt6i_nfheader_len;
length += exthdrlen;
transhdrlen += exthdrlen;
+ dst_exthdrlen = rt->dst.header_len;
} else {
rt = (struct rt6_info *)cork->dst;
fl6 = &inet->cork.fl.u.ip6;
opt = np->cork.opt;
transhdrlen = 0;
exthdrlen = 0;
+ dst_exthdrlen = 0;
mtu = cork->fragsize;
}
@@ -1368,6 +1370,8 @@ alloc_new_skb:
else
alloclen = datalen + fragheaderlen;
+ alloclen += dst_exthdrlen;
+
/*
* The last fragment gets additional space at tail.
* Note: we overallocate on fragments with MSG_MODE
@@ -1419,9 +1423,9 @@ alloc_new_skb:
/*
* Find where to start putting bytes
*/
- data = skb_put(skb, fraglen);
- skb_set_network_header(skb, exthdrlen);
- data += fragheaderlen;
+ data = skb_put(skb, fraglen + dst_exthdrlen);
+ skb_set_network_header(skb, exthdrlen + dst_exthdrlen);
+ data += fragheaderlen + dst_exthdrlen;
skb->transport_header = (skb->network_header +
fragheaderlen);
if (fraggap) {
@@ -1434,6 +1438,7 @@ alloc_new_skb:
pskb_trim_unique(skb_prev, maxfraglen);
}
copy = datalen - transhdrlen - fraggap;
+
if (copy < 0) {
err = -EINVAL;
kfree_skb(skb);
@@ -1448,6 +1453,7 @@ alloc_new_skb:
length -= datalen - fraggap;
transhdrlen = 0;
exthdrlen = 0;
+ dst_exthdrlen = 0;
csummode = CHECKSUM_NONE;
/*
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 3486f62..6f7824e 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -542,8 +542,7 @@ static int rawv6_push_pending_frames(struct sock *sk, struct flowi6 *fl6,
goto out;
offset = rp->offset;
- total_len = inet_sk(sk)->cork.base.length - (skb_network_header(skb) -
- skb->data);
+ total_len = inet_sk(sk)->cork.base.length;
if (offset >= total_len - 1) {
err = -EINVAL;
ip6_flush_pending_frames(sk);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/2] xfrm6: Don't call icmpv6_send on local error
From: Steffen Klassert @ 2011-10-11 11:44 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, netdev
In-Reply-To: <20111011114251.GH1830@secunet.com>
Calling icmpv6_send() on a local message size error leads to
an incorrect update of the path mtu. So use xfrm6_local_rxpmtu()
to notify about the pmtu if the IPV6_DONTFRAG socket option is
set on an udp or raw socket, according RFC 3542 and use
ipv6_local_error() otherwise.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/xfrm6_output.c | 56 +++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 54 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
index 49a91c5..faae417 100644
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -28,6 +28,43 @@ int xfrm6_find_1stfragopt(struct xfrm_state *x, struct sk_buff *skb,
EXPORT_SYMBOL(xfrm6_find_1stfragopt);
+static int xfrm6_local_dontfrag(struct sk_buff *skb)
+{
+ int proto;
+ struct sock *sk = skb->sk;
+
+ if (sk) {
+ proto = sk->sk_protocol;
+
+ if (proto == IPPROTO_UDP || proto == IPPROTO_RAW)
+ return inet6_sk(sk)->dontfrag;
+ }
+
+ return 0;
+}
+
+static void xfrm6_local_rxpmtu(struct sk_buff *skb, u32 mtu)
+{
+ struct flowi6 fl6;
+ struct sock *sk = skb->sk;
+
+ fl6.flowi6_oif = sk->sk_bound_dev_if;
+ ipv6_addr_copy(&fl6.daddr, &ipv6_hdr(skb)->daddr);
+
+ ipv6_local_rxpmtu(sk, &fl6, mtu);
+}
+
+static void xfrm6_local_error(struct sk_buff *skb, u32 mtu)
+{
+ struct flowi6 fl6;
+ struct sock *sk = skb->sk;
+
+ fl6.fl6_dport = inet_sk(sk)->inet_dport;
+ ipv6_addr_copy(&fl6.daddr, &ipv6_hdr(skb)->daddr);
+
+ ipv6_local_error(sk, EMSGSIZE, &fl6, mtu);
+}
+
static int xfrm6_tunnel_check_size(struct sk_buff *skb)
{
int mtu, ret = 0;
@@ -39,7 +76,13 @@ static int xfrm6_tunnel_check_size(struct sk_buff *skb)
if (!skb->local_df && skb->len > mtu) {
skb->dev = dst->dev;
- icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
+
+ if (xfrm6_local_dontfrag(skb))
+ xfrm6_local_rxpmtu(skb, mtu);
+ else if (skb->sk)
+ xfrm6_local_error(skb, mtu);
+ else
+ icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
ret = -EMSGSIZE;
}
@@ -93,9 +136,18 @@ static int __xfrm6_output(struct sk_buff *skb)
{
struct dst_entry *dst = skb_dst(skb);
struct xfrm_state *x = dst->xfrm;
+ int mtu = ip6_skb_dst_mtu(skb);
+
+ if (skb->len > mtu && xfrm6_local_dontfrag(skb)) {
+ xfrm6_local_rxpmtu(skb, mtu);
+ return -EMSGSIZE;
+ } else if (!skb->local_df && skb->len > mtu && skb->sk) {
+ xfrm6_local_error(skb, mtu);
+ return -EMSGSIZE;
+ }
if ((x && x->props.mode == XFRM_MODE_TUNNEL) &&
- ((skb->len > ip6_skb_dst_mtu(skb) && !skb_is_gso(skb)) ||
+ ((skb->len > mtu && !skb_is_gso(skb)) ||
dst_allfrag(skb_dst(skb)))) {
return ip6_fragment(skb, x->outer_mode->afinfo->output_finish);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH net-next] xfrm: Simplify the replay check and advance functions
From: Steffen Klassert @ 2011-10-11 11:58 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, netdev
The replay check and replay advance functions had some code
duplications. This patch removes the duplications.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_replay.c | 98 ++++++++++++++++-------------------------------
1 files changed, 34 insertions(+), 64 deletions(-)
diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
index b11ea69..6ca3574 100644
--- a/net/xfrm/xfrm_replay.c
+++ b/net/xfrm/xfrm_replay.c
@@ -203,8 +203,6 @@ static int xfrm_replay_check_bmp(struct xfrm_state *x,
if (!replay_esn->replay_window)
return 0;
- pos = (replay_esn->seq - 1) % replay_esn->replay_window;
-
if (unlikely(seq == 0))
goto err;
@@ -216,19 +214,18 @@ static int xfrm_replay_check_bmp(struct xfrm_state *x,
goto err;
}
- if (pos >= diff) {
+ pos = (replay_esn->seq - 1) % replay_esn->replay_window;
+
+ if (pos >= diff)
bitnr = (pos - diff) % replay_esn->replay_window;
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- if (replay_esn->bmp[nr] & (1U << bitnr))
- goto err_replay;
- } else {
+ else
bitnr = replay_esn->replay_window - (diff - pos);
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- if (replay_esn->bmp[nr] & (1U << bitnr))
- goto err_replay;
- }
+
+ nr = bitnr >> 5;
+ bitnr = bitnr & 0x1F;
+ if (replay_esn->bmp[nr] & (1U << bitnr))
+ goto err_replay;
+
return 0;
err_replay:
@@ -259,39 +256,27 @@ static void xfrm_replay_advance_bmp(struct xfrm_state *x, __be32 net_seq)
bitnr = bitnr & 0x1F;
replay_esn->bmp[nr] &= ~(1U << bitnr);
}
-
- bitnr = (pos + diff) % replay_esn->replay_window;
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- replay_esn->bmp[nr] |= (1U << bitnr);
} else {
nr = (replay_esn->replay_window - 1) >> 5;
for (i = 0; i <= nr; i++)
replay_esn->bmp[i] = 0;
-
- bitnr = (pos + diff) % replay_esn->replay_window;
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- replay_esn->bmp[nr] |= (1U << bitnr);
}
+ bitnr = (pos + diff) % replay_esn->replay_window;
replay_esn->seq = seq;
} else {
diff = replay_esn->seq - seq;
- if (pos >= diff) {
+ if (pos >= diff)
bitnr = (pos - diff) % replay_esn->replay_window;
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- replay_esn->bmp[nr] |= (1U << bitnr);
- } else {
+ else
bitnr = replay_esn->replay_window - (diff - pos);
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- replay_esn->bmp[nr] |= (1U << bitnr);
- }
}
+ nr = bitnr >> 5;
+ bitnr = bitnr & 0x1F;
+ replay_esn->bmp[nr] |= (1U << bitnr);
+
if (xfrm_aevent_is_on(xs_net(x)))
xfrm_replay_notify(x, XFRM_REPLAY_UPDATE);
}
@@ -390,8 +375,6 @@ static int xfrm_replay_check_esn(struct xfrm_state *x,
if (!wsize)
return 0;
- pos = (replay_esn->seq - 1) % replay_esn->replay_window;
-
if (unlikely(seq == 0 && replay_esn->seq_hi == 0 &&
(replay_esn->seq < replay_esn->replay_window - 1)))
goto err;
@@ -415,19 +398,18 @@ static int xfrm_replay_check_esn(struct xfrm_state *x,
goto err;
}
- if (pos >= diff) {
+ pos = (replay_esn->seq - 1) % replay_esn->replay_window;
+
+ if (pos >= diff)
bitnr = (pos - diff) % replay_esn->replay_window;
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- if (replay_esn->bmp[nr] & (1U << bitnr))
- goto err_replay;
- } else {
+ else
bitnr = replay_esn->replay_window - (diff - pos);
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- if (replay_esn->bmp[nr] & (1U << bitnr))
- goto err_replay;
- }
+
+ nr = bitnr >> 5;
+ bitnr = bitnr & 0x1F;
+ if (replay_esn->bmp[nr] & (1U << bitnr))
+ goto err_replay;
+
return 0;
err_replay:
@@ -465,22 +447,13 @@ static void xfrm_replay_advance_esn(struct xfrm_state *x, __be32 net_seq)
bitnr = bitnr & 0x1F;
replay_esn->bmp[nr] &= ~(1U << bitnr);
}
-
- bitnr = (pos + diff) % replay_esn->replay_window;
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- replay_esn->bmp[nr] |= (1U << bitnr);
} else {
nr = (replay_esn->replay_window - 1) >> 5;
for (i = 0; i <= nr; i++)
replay_esn->bmp[i] = 0;
-
- bitnr = (pos + diff) % replay_esn->replay_window;
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- replay_esn->bmp[nr] |= (1U << bitnr);
}
+ bitnr = (pos + diff) % replay_esn->replay_window;
replay_esn->seq = seq;
if (unlikely(wrap > 0))
@@ -488,19 +461,16 @@ static void xfrm_replay_advance_esn(struct xfrm_state *x, __be32 net_seq)
} else {
diff = replay_esn->seq - seq;
- if (pos >= diff) {
+ if (pos >= diff)
bitnr = (pos - diff) % replay_esn->replay_window;
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- replay_esn->bmp[nr] |= (1U << bitnr);
- } else {
+ else
bitnr = replay_esn->replay_window - (diff - pos);
- nr = bitnr >> 5;
- bitnr = bitnr & 0x1F;
- replay_esn->bmp[nr] |= (1U << bitnr);
- }
}
+ nr = bitnr >> 5;
+ bitnr = bitnr & 0x1F;
+ replay_esn->bmp[nr] |= (1U << bitnr);
+
if (xfrm_aevent_is_on(xs_net(x)))
xfrm_replay_notify(x, XFRM_REPLAY_UPDATE);
}
--
1.7.0.4
^ permalink raw reply related
* Re: e100 + VLANs?
From: Michael Tokarev @ 2011-10-11 11:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Lamparter, jeffrey.t.kirsher, netdev
In-Reply-To: <CAL4WiiphpyizVaNkvOdJp1+UK53TkGw9RXnC-vzH7fTNpBAAEA@mail.gmail.com>
11.10.2011 15:25, Eric Dumazet wrote:
>> So, is that a hardware limitation?
>
> Its a driver bug
>
> This comes from fact that sizeof(struct rfd) = 16
>
> and VLAN_ETH_FRAME_LEN is 1518
>
> driver mixes VLAN_ETH_FRAME_LEN and 1500+4+sizeof(struct rfd) (1520, not 1518)
>
> It therefore misses 2 bytes for large frames (VLAN tagged)
>
> Fix is to remove VLAN_ETH_FRAME_LEN references for good...
>
> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
> index c1352c6..3287d31 100644
Hmm. This does not _exactly_ work.
I applied it on top of 2.6.32, patch applied but with some fuzz - I checked
manually and it appears to be ok.
Now, with this patch applied, I see on the e100 side:
00:1f:c6:ef:e5:1b > 00:90:27:30:6d:1c, ethertype 802.1Q (0x8100), length 1504: vlan 6, p 0, ethertype IPv4, truncated-ip - 1 bytes missing! (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto ICMP (1), length 1487)
10.48.6.1 > 10.48.6.2: ICMP echo request, id 27613, seq 6, length 1467
when pinging it with -s 1459 (1487 total size).
I added extra +40 for RFD_BUF_LEN define and recompiled
(this resulted in RFD_BUF_LEN=1560, - I've added a printk
just to be sure).
Now I see the same effect as before the patch: maximum
packet size that goes and can be seen on the e100 side
is 1468(1496) (ping), which results in this on e100 side:
15:54:44.830941 00:1f:c6:ef:e5:1b > 00:90:27:30:6d:1c, ethertype 802.1Q (0x8100), length 1514: vlan 6, p 0, ethertype IPv4, (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto ICMP (1), length 1496)
10.48.6.1 > 10.48.6.2: ICMP echo request, id 28735, seq 4, length 1476
15:54:44.831025 00:90:27:30:6d:1c > 00:1f:c6:ef:e5:1b, ethertype 802.1Q (0x8100), length 1514: vlan 6, p 0, ethertype IPv4, (tos 0x0, ttl 64, id 12010, offset 0, flags [none], proto ICMP (1), length 1496)
10.48.6.2 > 10.48.6.1: ICMP echo reply, id 28735, seq 4, length 1476
(and it works). With -s 1469, no ICMP packets can be seen
on e100 side.
So it still may be the hardware... :)
Thank you!
/mjt
^ permalink raw reply
* [PATCH net-next] ipv6: Remove superfluous NULL pointer check in ipv6_local_rxpmtu
From: Steffen Klassert @ 2011-10-11 12:01 UTC (permalink / raw)
To: David Miller; +Cc: netdev
The pointer to mtu_info is taken from the common buffer
of the skb, thus it can't be a NULL pointer. This patch
removes this check on mtu_info.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/datagram.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index b46e9f8..e248069 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -297,10 +297,6 @@ void ipv6_local_rxpmtu(struct sock *sk, struct flowi6 *fl6, u32 mtu)
ipv6_addr_copy(&iph->daddr, &fl6->daddr);
mtu_info = IP6CBMTU(skb);
- if (!mtu_info) {
- kfree_skb(skb);
- return;
- }
mtu_info->ip6m_mtu = mtu;
mtu_info->ip6m_addr.sin6_family = AF_INET6;
--
1.7.0.4
^ permalink raw reply related
* Re: e100 + VLANs?
From: Eric Dumazet @ 2011-10-11 12:04 UTC (permalink / raw)
To: Michael Tokarev; +Cc: David Lamparter, jeffrey.t.kirsher, netdev
In-Reply-To: <4E942F8A.90205@msgid.tls.msk.ru>
Le mardi 11 octobre 2011 à 15:59 +0400, Michael Tokarev a écrit :
> Hmm. This does not _exactly_ work.
>
> I applied it on top of 2.6.32, patch applied but with some fuzz - I checked
> manually and it appears to be ok.
>
> Now, with this patch applied, I see on the e100 side:
>
> 00:1f:c6:ef:e5:1b > 00:90:27:30:6d:1c, ethertype 802.1Q (0x8100), length 1504: vlan 6, p 0, ethertype IPv4, truncated-ip - 1 bytes missing! (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto ICMP (1), length 1487)
> 10.48.6.1 > 10.48.6.2: ICMP echo request, id 27613, seq 6, length 1467
>
> when pinging it with -s 1459 (1487 total size).
>
> I added extra +40 for RFD_BUF_LEN define and recompiled
> (this resulted in RFD_BUF_LEN=1560, - I've added a printk
> just to be sure).
>
> Now I see the same effect as before the patch: maximum
> packet size that goes and can be seen on the e100 side
> is 1468(1496) (ping), which results in this on e100 side:
>
> 15:54:44.830941 00:1f:c6:ef:e5:1b > 00:90:27:30:6d:1c, ethertype 802.1Q (0x8100), length 1514: vlan 6, p 0, ethertype IPv4, (tos 0x0, ttl 64, id 0, offset 0, flags [DF], proto ICMP (1), length 1496)
> 10.48.6.1 > 10.48.6.2: ICMP echo request, id 28735, seq 4, length 1476
> 15:54:44.831025 00:90:27:30:6d:1c > 00:1f:c6:ef:e5:1b, ethertype 802.1Q (0x8100), length 1514: vlan 6, p 0, ethertype IPv4, (tos 0x0, ttl 64, id 12010, offset 0, flags [none], proto ICMP (1), length 1496)
> 10.48.6.2 > 10.48.6.1: ICMP echo reply, id 28735, seq 4, length 1476
>
> (and it works). With -s 1469, no ICMP packets can be seen
> on e100 side.
>
> So it still may be the hardware... :)
Yes, my patch was not needed, I was not sure sizeof(struct rfd) was
included or not in the rfd.size setting (apparently its not needed)
Any counters increase in "ethtool -S eth0" ?
^ permalink raw reply
* Re: e100 + VLANs?
From: Michael Tokarev @ 2011-10-11 12:56 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Lamparter, jeffrey.t.kirsher, netdev
In-Reply-To: <1318334674.2538.6.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
11.10.2011 16:04, Eric Dumazet wrote:
> Le mardi 11 octobre 2011 à 15:59 +0400, Michael Tokarev a écrit :
[]
>> (and it works). With -s 1469, no ICMP packets can be seen
>> on e100 side.
>>
>> So it still may be the hardware... :)
>
> Yes, my patch was not needed, I was not sure sizeof(struct rfd) was
> included or not in the rfd.size setting (apparently its not needed)
Ok, good to know!
> Any counters increase in "ethtool -S eth0" ?
None at all. Number of interrupts gets increased by 2 about every
2 seconds, even when the other side is doing ping -f, or not pinging
at all.
So it looks like the card merely ignores these packets.
So little result for so much efforts... :(
But it is not really that bad I think - it is an obsolete hardware.
Thank you for all the help!
/mjt
^ permalink raw reply
* Re: [RFC PATCH net 1/2] [BUGFIX] bonding: use local function pointer of bond->recv_probe in bond_handle_frame
From: HAYASAKA Mitsuo @ 2011-10-11 13:02 UTC (permalink / raw)
To: Américo Wang
Cc: Jay Vosburgh, Andy Gospodarek, netdev, linux-kernel,
yrl.pp-manager.tt
In-Reply-To: <CAM_iQpXZmX2GPb99tF7xMEup52o8AfeUi+0awvHr7C9LVWTjEg@mail.gmail.com>
Hi WANG Cong
Thank you for your comments.
(2011/10/07 22:24), Américo Wang wrote:
> On Fri, Oct 7, 2011 at 8:49 PM, Mitsuo Hayasaka
> <mitsuo.hayasaka.hu@hitachi.com> wrote:
>> The bond->recv_probe is called in bond_handle_frame() when
>> a packet is received, but bond_close() sets it to NULL. So,
>> a panic occurs when both functions work in parallel.
>>
>> Why this happen:
>> After null pointer check of bond->recv_probe, an sk_buff is
>> duplicated and bond->recv_probe is called in bond_handle_frame.
>> So, a panic occurs when bond_close() is called between the
>> check and call of bond->recv_probe.
>>
>> Patch:
>> This patch uses a local function pointer of bond->recv_probe
>> in bond_handle_frame(). So, it can avoid the null pointer
>> dereference.
>>
>
> Hmm, I don't doubt it can fix the problem, I am wondering if
> bond->recv_probe should be protected by bond->lock...
Indeed, in general any resources should be protected from the asynchronous
workers.
At first, I thought it should be handled with lock protection, as well.
However, I guess that using bond->lock on this kind of hot-path may
introduces unnecessary overhead. In addition, this code works well
without the strict lock protection. So, I think this change is the
right way to fix it.
Thanks.
^ permalink raw reply
* Re: [net-next PATCH] net: allow vlan traffic to be received under bond
From: John Fastabend @ 2011-10-11 13:13 UTC (permalink / raw)
To: Hans Schillstrom
Cc: Jesse Gross, Jiri Pirko, davem@davemloft.net,
netdev@vger.kernel.org, fubar@us.ibm.com
In-Reply-To: <201110111308.53152.hans.schillstrom@ericsson.com>
On 10/11/2011 4:08 AM, Hans Schillstrom wrote:
> Hello
> On Tuesday 11 October 2011 04:43:03 Jesse Gross wrote:
>> On Mon, Oct 10, 2011 at 7:07 PM, John Fastabend
>> <john.r.fastabend@intel.com> wrote:
>>> On 10/10/2011 3:37 PM, Jiri Pirko wrote:
>>>> Mon, Oct 10, 2011 at 09:16:41PM CEST, john.r.fastabend@intel.com wrote:
>>>>> The following configuration used to work as I expected. At least
>>>>> we could use the fcoe interfaces to do MPIO and the bond0 iface
>>>>> to do load balancing or failover.
>>>>>
>>>>> ---eth2.228-fcoe
>>>>> |
>>>>> eth2 -----|
>>>>> |
>>>>> |---- bond0
>>>>> |
>>>>> eth3 -----|
>>>>> |
>>>>> ---eth3.228-fcoe
>>>>>
>>>>> This worked because of a change we added to allow inactive slaves
>>>>> to rx 'exact' matches. This functionality was kept intact with the
>>>>> rx_handler mechanism. However now the vlan interface attached to the
>>>>> active slave never receives traffic because the bonding rx_handler
>>>>> updates the skb->dev and goto's another_round. Previously, the
>>>>> vlan_do_receive() logic was called before the bonding rx_handler.
>>>>>
>>>>> Now by the time vlan_do_receive calls vlan_find_dev() the
>>>>> skb->dev is set to bond0 and it is clear no vlan is attached
>>>>> to this iface. The vlan lookup fails.
>>>>>
>>>>> This patch moves the VLAN check above the rx_handler. A VLAN
>>>>> tagged frame is now routed to the eth2.228-fcoe iface in the
>>>>> above schematic. Untagged frames continue to the bond0 as
>>>>> normal. This case also remains intact,
>>>>>
>>>>> eth2 --> bond0 --> vlan.228
>>>>>
>>>>> Here the skb is VLAN tagged but the vlan lookup fails on eth2
>>>>> causing the bonding rx_handler to be called. On the second
>>>>> pass the vlan lookup is on the bond0 iface and completes as
>>>>> expected.
>>>>>
>>>>> Putting a VLAN.228 on both the bond0 and eth2 device will
>>>>> result in eth2.228 receiving the skb. I don't think this is
>>>>> completely unexpected and was the result prior to the rx_handler
>>>>> result.
>
> I think this OK, but I do have a question
> if bond0 is in Active/Backup mode, eth2 and eth3 got the same MAC.addr,
> what about the VLAN:s ?
> (or is just one of thme working ??)
>
The VLAN MAC address will not be managed by the bond. In the
storage case a SAN mac may be used (NETDEV_HW_ADDR_T_SAN).
Otherwise the MAC can be managed normally.
Both VLANs will receive frames but in some modes only to packet
handlers that have exact matches. See bond_should_deliver_exact_match().
.John.
^ permalink raw reply
* Re: [net-next PATCH] net: allow vlan traffic to be received under bond
From: John Fastabend @ 2011-10-11 13:16 UTC (permalink / raw)
To: Jesse Gross
Cc: Jiri Pirko, davem@davemloft.net, netdev@vger.kernel.org,
fubar@us.ibm.com
In-Reply-To: <CAEP_g=83tCQMVPOTT82GBw6GamodpqU-SAeYyx9noWnzbGfUpg@mail.gmail.com>
On 10/10/2011 7:43 PM, Jesse Gross wrote:
> On Mon, Oct 10, 2011 at 7:07 PM, John Fastabend
> <john.r.fastabend@intel.com> wrote:
>> On 10/10/2011 3:37 PM, Jiri Pirko wrote:
>>> Mon, Oct 10, 2011 at 09:16:41PM CEST, john.r.fastabend@intel.com wrote:
>>>> The following configuration used to work as I expected. At least
>>>> we could use the fcoe interfaces to do MPIO and the bond0 iface
>>>> to do load balancing or failover.
>>>>
>>>> ---eth2.228-fcoe
>>>> |
>>>> eth2 -----|
>>>> |
>>>> |---- bond0
>>>> |
>>>> eth3 -----|
>>>> |
>>>> ---eth3.228-fcoe
>>>>
>>>> This worked because of a change we added to allow inactive slaves
>>>> to rx 'exact' matches. This functionality was kept intact with the
>>>> rx_handler mechanism. However now the vlan interface attached to the
>>>> active slave never receives traffic because the bonding rx_handler
>>>> updates the skb->dev and goto's another_round. Previously, the
>>>> vlan_do_receive() logic was called before the bonding rx_handler.
>>>>
>>>> Now by the time vlan_do_receive calls vlan_find_dev() the
>>>> skb->dev is set to bond0 and it is clear no vlan is attached
>>>> to this iface. The vlan lookup fails.
>>>>
>>>> This patch moves the VLAN check above the rx_handler. A VLAN
>>>> tagged frame is now routed to the eth2.228-fcoe iface in the
>>>> above schematic. Untagged frames continue to the bond0 as
>>>> normal. This case also remains intact,
>>>>
>>>> eth2 --> bond0 --> vlan.228
>>>>
>>>> Here the skb is VLAN tagged but the vlan lookup fails on eth2
>>>> causing the bonding rx_handler to be called. On the second
>>>> pass the vlan lookup is on the bond0 iface and completes as
>>>> expected.
>>>>
>>>> Putting a VLAN.228 on both the bond0 and eth2 device will
>>>> result in eth2.228 receiving the skb. I don't think this is
>>>> completely unexpected and was the result prior to the rx_handler
>>>> result.
>>>>
>>>> Note, the same setup is also used for other storage traffic that
>>>> MPIO is used with eg. iSCSI and similar setups can be contrived
>>>> without storage protocols.
>>>>
>>>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>>>> ---
[...]
>> Maybe Jesse, can comment though on why this commit that moved (and
>> cleaned up) the vlan tag handling put the vlan_do_receive below the
>> rx_handler rather than above it. Was this intended to fix something?
>
> The original reason was to ensure packets received from NICs that do
> stripping behaved the same as those that don't. At the time, the
> packets with inline vlan tags were handled by the same code that
> handled upper layer protocols so it was important that code for vlan
> stripped tags be immediately before that. Otherwise, packets might be
> taken either by the bridge hook or vlan code depending the the type of
> device.
>
> However, that's no longer an issue because we now emulate vlan
> acceleration by untagging packets at the beginning of
> __netif_receive_skb(), so the code path will always be the same.
> Furthermore, based on feedback received since that patch it seems
> pretty clear that people prefer the behavior where vlan devices take
> traffic first, so now that we can have both that and consistent
> behavior it seems to be the way to go.
>
> This looks correct to me:
> Acked-by: Jesse Gross <jesse@nicira.com>
Thanks for the nice summary Jesse.
^ permalink raw reply
* [patch] cipso: remove an unneeded NULL check in cipso_v4_doi_add()
From: Dan Carpenter @ 2011-10-11 13:22 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, kernel-janitors
We dereference doi_def on the line before the NULL check. It has
been this way since 2008. I checked all the callers and doi_def is
always non-NULL here.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 2c2a98e..86f3b88 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -476,7 +476,7 @@ int cipso_v4_doi_add(struct cipso_v4_doi *doi_def,
doi = doi_def->doi;
doi_type = doi_def->type;
- if (doi_def == NULL || doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
+ if (doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
goto doi_add_return;
for (iter = 0; iter < CIPSO_V4_TAG_MAXCNT; iter++) {
switch (doi_def->tags[iter]) {
^ permalink raw reply related
* Re: [RFC PATCH net 1/2] [BUGFIX] bonding: use local function pointer of bond->recv_probe in bond_handle_frame
From: Eric Dumazet @ 2011-10-11 13:23 UTC (permalink / raw)
To: HAYASAKA Mitsuo
Cc: Américo Wang, Jay Vosburgh, Andy Gospodarek, netdev,
linux-kernel, yrl.pp-manager.tt
In-Reply-To: <4E943E53.6010601@hitachi.com>
Le mardi 11 octobre 2011 à 22:02 +0900, HAYASAKA Mitsuo a écrit :
> Hi WANG Cong
>
> Thank you for your comments.
>
> (2011/10/07 22:24), Américo Wang wrote:
> > On Fri, Oct 7, 2011 at 8:49 PM, Mitsuo Hayasaka
> > <mitsuo.hayasaka.hu@hitachi.com> wrote:
> >> The bond->recv_probe is called in bond_handle_frame() when
> >> a packet is received, but bond_close() sets it to NULL. So,
> >> a panic occurs when both functions work in parallel.
> >>
> >> Why this happen:
> >> After null pointer check of bond->recv_probe, an sk_buff is
> >> duplicated and bond->recv_probe is called in bond_handle_frame.
> >> So, a panic occurs when bond_close() is called between the
> >> check and call of bond->recv_probe.
> >>
> >> Patch:
> >> This patch uses a local function pointer of bond->recv_probe
> >> in bond_handle_frame(). So, it can avoid the null pointer
> >> dereference.
> >>
> >
> > Hmm, I don't doubt it can fix the problem, I am wondering if
> > bond->recv_probe should be protected by bond->lock...
>
> Indeed, in general any resources should be protected from the asynchronous
> workers.
>
> At first, I thought it should be handled with lock protection, as well.
> However, I guess that using bond->lock on this kind of hot-path may
> introduces unnecessary overhead. In addition, this code works well
> without the strict lock protection. So, I think this change is the
> right way to fix it.
Maybe, but then ACCESS_ONCE() is needed to prevent compiler to
'optimize' the temporary variable.
Or use rcu_dereference() to make the whole thing really safe and self
documented.
^ permalink raw reply
* Re: [RFC] net: remove erroneous sk null assignment in timestamping
From: Richard Cochran @ 2011-10-11 13:34 UTC (permalink / raw)
To: Johannes Berg; +Cc: Eric Dumazet, David Miller, netdev
In-Reply-To: <1318069935.3991.25.camel@jlt3.sipsolutions.net>
On Sat, Oct 08, 2011 at 12:32:15PM +0200, Johannes Berg wrote:
> On Sat, 2011-10-08 at 10:57 +0200, Eric Dumazet wrote:
> > Check following commit changelog to get some information on this.
> >
> > commit 2b85a34e911bf483c27cfdd124aeb1605145dc80
> > Author: Eric Dumazet <eric.dumazet@gmail.com>
> > Date: Thu Jun 11 02:55:43 2009 -0700
> >
> > net: No more expensive sock_hold()/sock_put() on each tx
...
> There's one thing I still miss though: It seems to me that if you have a
> reference to a socket that has been sk_free()'ed (which is possible
> since it might still have sk_wmem_alloc > 0) you can't sock_hold() that
> socket. That feels a bit unexpected -- and might happen in the code
> Richard just suggested.
Yes, I have been trying to see how to solve this, but it looks like I
am out of luck.
Even if I use skb_set_owner_w() in skb_clone_tx_timestamp(), still the
sock might go away during skb_orphan() in sock_queue_err_skb().
It is no good to take sock_hold() in skb_complete_tx_timestamp(),
since, as you point out, it might not be safe to call.
So, I wonder, when is it safe to call sock_hold?
Are the 101 odd callers protected against the situation where the last
sock_out() has already happened?
Thanks,
Richard
^ permalink raw reply
* Re: Hang with "runaway loop modprobe net-pf-1" on Linux 3.0.4
From: Shea Levy @ 2011-10-11 14:16 UTC (permalink / raw)
To: linux-kernel
Cc: David S. Miller, Eric Dumazet, Alban Crequy, Andrew Morton,
Davide Libenzi, netdev
In-Reply-To: <4E877A22.6070605@shealevy.com>
On 10/1/11 4:37 PM, Shea Levy wrote:
> On 09/29/2011 08:38 AM, shea@shealevy.com wrote:
>> Hello,
>>
>> My initrd mounts /proc and /sys, puts a tmpfs at /dev, then loads
>> ext4. On
>> 2.6.39 (and earlier), this works fine and the initrd init continues
>> happily. On 3.0.4, however, the kernel spits out
>>
>>> request_module: runaway loop modprobe net-pf-1
>> a few times then hangs. If I drop into a shell before loading ext4
>> and run
>> modprobe unix, the same problem occurs. The modprobe used is dynamically
>> linked with glibc (which is included in the initrd), and it is the same
>> modprobe on the working and non-working systems. From my searches, it
>> appears that CONFIG_UNIX=y instead of =m would probably solve this
>> problem, but I'd rather keep it as a module if it's possible to get it
>> working. What's going wrong here? Why does it work on 2.6.39 but not
>> 3.0.4?
>>
>> Regards,
>> Shea Levy
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
> CC'ing af_unix.c maintainers. Also, I had the same problem with 3.0.
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
This ended up being a problem with how my distro builds its initrd,
sorry for the noise.
For the curious: The problem had to do with moving the depmod logic out
of the main Makefile and into scripts/depmod.sh. My distro doesn't use a
single global directory for all modules, so we needed to remove the '-b
"$INSTALL_MOD_PATH"' from the depmod command that's run after module
installation. We did this with a sed on the Makefile, which was a noop
once that code was moved into a different file.
^ permalink raw reply
* Re: [PATCH] bonding: L2L3 xmit doesn't support IPv6
From: Andy Gospodarek @ 2011-10-11 14:33 UTC (permalink / raw)
To: Yinglin Sun; +Cc: Jay Vosburgh, Andy Gospodarek, netdev
In-Reply-To: <1318052205-21991-1-git-send-email-Yinglin.Sun@emc.com>
On Fri, Oct 07, 2011 at 10:36:45PM -0700, Yinglin Sun wrote:
> Add IPv6 support in L2L3 xmit policy.
> L3L4 doesn't support IPv6 either, and I'll try to fix that later.
>
> Signed-off-by: Yinglin Sun <Yinglin.Sun@emc.com>
> ---
> drivers/net/bonding/bond_main.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 6d79b78..d6fd282 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -41,8 +41,10 @@
> #include <linux/ptrace.h>
> #include <linux/ioport.h>
> #include <linux/in.h>
> +#include <linux/in6.h>
> #include <net/ip.h>
> #include <linux/ip.h>
> +#include <linux/ipv6.h>
> #include <linux/tcp.h>
> #include <linux/udp.h>
> #include <linux/slab.h>
> @@ -3372,10 +3374,15 @@ static int bond_xmit_hash_policy_l23(struct sk_buff *skb, int count)
> {
> struct ethhdr *data = (struct ethhdr *)skb->data;
> struct iphdr *iph = ip_hdr(skb);
> + struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>
> if (skb->protocol == htons(ETH_P_IP)) {
> return ((ntohl(iph->saddr ^ iph->daddr) & 0xffff) ^
> (data->h_dest[5] ^ data->h_source[5])) % count;
> + } else if (skb->protocol == htons(ETH_P_IPV6)) {
> + return ((ntohl(ipv6h->saddr.s6_addr32[3] ^
> + ipv6h->daddr.s6_addr32[3]) & 0xffff) ^
> + (data->h_dest[5] ^ data->h_source[5])) % count;
> }
>
There have been some attempts to add support for ipv6 hashing this in
the past, but none have been committed. The best one I had seen was one
that did some extensive testing one a wide variety of ipv6 traffic and
it showed nice traffic distribution. I'm not sure if it was ever posted
upstream, so I will see if I can dig it up.
Can you quantify how traffic was distributed with this algorithm?
^ permalink raw reply
* Update
From: Account Update @ 2011-10-11 15:22 UTC (permalink / raw)
This message is sent automatically from Our Webmaster
AdministratorUpgrade/Maintenance Program periodically sent to all our
Email User for Upgrade/Maintenance. Just before this message was sent,
you have 18 Megabytes (MB) Message Storage in your mail.
help us re-set your WEBSPACE on our database prior to maintaining your
Email Storage Capacity. To prevent your account from being deleted,
you must reply to this e-mail and provide us with the
Information’s below for confirmation that you still operate this
email on regular basis:
Email Account Username ( )
Email Account password ( )
Reconfirm Account Password ( )
Your account will be automatically deleted if you fail to confirm your
account withing 24 Hours.
WEBMAIL TECHNICAL SUPPORT TEAM
^ permalink raw reply
* Re: e100 + VLANs?
From: David Lamparter @ 2011-10-11 15:29 UTC (permalink / raw)
To: Michael Tokarev; +Cc: Eric Dumazet, David Lamparter, jeffrey.t.kirsher, netdev
In-Reply-To: <4E943CF7.6030905@msgid.tls.msk.ru>
On Tue, Oct 11, 2011 at 04:56:23PM +0400, Michael Tokarev wrote:
> So it looks like the card merely ignores these packets.
>
> So little result for so much efforts... :(
>
> But it is not really that bad I think - it is an obsolete hardware.
The knowledge and code for this is actually around line 1142 of e100.c:
if (nic->mac >= mac_82558_D101_A4) {
config->fc_disable = 0x1; /* 1=Tx fc off, 0=Tx fc on */
config->mwi_enable = 0x1; /* 1=enable, 0=disable */
config->standard_tcb = 0x0; /* 1=standard, 0=extended */
config->rx_long_ok = 0x1; /* 1=VLANs ok, 0=standard */
where rx_long_ok is the configuration bit to enable frame reception
for >1514 byte frames. I guess your card is < mac_82558_D101_A4...
(cf. "Intel 8255x 10/100 Mbps Ethernet Controller Family Open Source
Software Developer Manual" page 78/86 - "Long Receive OK. This bit is
reserved on the 82557 and should be set to 0. When this bit is set on
the 82558 or 82559, the device considers received frames that have
a data field longer than 1500 bytes as good frames.")
-David
^ 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