* [PATCH net-next 3/7] be2net: Fix ethtool self test for Lancer
From: Padmanabh Ratnakar @ 2012-04-25 11:46 UTC (permalink / raw)
To: netdev; +Cc: Padmanabh Ratnakar
Lancer does not support DDR self test. Fix ethtool self test by
skipping this test for Lancer.
Signed-off-by: Padmanabh Ratnakar <padmanabh.ratnakar@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_ethtool.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index dc9f74c..0c6f06e 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -793,7 +793,7 @@ be_self_test(struct net_device *netdev, struct ethtool_test *test, u64 *data)
}
}
- if (be_test_ddr_dma(adapter) != 0) {
+ if (!lancer_chip(adapter) && be_test_ddr_dma(adapter) != 0) {
data[3] = 1;
test->flags |= ETH_TEST_FL_FAILED;
}
--
1.6.0.2
^ permalink raw reply related
* [PATCH net-next 4/7] be2net: Fix traffic stall INTx mode
From: Padmanabh Ratnakar @ 2012-04-25 11:46 UTC (permalink / raw)
To: netdev; +Cc: Padmanabh Ratnakar
EQ is getting armed wrongly in INTx mode as INTx interrupt is taking
some time to deassert. This can cause another interrupt while NAPI is
scheduled and scheduling a NAPI in interrupt does not take effect.
This causes interrupt to be missed and traffic stalls. Fixing this by
preventing wrong arming of EQ.
Signed-off-by: Padmanabh Ratnakar <padmanabh.ratnakar@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index a9a11d4..9d42fab 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1571,7 +1571,9 @@ static int event_handle(struct be_eq_obj *eqo)
if (!num)
rearm = true;
- be_eq_notify(eqo->adapter, eqo->q.id, rearm, true, num);
+ if (num || msix_enabled(eqo->adapter))
+ be_eq_notify(eqo->adapter, eqo->q.id, rearm, true, num);
+
if (num)
napi_schedule(&eqo->napi);
--
1.6.0.2
^ permalink raw reply related
* [PATCH net-next 2/7] be2net: Fix FW download in Lancer
From: Padmanabh Ratnakar @ 2012-04-25 11:46 UTC (permalink / raw)
To: netdev; +Cc: Padmanabh Ratnakar
Increase time given by driver to adapter for completing FW download
to 30 seconds. Also return correct status when FW download times out.
Signed-off-by: Padmanabh Ratnakar <padmanabh.ratnakar@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_cmds.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 22be08c..3ba4aed 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -1824,18 +1824,16 @@ int lancer_cmd_write_object(struct be_adapter *adapter, struct be_dma_mem *cmd,
spin_unlock_bh(&adapter->mcc_lock);
if (!wait_for_completion_timeout(&adapter->flash_compl,
- msecs_to_jiffies(12000)))
+ msecs_to_jiffies(30000)))
status = -1;
else
status = adapter->flash_status;
resp = embedded_payload(wrb);
- if (!status) {
+ if (!status)
*data_written = le32_to_cpu(resp->actual_write_len);
- } else {
+ else
*addn_status = resp->additional_status;
- status = resp->status;
- }
return status;
--
1.6.0.2
^ permalink raw reply related
* [PATCH net-next 1/7] be2net: Fix VLAN/multicast packet reception
From: Padmanabh Ratnakar @ 2012-04-25 11:46 UTC (permalink / raw)
To: netdev; +Cc: Padmanabh Ratnakar
VLAN and multicast hardware filters are limited and can get
exhausted in adapters with many PCI functions. If setting
a VLAN or multicast filter fails due to lack of sufficient
hardware resources, these packets get dropped. Fix this by
switching to VLAN or multicast promiscous mode so that these
packets are not dropped.
Signed-off-by: Padmanabh Ratnakar <padmanabh.ratnakar@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 44 ++++++++++++++++++--------
1 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index a5bc608..a9a11d4 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -797,22 +797,30 @@ static int be_vid_config(struct be_adapter *adapter, bool vf, u32 vf_num)
if (adapter->promiscuous)
return 0;
- if (adapter->vlans_added <= adapter->max_vlans) {
- /* Construct VLAN Table to give to HW */
- for (i = 0; i < VLAN_N_VID; i++) {
- if (adapter->vlan_tag[i]) {
- vtag[ntags] = cpu_to_le16(i);
- ntags++;
- }
- }
- status = be_cmd_vlan_config(adapter, adapter->if_handle,
- vtag, ntags, 1, 0);
- } else {
- status = be_cmd_vlan_config(adapter, adapter->if_handle,
- NULL, 0, 1, 1);
+ if (adapter->vlans_added > adapter->max_vlans)
+ goto set_vlan_promisc;
+
+ /* Construct VLAN Table to give to HW */
+ for (i = 0; i < VLAN_N_VID; i++)
+ if (adapter->vlan_tag[i])
+ vtag[ntags++] = cpu_to_le16(i);
+
+ status = be_cmd_vlan_config(adapter, adapter->if_handle,
+ vtag, ntags, 1, 0);
+
+ /* Set to VLAN promisc mode as setting VLAN filter failed */
+ if (status) {
+ dev_info(&adapter->pdev->dev, "Exhausted VLAN HW filters.\n");
+ dev_info(&adapter->pdev->dev, "Disabling HW VLAN filtering.\n");
+ goto set_vlan_promisc;
}
return status;
+
+set_vlan_promisc:
+ status = be_cmd_vlan_config(adapter, adapter->if_handle,
+ NULL, 0, 1, 1);
+ return status;
}
static int be_vlan_add_vid(struct net_device *netdev, u16 vid)
@@ -862,6 +870,7 @@ ret:
static void be_set_rx_mode(struct net_device *netdev)
{
struct be_adapter *adapter = netdev_priv(netdev);
+ int status;
if (netdev->flags & IFF_PROMISC) {
be_cmd_rx_filter(adapter, IFF_PROMISC, ON);
@@ -908,7 +917,14 @@ static void be_set_rx_mode(struct net_device *netdev)
}
}
- be_cmd_rx_filter(adapter, IFF_MULTICAST, ON);
+ status = be_cmd_rx_filter(adapter, IFF_MULTICAST, ON);
+
+ /* Set to MCAST promisc mode if setting MULTICAST address fails */
+ if (status) {
+ dev_info(&adapter->pdev->dev, "Exhausted multicast HW filters.\n");
+ dev_info(&adapter->pdev->dev, "Disabling HW multicast filtering.\n");
+ be_cmd_rx_filter(adapter, IFF_ALLMULTI, ON);
+ }
done:
return;
}
--
1.6.0.2
^ permalink raw reply related
* [PATCH net-next 0/7] be2net fixes
From: Padmanabh Ratnakar @ 2012-04-25 11:45 UTC (permalink / raw)
To: netdev; +Cc: Padmanabh Ratnakar
Please apply.
Thanks,
Padmanabh
Padmanabh Ratnakar (7):
be2net: Fix VLAN/multicast packet reception
be2net: Fix FW download in Lancer
be2net: Fix ethtool self test for Lancer
be2net: Fix traffic stall INTx mode
be2net: Fix Lancer statistics
be2net: Fix wrong status getting returned for MCC commands
be2net: Fix FW download for BE
drivers/net/ethernet/emulex/benet/be.h | 5 +
drivers/net/ethernet/emulex/benet/be_cmds.c | 93 +++++++----
drivers/net/ethernet/emulex/benet/be_cmds.h | 8 +-
drivers/net/ethernet/emulex/benet/be_ethtool.c | 2 +-
drivers/net/ethernet/emulex/benet/be_hw.h | 74 ++++++---
drivers/net/ethernet/emulex/benet/be_main.c | 221 ++++++++++++++++--------
6 files changed, 276 insertions(+), 127 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-25 11:02 UTC (permalink / raw)
To: Tore Anderson; +Cc: Eric Dumazet, David Miller, netdev, Tom Herbert
In-Reply-To: <7333a1d306fa2eca215bc9f55d23d03c@greed.fud.no>
> I think you forgot to include the explanation why. :-)
I did try, I just didn't do a very good job.
> I suppose. This would be invisible to IPv6, though - the fragmentation and
> reassembly happens at a lower layer than IPv6. Same as ATM for example. Situation is
> described by RFC 2460:
It would be invisible (*), and you probably wouldn't really need the
frag header in the ipv6 packet,
but it would still be desirable to have ipv6 already have packets
smaller than ipv4
mtu - 20, rather than have to frag/unfrag at the tunnel endpoint.
Since it is always more efficient to have fragmented correctly in the
first place.
(*) Would it be legal for a tunnel endpoint to support ipv6 packets up
to 1280 bytes in size
but still send back a 'packet to big please use 1K mtu' message?
> «On any link that cannot convey a 1280-octet packet in one piece,
> link-specific fragmentation and reassembly must be provided at a layer below IPv6.»
True... I wonder how far we should bend over, just because it'll do
the work for us,
doesn't mean it isn't more efficient to do it ourselves...
Eh, not sure it's really worth the bother, I've never seen ipv6
tunneled over something with a small (<1280) but not tiny (>200) mtu.
>> (re: Eric's patch, I think it should protect itself against malicious
>> PMTU messages with too small MTUs, like 0 or 1 or 68 [not enough for
>> timestamped ipv6/tcp)
>
>
> Does this happen for IPv4, I wonder? IMHO, it makes sense to keep the the
> minimum
> PMTUDs allowed in sync. If PMTUD=1 is allowed in IPv4, and this is not
> problematic,
> I don't see why it couldn't be allowed in IPv6 either.
>
> Tore
>
>
--
Maciej A. Żenczykowski
Kernel Networking Developer @ Google
1600 Amphitheatre Parkway, Mountain View, CA 94043
tel: +1 (650) 253-0062
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Tore Anderson @ 2012-04-25 10:45 UTC (permalink / raw)
To: Maciej Żenczykowski; +Cc: Eric Dumazet, David Miller, netdev, Tom Herbert
In-Reply-To: <CANP3RGf5oTd9mQsXqKj3uXgweMwjs9N3VSeXBHjDXQ7vG0NaSg@mail.gmail.com>
* Maciej Żenczykowski
>> The sensible default would be either 1280 (and keep allfrag
>> feature), or
>> the minimum IPv4 PMTU currently enforced by the kernel + 20 bytes
>> (to
>> compensate for the larger IPv6 header size). I don't know what the
>> current
>> minimum PMTU is.
>
> I'd actually go with min IPv4 PMTU - 20, not + 20, see below for why.
I think you forgot to include the explanation why. :-)
> Hmm, it may be best to honour PMTUs below 1280 to some small but not
> too small value (512?), and below that give up, say mtu remains 1280
> but still add the frag header.
That is also possible, yes.
> Hmm, I thought one way to implement an IPv6 over IPv4 tunnel was to
> basically
> rely on IPv4 fragmentation, hence you would actually be using the
> same mechanism
> as for an IPv6-IPv4 translator, then you would be tunneling the IPv6
> packet over IPv4,
> so your IPv6 mtu would be 20 less than the v4 one, not 20 more which
> you get if you
> replace the v6 header with a smaller v4 one.
>
> Anyway, this certainly seems like an ipv6-in-ipv4 tunneling mechanism
> which would currently work, wouldn't it?
I suppose. This would be invisible to IPv6, though - the fragmentation
and reassembly
happens at a lower layer than IPv6. Same as ATM for example. Situation
is described
by RFC 2460:
«On any link that cannot convey a 1280-octet packet in one piece,
link-specific
fragmentation and reassembly must be provided at a layer below IPv6.»
> (re: Eric's patch, I think it should protect itself against malicious
> PMTU messages with too small MTUs, like 0 or 1 or 68 [not enough for
> timestamped ipv6/tcp)
Does this happen for IPv4, I wonder? IMHO, it makes sense to keep the
the minimum
PMTUDs allowed in sync. If PMTUD=1 is allowed in IPv4, and this is not
problematic,
I don't see why it couldn't be allowed in IPv6 either.
Tore
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Eric Dumazet @ 2012-04-25 10:44 UTC (permalink / raw)
To: Maciej Żenczykowski; +Cc: Tore Anderson, David Miller, netdev, Tom Herbert
In-Reply-To: <CANP3RGf5oTd9mQsXqKj3uXgweMwjs9N3VSeXBHjDXQ7vG0NaSg@mail.gmail.com>
On Wed, 2012-04-25 at 03:30 -0700, Maciej Żenczykowski wrote:
> Hmm, it may be best to honour PMTUs below 1280 to some small but not
> too small value (512?), and below that give up, say mtu remains 1280
> but still add the frag header.
>
Thats a good idea.
> (re: Eric's patch, I think it should protect itself against malicious
> PMTU messages with too small MTUs, like 0 or 1 or 68 [not enough for
> timestamped ipv6/tcp)
Yes, in fact we are going to keep the allfrag feature, and arm it if we
receive an mtu < 512.
I'll send a v3 patch soon.
Thanks
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-25 10:30 UTC (permalink / raw)
To: Tore Anderson; +Cc: Eric Dumazet, David Miller, netdev, Tom Herbert
In-Reply-To: <4c435f101fb7c653fd3b4e81980250e7@greed.fud.no>
> The sensible default would be either 1280 (and keep allfrag feature), or
> the minimum IPv4 PMTU currently enforced by the kernel + 20 bytes (to
> compensate for the larger IPv6 header size). I don't know what the current
> minimum PMTU is.
I'd actually go with min IPv4 PMTU - 20, not + 20, see below for why.
Hmm, it may be best to honour PMTUs below 1280 to some small but not
too small value (512?), and below that give up, say mtu remains 1280
but still add the frag header.
> Also, I'm not really sure if the IPv4 minimum PMTU is defined as 576 or
> 68 bytes. There are some conflicting information out there, and nothing
> really
> authoritative either way (that I've found at least).
Yeah, I've seen conflicting information.
I wonder if there's some tiny embedded devices out there with
miniscule mtus, because of tiny amounts of ram.
> Yup, only difference is that an IPv6 tunnel is guaranteed to have a MTU of
> 1280, so no need for allfrag or dropping PMTU below 1280. No such guarantees
> exist for IPv4 links.
Hmm, I thought one way to implement an IPv6 over IPv4 tunnel was to basically
rely on IPv4 fragmentation, hence you would actually be using the same mechanism
as for an IPv6-IPv4 translator, then you would be tunneling the IPv6
packet over IPv4,
so your IPv6 mtu would be 20 less than the v4 one, not 20 more which
you get if you
replace the v6 header with a smaller v4 one.
Anyway, this certainly seems like an ipv6-in-ipv4 tunneling mechanism
which would currently work, wouldn't it?
(re: Eric's patch, I think it should protect itself against malicious
PMTU messages with too small MTUs, like 0 or 1 or 68 [not enough for
timestamped ipv6/tcp)
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Eric Dumazet @ 2012-04-25 10:15 UTC (permalink / raw)
To: Tore Anderson; +Cc: Maciej Żenczykowski, David Miller, netdev, Tom Herbert
In-Reply-To: <4c435f101fb7c653fd3b4e81980250e7@greed.fud.no>
On Wed, 2012-04-25 at 12:04 +0200, Tore Anderson wrote:
>
> Also, I'm not really sure if the IPv4 minimum PMTU is defined as 576 or
> 68 bytes. There are some conflicting information out there, and nothing
> really
> authoritative either way (that I've found at least).
576 certainly not.
68 seems to be allowed
^ permalink raw reply
* Re: My e1000e GBE card is eating all port 623 pks
From: Joakim Tjernlund @ 2012-04-25 10:13 UTC (permalink / raw)
To: Brice Goglin; +Cc: Jesse Brandeburg, netdev
In-Reply-To: <4F97CA9B.3060105@ens-lyon.org>
Brice Goglin <Brice.Goglin@ens-lyon.org> wrote on 2012/04/25 11:57:47:
>
> On 25/04/2012 11:06, Joakim Tjernlund wrote:
> > Jesse Brandeburg<jesse.brandeburg@intel.com> wrote on 2012/04/24 23:51:28:
> >> On Sat, 21 Apr 2012 15:31:09 +0200
> >> Joakim Tjernlund<joakim.tjernlund@transmode.se> wrote:
> >>
> >>> Looks like port 623 is some mgmt protocol and our e1000e boards are eating these
> >>> pkgs and this trips NIS, finger and yptest hangs for a long time before timing out and
> >>> moving on.
> >>>
> >>> Is there a way to tell the network stack not to eat port 623 pkgs or
> >>> have NIS not to use port 623?
> >> I think you might be looking for something like portreserve
> >> (see man portreserve)
> > Seen portreserve on the web(gentoo does not have it) but it seems like a workaround.
> > Should not eating port 623 be something one should turn on? Now it is default on
> > and I cannot find a way to turn it off.
> >
>
> It might be related to IPMI (UDP/623 iirc). Try looking in your BIOS
> and/or network firmware config at boot, there might be things to disable
> there.
Been looking around but didn't find anything to turn this off, can you be more specific
where to look?
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Tore Anderson @ 2012-04-25 10:04 UTC (permalink / raw)
To: Maciej Żenczykowski; +Cc: Eric Dumazet, David Miller, netdev, Tom Herbert
In-Reply-To: <CANP3RGcGH6Su8Lvt34eTDF2iv2HSipo2DvybXGQA4pXZj_qb9Q@mail.gmail.com>
* Maciej Żenczykowski
>> That is a different issue entirely, but I don't disagree with you. A
>> "min_pmtu" sysctl or something like that would be useful.
>
> I don't really know what the default value should be? Something
> around 500?
> [to handle IPv4s min mtu of 576?]
The sensible default would be either 1280 (and keep allfrag feature),
or
the minimum IPv4 PMTU currently enforced by the kernel + 20 bytes (to
compensate for the larger IPv6 header size). I don't know what the
current
minimum PMTU is.
Also, I'm not really sure if the IPv4 minimum PMTU is defined as 576 or
68 bytes. There are some conflicting information out there, and nothing
really
authoritative either way (that I've found at least).
> Do we have any idea what values of small mtu actually show up in
> practice?
I have no data on this, I'm afraid. But I believe small MTUs (<1260,
which
currently triggers the need for allfrag), are very rare - at least
where I'm
from. Anectdotal, but - we've been running our corporate web site
IPv6-only
with IPv4 access through stateless translation on a Linux server with
the
buggy allfrag feature for several months, and there has been no
complaints.
>> However, the use case for the allfrag feature is not handling
>> tunnels,
>> but IPv4<->IPv6 translation. The issue is that a IPv6 host may very
>> well
>> receive an ICMPv6 Packet Too Big indicating a PMTU of <1280 that was
>> originally transmitted by an IPv4 router (as an ICMPv4 Need To
>> Fragment)
>> and underwent translation to IPv6.
>
> Very good point, although that's basically kind of like half a tunnel
> ;-)
Yup, only difference is that an IPv6 tunnel is guaranteed to have a MTU
of
1280, so no need for allfrag or dropping PMTU below 1280. No such
guarantees
exist for IPv4 links.
Tore
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Eric Dumazet @ 2012-04-25 10:02 UTC (permalink / raw)
To: Tore Anderson; +Cc: Maciej Żenczykowski, David Miller, netdev, Tom Herbert
In-Reply-To: <1335346710.3274.24.camel@edumazet-glaptop>
On Wed, 2012-04-25 at 11:38 +0200, Eric Dumazet wrote:
> Hmm, but what if we change linux to choice a) instead of b) ?
>
> That is, not cap mtu to minimum value 1280 (and not use anymore
> RTAX_FEATURE_ALLFRAG) : dst_allfrag() would be always false.
>
> In this case, do we still need to send the frag header ?
>
> I ask this because some TSO6 implementations probably dont cope very
> well with this added header (untested path)
>
So a patch against net-next would looks like :
(Incredible, we remove some code in linux ;) )
include/linux/rtnetlink.h | 2 +-
include/net/dst.h | 7 -------
include/net/inet_sock.h | 2 +-
net/ipv6/ip6_output.c | 14 +++++---------
net/ipv6/route.c | 28 ----------------------------
net/ipv6/xfrm6_output.c | 9 ++++-----
6 files changed, 11 insertions(+), 51 deletions(-)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 2c1de89..92ed273 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -380,7 +380,7 @@ enum {
#define RTAX_FEATURE_ECN 0x00000001
#define RTAX_FEATURE_SACK 0x00000002
#define RTAX_FEATURE_TIMESTAMP 0x00000004
-#define RTAX_FEATURE_ALLFRAG 0x00000008
+#define RTAX_FEATURE_ALLFRAG 0x00000008 /* not used anymore */
struct rta_session {
__u8 proto;
diff --git a/include/net/dst.h b/include/net/dst.h
index ff4da42..e69c55b 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -226,13 +226,6 @@ static inline void set_dst_metric_rtt(struct dst_entry *dst, int metric,
dst_metric_set(dst, metric, jiffies_to_msecs(rtt));
}
-static inline u32
-dst_allfrag(const struct dst_entry *dst)
-{
- int ret = dst_feature(dst, RTAX_FEATURE_ALLFRAG);
- return ret;
-}
-
static inline int
dst_metric_locked(const struct dst_entry *dst, int metric)
{
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index ae17e13..381ddc3 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -177,7 +177,7 @@ struct inet_sock {
};
#define IPCORK_OPT 1 /* ip-options has been held in ipcork.opt */
-#define IPCORK_ALLFRAG 2 /* always fragment (for ipv6 for now) */
+#define IPCORK_ALLFRAG 2 /* always fragment (for ipv6 for now), unused */
static inline struct inet_sock *inet_sk(const struct sock *sk)
{
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index b7ca461..314275e 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -152,11 +152,10 @@ static int ip6_finish_output2(struct sk_buff *skb)
static int ip6_finish_output(struct sk_buff *skb)
{
- if ((skb->len > ip6_skb_dst_mtu(skb) && !skb_is_gso(skb)) ||
- dst_allfrag(skb_dst(skb)))
+ if (skb->len > ip6_skb_dst_mtu(skb) && !skb_is_gso(skb))
return ip6_fragment(skb, ip6_finish_output2);
- else
- return ip6_finish_output2(skb);
+
+ return ip6_finish_output2(skb);
}
int ip6_output(struct sk_buff *skb)
@@ -1255,8 +1254,6 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
mtu = np->frag_size;
}
cork->fragsize = mtu;
- if (dst_allfrag(rt->dst.path))
- cork->flags |= IPCORK_ALLFRAG;
cork->length = 0;
sk->sk_sndmsg_page = NULL;
sk->sk_sndmsg_off = 0;
@@ -1335,7 +1332,7 @@ int ip6_append_data(struct sock *sk, int getfrag(void *from, char *to,
while (length > 0) {
/* Check if the remaining data fits into current packet. */
- copy = (cork->length <= mtu && !(cork->flags & IPCORK_ALLFRAG) ? mtu : maxfraglen) - skb->len;
+ copy = (cork->length <= mtu ? mtu : maxfraglen) - skb->len;
if (copy < length)
copy = maxfraglen - skb->len;
@@ -1360,7 +1357,7 @@ alloc_new_skb:
* we know we need more fragment(s).
*/
datalen = length + fraggap;
- if (datalen > (cork->length <= mtu && !(cork->flags & IPCORK_ALLFRAG) ? mtu : maxfraglen) - fragheaderlen)
+ if (datalen > (cork->length <= mtu ? mtu : maxfraglen) - fragheaderlen)
datalen = maxfraglen - fragheaderlen;
fraglen = datalen + fragheaderlen;
@@ -1550,7 +1547,6 @@ static void ip6_cork_release(struct inet_sock *inet, struct ipv6_pinfo *np)
if (inet->cork.base.dst) {
dst_release(inet->cork.base.dst);
inet->cork.base.dst = NULL;
- inet->cork.base.flags &= ~IPCORK_ALLFRAG;
}
memset(&inet->cork.fl, 0, sizeof(inet->cork.fl));
}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 0aefc36..43e2ec1 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1044,12 +1044,6 @@ static void ip6_rt_update_pmtu(struct dst_entry *dst, u32 mtu)
if (mtu < dst_mtu(dst) && rt6->rt6i_dst.plen == 128) {
rt6->rt6i_flags |= RTF_MODIFIED;
- if (mtu < IPV6_MIN_MTU) {
- u32 features = dst_metric(dst, RTAX_FEATURES);
- mtu = IPV6_MIN_MTU;
- features |= RTAX_FEATURE_ALLFRAG;
- dst_metric_set(dst, RTAX_FEATURES, features);
- }
dst_metric_set(dst, RTAX_MTU, mtu);
}
}
@@ -1707,7 +1701,6 @@ static void rt6_do_pmtu_disc(const struct in6_addr *daddr, const struct in6_addr
struct net *net, u32 pmtu, int ifindex)
{
struct rt6_info *rt, *nrt;
- int allfrag = 0;
again:
rt = rt6_lookup(net, daddr, saddr, ifindex, 0);
if (!rt)
@@ -1721,17 +1714,6 @@ again:
if (pmtu >= dst_mtu(&rt->dst))
goto out;
- if (pmtu < IPV6_MIN_MTU) {
- /*
- * According to RFC2460, PMTU is set to the IPv6 Minimum Link
- * MTU (1280) and a fragment header should always be included
- * after a node receiving Too Big message reporting PMTU is
- * less than the IPv6 Minimum Link MTU.
- */
- pmtu = IPV6_MIN_MTU;
- allfrag = 1;
- }
-
/* New mtu received -> path was valid.
They are sent only in response to data packets,
so that this nexthop apparently is reachable. --ANK
@@ -1745,11 +1727,6 @@ again:
*/
if (rt->rt6i_flags & RTF_CACHE) {
dst_metric_set(&rt->dst, RTAX_MTU, pmtu);
- if (allfrag) {
- u32 features = dst_metric(&rt->dst, RTAX_FEATURES);
- features |= RTAX_FEATURE_ALLFRAG;
- dst_metric_set(&rt->dst, RTAX_FEATURES, features);
- }
rt6_update_expires(rt, net->ipv6.sysctl.ip6_rt_mtu_expires);
rt->rt6i_flags |= RTF_MODIFIED;
goto out;
@@ -1767,11 +1744,6 @@ again:
if (nrt) {
dst_metric_set(&nrt->dst, RTAX_MTU, pmtu);
- if (allfrag) {
- u32 features = dst_metric(&nrt->dst, RTAX_FEATURES);
- features |= RTAX_FEATURE_ALLFRAG;
- dst_metric_set(&nrt->dst, RTAX_FEATURES, features);
- }
/* According to RFC 1981, detecting PMTU increase shouldn't be
* happened within 5 mins, the recommended timer is 10 mins.
diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
index 8755a30..8242af0 100644
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -146,11 +146,10 @@ static int __xfrm6_output(struct sk_buff *skb)
return -EMSGSIZE;
}
- if (x->props.mode == XFRM_MODE_TUNNEL &&
- ((skb->len > mtu && !skb_is_gso(skb)) ||
- dst_allfrag(skb_dst(skb)))) {
- return ip6_fragment(skb, x->outer_mode->afinfo->output_finish);
- }
+ if (x->props.mode == XFRM_MODE_TUNNEL && skb->len > mtu &&
+ !skb_is_gso(skb))
+ return ip6_fragment(skb, x->outer_mode->afinfo->output_finish);
+
return x->outer_mode->afinfo->output_finish(skb);
}
^ permalink raw reply related
* Re: My e1000e GBE card is eating all port 623 pks
From: Brice Goglin @ 2012-04-25 9:57 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: Jesse Brandeburg, netdev
In-Reply-To: <OF497D4E52.CC9EA3C0-ONC12579EB.0031DCB3-C12579EB.003213E4@transmode.se>
On 25/04/2012 11:06, Joakim Tjernlund wrote:
> Jesse Brandeburg<jesse.brandeburg@intel.com> wrote on 2012/04/24 23:51:28:
>> On Sat, 21 Apr 2012 15:31:09 +0200
>> Joakim Tjernlund<joakim.tjernlund@transmode.se> wrote:
>>
>>> Looks like port 623 is some mgmt protocol and our e1000e boards are eating these
>>> pkgs and this trips NIS, finger and yptest hangs for a long time before timing out and
>>> moving on.
>>>
>>> Is there a way to tell the network stack not to eat port 623 pkgs or
>>> have NIS not to use port 623?
>> I think you might be looking for something like portreserve
>> (see man portreserve)
> Seen portreserve on the web(gentoo does not have it) but it seems like a workaround.
> Should not eating port 623 be something one should turn on? Now it is default on
> and I cannot find a way to turn it off.
>
It might be related to IPMI (UDP/623 iirc). Try looking in your BIOS
and/or network firmware config at boot, there might be things to disable
there.
Brice
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-25 9:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Tore Anderson, David Miller, netdev, Tom Herbert
In-Reply-To: <1335346710.3274.24.camel@edumazet-glaptop>
> Hmm, but what if we change linux to choice a) instead of b) ?
>
> That is, not cap mtu to minimum value 1280 (and not use anymore
> RTAX_FEATURE_ALLFRAG) : dst_allfrag() would be always false.
>
> In this case, do we still need to send the frag header ?
Yeah, I was wondering about that myself.
By my reading of the relevant RFC it's not quite clear whether you
truly must include the frag header even if you choose to obey the
lower than 1280 mtu.
Although I don't see any reason why you would need to...
So long as there's a decent minimum pmtu we're willing to obey.
> I ask this because some TSO6 implementations probably dont cope very
> well with this added header (untested path)
Yes, I was thinking the same thing, hence why I mentioned GSO.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Tore Anderson @ 2012-04-25 9:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Maciej Żenczykowski, David Miller, netdev, Tom Herbert
In-Reply-To: <1335346710.3274.24.camel@edumazet-glaptop>
* Eric Dumazet
> Hmm, but what if we change linux to choice a) instead of b) ?
>
> That is, not cap mtu to minimum value 1280 (and not use anymore
> RTAX_FEATURE_ALLFRAG) : dst_allfrag() would be always false.
Yep.
> In this case, do we still need to send the frag header ?
No - the (translated) IPv4 packet should then fit onto the small-MTU
IPv4 link without any fragmentation (by the IPv4 router) required. So
you end up with perfectly regular end-to-end Path MTU Discovery.
Tore
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-25 9:48 UTC (permalink / raw)
To: Tore Anderson; +Cc: Eric Dumazet, David Miller, netdev, Tom Herbert
In-Reply-To: <f88609a3e80bbe53233e62dec2699a3e@greed.fud.no>
>> I get that we _choose_ to behave such, and I agree this adheres to
>> specs.
>
> "Chose" (past), not "choose" (present). ;-)
Details, but until we 'choose' to change it we continuously 'choose'
to have the current behaviour. ;-)
> This patch does not make this choice. This patch merely fixes a bug in
> the implementation of the choice that was made a long time ago.
yes, I wasn't saying this patch was bad, I was just wanting to point
that we should perhaps revisit the design choice.
>> But I'm not convinced that (even though this is allowed per RFC) this
>> is the right choice.
>
> That is a different issue entirely, but I don't disagree with you. A
> "min_pmtu" sysctl or something like that would be useful.
I don't really know what the default value should be? Something around 500?
[to handle IPv4s min mtu of 576?]
Do we have any idea what values of small mtu actually show up in practice?
> Actually, in IPv6, fragmentation *must* be performed by end hosts,
> routers (including tunnel end points) *cannot* fragment.
Yes, I miss-phrased that, that's what I meant.
> However, the use case for the allfrag feature is not handling tunnels,
> but IPv4<->IPv6 translation. The issue is that a IPv6 host may very well
> receive an ICMPv6 Packet Too Big indicating a PMTU of <1280 that was
> originally transmitted by an IPv4 router (as an ICMPv4 Need To Fragment)
> and underwent translation to IPv6.
Very good point, although that's basically kind of like half a tunnel ;-)
> In case you're interested, I have a slide deck below that explains the
> use case for IPv4<->IPv6 translation. Slide 25 is about the particular
> corner case where the allfrag feature is necessary. URL:
>
> http://fud.no/talks/20120417-RIPE64-The_Case_for_IPv6_Only_Data_Centres.pdf
Yes, I've seen this slide set a couple days ago - very good.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Tore Anderson @ 2012-04-25 9:20 UTC (permalink / raw)
To: Maciej Żenczykowski; +Cc: Eric Dumazet, David Miller, netdev, Tom Herbert
In-Reply-To: <CANP3RGdhs8s_RytR=f8ismSZdGs91bpVq=ZAjb0EOm-gCsDPAw@mail.gmail.com>
* Maciej Żenczykowski
>> But we chose to _not_ decrease mtu and adhere to the specs.
>
> I get that we _choose_ to behave such, and I agree this adheres to
> specs.
"Chose" (past), not "choose" (present). ;-)
This patch does not make this choice. This patch merely fixes a bug in
the implementation of the choice that was made a long time ago.
> But I'm not convinced that (even though this is allowed per RFC) this
> is the right choice.
That is a different issue entirely, but I don't disagree with you. A
"min_pmtu" sysctl or something like that would be useful.
> Also note that IPv6 prefers to see fragmentation happen at the end
> hosts, and not at the routers.
> Although of course it doesn't treat a tunnel end point as a router.
Actually, in IPv6, fragmentation *must* be performed by end hosts,
routers (including tunnel end points) *cannot* fragment.
However, the use case for the allfrag feature is not handling tunnels,
but IPv4<->IPv6 translation. The issue is that a IPv6 host may very
well
receive an ICMPv6 Packet Too Big indicating a PMTU of <1280 that was
originally transmitted by an IPv4 router (as an ICMPv4 Need To
Fragment)
and underwent translation to IPv6.
In this case, the IPv6 node does not need to reduce the PMTU to <1280
(Linux does not), but it is not invalid to have a <1280 MTU link in the
IPv4 internet either, so something else must be done for the
communication to work. The solution is then to include the IPv6
Fragment
extension header, so that the translator have a suitable Identification
value to copy into the translated IPv4 header, and may therefore clear
the Don't Fragment flag, so that the IPv4 router will fragment the
packet as it is forwarded onto the low-MTU link.
In case you're interested, I have a slide deck below that explains the
use case for IPv4<->IPv6 translation. Slide 25 is about the particular
corner case where the allfrag feature is necessary. URL:
http://fud.no/talks/20120417-RIPE64-The_Case_for_IPv6_Only_Data_Centres.pdf
Tore
^ permalink raw reply
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Eric Dumazet @ 2012-04-25 9:38 UTC (permalink / raw)
To: Tore Anderson; +Cc: Maciej Żenczykowski, David Miller, netdev, Tom Herbert
In-Reply-To: <f88609a3e80bbe53233e62dec2699a3e@greed.fud.no>
On Wed, 2012-04-25 at 11:20 +0200, Tore Anderson wrote:
> * Maciej Żenczykowski
>
> >> But we chose to _not_ decrease mtu and adhere to the specs.
> >
> > I get that we _choose_ to behave such, and I agree this adheres to
> > specs.
>
> "Chose" (past), not "choose" (present). ;-)
>
> This patch does not make this choice. This patch merely fixes a bug in
> the implementation of the choice that was made a long time ago.
>
> > But I'm not convinced that (even though this is allowed per RFC) this
> > is the right choice.
>
> That is a different issue entirely, but I don't disagree with you. A
> "min_pmtu" sysctl or something like that would be useful.
>
> > Also note that IPv6 prefers to see fragmentation happen at the end
> > hosts, and not at the routers.
> > Although of course it doesn't treat a tunnel end point as a router.
>
> Actually, in IPv6, fragmentation *must* be performed by end hosts,
> routers (including tunnel end points) *cannot* fragment.
>
> However, the use case for the allfrag feature is not handling tunnels,
> but IPv4<->IPv6 translation. The issue is that a IPv6 host may very
> well
> receive an ICMPv6 Packet Too Big indicating a PMTU of <1280 that was
> originally transmitted by an IPv4 router (as an ICMPv4 Need To
> Fragment)
> and underwent translation to IPv6.
>
> In this case, the IPv6 node does not need to reduce the PMTU to <1280
> (Linux does not), but it is not invalid to have a <1280 MTU link in the
> IPv4 internet either, so something else must be done for the
> communication to work. The solution is then to include the IPv6
> Fragment
> extension header, so that the translator have a suitable Identification
> value to copy into the translated IPv4 header, and may therefore clear
> the Don't Fragment flag, so that the IPv4 router will fragment the
> packet as it is forwarded onto the low-MTU link.
>
> In case you're interested, I have a slide deck below that explains the
> use case for IPv4<->IPv6 translation. Slide 25 is about the particular
> corner case where the allfrag feature is necessary. URL:
>
> http://fud.no/talks/20120417-RIPE64-The_Case_for_IPv6_Only_Data_Centres.pdf
Hmm, but what if we change linux to choice a) instead of b) ?
That is, not cap mtu to minimum value 1280 (and not use anymore
RTAX_FEATURE_ALLFRAG) : dst_allfrag() would be always false.
In this case, do we still need to send the frag header ?
I ask this because some TSO6 implementations probably dont cope very
well with this added header (untested path)
^ permalink raw reply
* Re: [PATCH 1/1] ipvs: kernel oops - do_ip_vs_get_ctl
From: Hans Schillstrom @ 2012-04-25 9:36 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: horms@verge.net.au, ja@ssi.bg, wensong@linux-vs.org,
lvs-devel@vger.kernel.org, netdev@vger.kernel.org,
netfilter-devel@vger.kernel.org, hans@schillstrom.com
In-Reply-To: <1335344761.6337.42.camel@t520>
On Wednesday 25 April 2012 11:06:01 Jesper Dangaard Brouer wrote:
> Hi Hans,
>
> Thank you for your work.
> Just some whitespace nitpicks below.
OK thanks I forgott to run checkpatch ...
I'll send a new patch
> I have not been able to reproduce the bug in:
> https://bugzilla.redhat.com/show_bug.cgi?id=806704
>
> Can you recommend a (better) way to reproduce this bug?
> I just want to be able to verify that this fixes the bug in question.
I think you need two threads or procs.
Start a modprobe in thread one then issue a ipvs ioctl in the other.
The timing is not that easy here :-)
>
>
> On Wed, 2012-04-25 at 09:44 +0200, Hans Schillstrom wrote:
> > Change order of init so netns init is ready
> > when register ioctl and netlink.
> >
> > Reported-by: "Ryan O'Hara" <rohara@redhat.com>
> > Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
> > ---
> > include/net/ip_vs.h | 2 +
> > net/netfilter/ipvs/ip_vs_core.c | 9 ++++++
> > net/netfilter/ipvs/ip_vs_ctl.c | 52 ++++++++++++++++++++++----------------
> > 3 files changed, 41 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> > index f967395..93b81aa 100644
> > --- a/include/net/ip_vs.h
> > +++ b/include/net/ip_vs.h
> > @@ -1201,6 +1201,8 @@ ip_vs_lookup_real_service(struct net *net, int af, __u16 protocol,
> >
> > extern int ip_vs_use_count_inc(void);
> > extern void ip_vs_use_count_dec(void);
> > +extern int ip_vs_register_nl_ioctl(void);
> > +extern void ip_vs_unregister_nl_ioctl(void);
> > extern int ip_vs_control_init(void);
> > extern void ip_vs_control_cleanup(void);
> > extern struct ip_vs_dest *
> > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> > index d8b1d30..c8f36b9 100644
> > --- a/net/netfilter/ipvs/ip_vs_core.c
> > +++ b/net/netfilter/ipvs/ip_vs_core.c
> > @@ -1995,10 +1995,18 @@ static int __init ip_vs_init(void)
> > goto cleanup_dev;
> > }
> >
> > + ret = ip_vs_register_nl_ioctl();
> > + if (ret < 0) {
> > + pr_err("can't register netlink/ioctl.\n");
> > + goto cleanup_hooks;
> > + }
> > +
> > pr_info("ipvs loaded.\n");
> >
> > return ret;
> >
> > +cleanup_hooks:
> > + nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
> > cleanup_dev:
> > unregister_pernet_device(&ipvs_core_dev_ops);
> > cleanup_sub:
> > @@ -2014,6 +2022,7 @@ exit:
> >
> > static void __exit ip_vs_cleanup(void)
> > {
> > + ip_vs_unregister_nl_ioctl();
> > nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
> > unregister_pernet_device(&ipvs_core_dev_ops);
> > unregister_pernet_subsys(&ipvs_core_ops); /* free ip_vs struct */
> > diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> > index 7131417..efaf484 100644
> > --- a/net/netfilter/ipvs/ip_vs_ctl.c
> > +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> > @@ -3750,21 +3750,10 @@ void __net_exit ip_vs_control_net_cleanup(struct net *net)
> > free_percpu(ipvs->tot_stats.cpustats);
> > }
> >
> > -int __init ip_vs_control_init(void)
> > +int ip_vs_register_nl_ioctl(void)
> > {
> > - int idx;
> > int ret;
> >
> > - EnterFunction(2);
> > -
> > - /* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
> > - for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
> > - INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> > - INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> > - }
> > -
> > - smp_wmb(); /* Do we really need it now ? */
> > -
> > ret = nf_register_sockopt(&ip_vs_sockopts);
> > if (ret) {
> > pr_err("cannot register sockopt.\n");
> > @@ -3776,28 +3765,47 @@ int __init ip_vs_control_init(void)
> > pr_err("cannot register Generic Netlink interface.\n");
> > goto err_genl;
> > }
> > -
> > - ret = register_netdevice_notifier(&ip_vs_dst_notifier);
> > - if (ret < 0)
> > - goto err_notf;
> > -
> > - LeaveFunction(2);
> > return 0;
> >
> > -err_notf:
> > - ip_vs_genl_unregister();
> > err_genl:
> > nf_unregister_sockopt(&ip_vs_sockopts);
> > err_sock:
> > return ret;
> > }
> >
> > +void ip_vs_unregister_nl_ioctl(void)
>
> There is a extra trailing whitespace behind
> ip_vs_unregister_nl_ioctl(void)
>
>
> > +{
> > + ip_vs_genl_unregister();
> > + nf_unregister_sockopt(&ip_vs_sockopts);
> > +}
> > +
> > +int __init ip_vs_control_init(void)
> > +{
> > + int idx;
> > + int ret;
> > +
> > + EnterFunction(2);
> > +
> > + /* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
> > + for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
>
> The for loop is funny spaced...
>
>
> > + INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> > + INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> > + }
> > +
> > + smp_wmb(); /* Do we really need it now ? */
> > +
> > + ret = register_netdevice_notifier(&ip_vs_dst_notifier);
> > + if (ret < 0)
> > + return ret;
> > +
> > + LeaveFunction(2);
> > + return 0;
> > +}
> > +
> >
> > void ip_vs_control_cleanup(void)
> > {
> > EnterFunction(2);
> > unregister_netdevice_notifier(&ip_vs_dst_notifier);
> > - ip_vs_genl_unregister();
> > - nf_unregister_sockopt(&ip_vs_sockopts);
> > LeaveFunction(2);
> > }
>
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply
* Re: [PATCH 2/2] ipvs: fix crash in ip_vs_control_net_cleanup on unload
From: Pablo Neira Ayuso @ 2012-04-25 9:15 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Sasha Levin, stable, Hans Schillstrom
In-Reply-To: <1335186732-16002-3-git-send-email-horms@verge.net.au>
On Mon, Apr 23, 2012 at 10:12:12PM +0900, Simon Horman wrote:
> From: Julian Anastasov <ja@ssi.bg>
>
> commit 14e405461e664b777e2a5636e10b2ebf36a686ec (2.6.39)
> ("Add __ip_vs_control_{init,cleanup}_sysctl()")
> introduced regression due to wrong __net_init for
> __ip_vs_control_cleanup_sysctl. This leads to crash when
> the ip_vs module is unloaded.
>
> Fix it by changing __net_init to __net_exit for
> the function that is already renamed to ip_vs_control_net_cleanup_sysctl.
>
> Cc: stable@vger.kernel.org
Removed this above.
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: Hans Schillstrom <hans@schillstrom.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
And applied, thanks Simon.
^ permalink raw reply
* Re: [PATCH 1/2] netfilter: ipvs: Verify that IP_VS protocol has been registered
From: Pablo Neira Ayuso @ 2012-04-25 9:14 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Sasha Levin
In-Reply-To: <1335186732-16002-2-git-send-email-horms@verge.net.au>
On Mon, Apr 23, 2012 at 10:12:11PM +0900, Simon Horman wrote:
> From: Sasha Levin <levinsasha928@gmail.com>
>
> The registration of a protocol might fail, there were no checks
> and all registrations were assumed to be correct. This lead to
> NULL ptr dereferences when apps tried registering.
>
> For example:
>
> [ 1293.226051] BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
> [ 1293.227038] IP: [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> [ 1293.227038] PGD 391de067 PUD 6c20b067 PMD 0
> [ 1293.227038] Oops: 0000 [#1] PREEMPT SMP
> [ 1293.227038] CPU 1
> [ 1293.227038] Pid: 19609, comm: trinity Tainted: G W 3.4.0-rc1-next-20120405-sasha-dirty #57
> [ 1293.227038] RIP: 0010:[<ffffffff822aacb0>] [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> [ 1293.227038] RSP: 0018:ffff880038c1dd18 EFLAGS: 00010286
> [ 1293.227038] RAX: ffffffffffffffc0 RBX: 0000000000001500 RCX: 0000000000010000
> [ 1293.227038] RDX: 0000000000000000 RSI: ffff88003a2d5888 RDI: 0000000000000282
> [ 1293.227038] RBP: ffff880038c1dd48 R08: 0000000000000000 R09: 0000000000000000
> [ 1293.227038] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88003a2d5668
> [ 1293.227038] R13: ffff88003a2d5988 R14: ffff8800696a8ff8 R15: 0000000000000000
> [ 1293.227038] FS: 00007f01930d9700(0000) GS:ffff88007ce00000(0000) knlGS:0000000000000000
> [ 1293.227038] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 1293.227038] CR2: 0000000000000018 CR3: 0000000065dfc000 CR4: 00000000000406e0
> [ 1293.227038] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 1293.227038] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 1293.227038] Process trinity (pid: 19609, threadinfo ffff880038c1c000, task ffff88002dc73000)
> [ 1293.227038] Stack:
> [ 1293.227038] ffff880038c1dd48 00000000fffffff4 ffff8800696aada0 ffff8800694f5580
> [ 1293.227038] ffffffff8369f1e0 0000000000001500 ffff880038c1dd98 ffffffff822a716b
> [ 1293.227038] 0000000000000000 ffff8800696a8ff8 0000000000000015 ffff8800694f5580
> [ 1293.227038] Call Trace:
> [ 1293.227038] [<ffffffff822a716b>] ip_vs_app_inc_new+0xdb/0x180
> [ 1293.227038] [<ffffffff822a7258>] register_ip_vs_app_inc+0x48/0x70
> [ 1293.227038] [<ffffffff822b2fea>] __ip_vs_ftp_init+0xba/0x140
> [ 1293.227038] [<ffffffff821c9060>] ops_init+0x80/0x90
> [ 1293.227038] [<ffffffff821c90cb>] setup_net+0x5b/0xe0
> [ 1293.227038] [<ffffffff821c9416>] copy_net_ns+0x76/0x100
> [ 1293.227038] [<ffffffff810dc92b>] create_new_namespaces+0xfb/0x190
> [ 1293.227038] [<ffffffff810dca21>] unshare_nsproxy_namespaces+0x61/0x80
> [ 1293.227038] [<ffffffff810afd1f>] sys_unshare+0xff/0x290
> [ 1293.227038] [<ffffffff8187622e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [ 1293.227038] [<ffffffff82665539>] system_call_fastpath+0x16/0x1b
> [ 1293.227038] Code: 89 c7 e8 34 91 3b 00 89 de 66 c1 ee 04 31 de 83 e6 0f 48 83 c6 22 48 c1 e6 04 4a 8b 14 26 49 8d 34 34 48 8d 42 c0 48 39 d6 74 13 <66> 39 58 58 74 22 48 8b 48 40 48 8d 41 c0 48 39 ce 75 ed 49 8d
> [ 1293.227038] RIP [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> [ 1293.227038] RSP <ffff880038c1dd18>
> [ 1293.227038] CR2: 0000000000000018
> [ 1293.379284] ---[ end trace 364ab40c7011a009 ]---
> [ 1293.381182] Kernel panic - not syncing: Fatal exception in interrupt
>
> Cc: stable@vger.kernel.org
I've removed this above from the patch. I prefer not to add confusing
information (this means you'll have to rebase, sorry).
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> Acked-by: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: Simon Horman <horms@verge.net.au>
Applied. Thanks.
BTW, did you consider using an array for ipvs->ip_vs_proto_table?
I think a hash table for only 5 protocols is overkill. I know, we'll
consume more memory but lookups will definitely be faster, but it will
consume a bit more memory (the space-time tradeoff again).
^ permalink raw reply
* Re: My e1000e GBE card is eating all port 623 pks
From: Joakim Tjernlund @ 2012-04-25 9:06 UTC (permalink / raw)
To: Jesse Brandeburg; +Cc: netdev
In-Reply-To: <20120424145128.000070a8@unknown>
Jesse Brandeburg <jesse.brandeburg@intel.com> wrote on 2012/04/24 23:51:28:
>
> On Sat, 21 Apr 2012 15:31:09 +0200
> Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
>
> > Looks like port 623 is some mgmt protocol and our e1000e boards are eating these
> > pkgs and this trips NIS, finger and yptest hangs for a long time before timing out and
> > moving on.
> >
> > Is there a way to tell the network stack not to eat port 623 pkgs or
> > have NIS not to use port 623?
>
> I think you might be looking for something like portreserve
> (see man portreserve)
Seen portreserve on the web(gentoo does not have it) but it seems like a workaround.
Should not eating port 623 be something one should turn on? Now it is default on
and I cannot find a way to turn it off.
Jocke
^ permalink raw reply
* Re: [PATCH 1/1] ipvs: kernel oops - do_ip_vs_get_ctl
From: Jesper Dangaard Brouer @ 2012-04-25 9:06 UTC (permalink / raw)
To: Hans Schillstrom
Cc: horms, ja, wensong, lvs-devel, netdev, netfilter-devel, hans
In-Reply-To: <1335339884-29825-1-git-send-email-hans.schillstrom@ericsson.com>
Hi Hans,
Thank you for your work.
Just some whitespace nitpicks below.
I have not been able to reproduce the bug in:
https://bugzilla.redhat.com/show_bug.cgi?id=806704
Can you recommend a (better) way to reproduce this bug?
I just want to be able to verify that this fixes the bug in question.
On Wed, 2012-04-25 at 09:44 +0200, Hans Schillstrom wrote:
> Change order of init so netns init is ready
> when register ioctl and netlink.
>
> Reported-by: "Ryan O'Hara" <rohara@redhat.com>
> Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
> ---
> include/net/ip_vs.h | 2 +
> net/netfilter/ipvs/ip_vs_core.c | 9 ++++++
> net/netfilter/ipvs/ip_vs_ctl.c | 52 ++++++++++++++++++++++----------------
> 3 files changed, 41 insertions(+), 22 deletions(-)
>
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index f967395..93b81aa 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -1201,6 +1201,8 @@ ip_vs_lookup_real_service(struct net *net, int af, __u16 protocol,
>
> extern int ip_vs_use_count_inc(void);
> extern void ip_vs_use_count_dec(void);
> +extern int ip_vs_register_nl_ioctl(void);
> +extern void ip_vs_unregister_nl_ioctl(void);
> extern int ip_vs_control_init(void);
> extern void ip_vs_control_cleanup(void);
> extern struct ip_vs_dest *
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index d8b1d30..c8f36b9 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1995,10 +1995,18 @@ static int __init ip_vs_init(void)
> goto cleanup_dev;
> }
>
> + ret = ip_vs_register_nl_ioctl();
> + if (ret < 0) {
> + pr_err("can't register netlink/ioctl.\n");
> + goto cleanup_hooks;
> + }
> +
> pr_info("ipvs loaded.\n");
>
> return ret;
>
> +cleanup_hooks:
> + nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
> cleanup_dev:
> unregister_pernet_device(&ipvs_core_dev_ops);
> cleanup_sub:
> @@ -2014,6 +2022,7 @@ exit:
>
> static void __exit ip_vs_cleanup(void)
> {
> + ip_vs_unregister_nl_ioctl();
> nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
> unregister_pernet_device(&ipvs_core_dev_ops);
> unregister_pernet_subsys(&ipvs_core_ops); /* free ip_vs struct */
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 7131417..efaf484 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3750,21 +3750,10 @@ void __net_exit ip_vs_control_net_cleanup(struct net *net)
> free_percpu(ipvs->tot_stats.cpustats);
> }
>
> -int __init ip_vs_control_init(void)
> +int ip_vs_register_nl_ioctl(void)
> {
> - int idx;
> int ret;
>
> - EnterFunction(2);
> -
> - /* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
> - for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
> - INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> - INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> - }
> -
> - smp_wmb(); /* Do we really need it now ? */
> -
> ret = nf_register_sockopt(&ip_vs_sockopts);
> if (ret) {
> pr_err("cannot register sockopt.\n");
> @@ -3776,28 +3765,47 @@ int __init ip_vs_control_init(void)
> pr_err("cannot register Generic Netlink interface.\n");
> goto err_genl;
> }
> -
> - ret = register_netdevice_notifier(&ip_vs_dst_notifier);
> - if (ret < 0)
> - goto err_notf;
> -
> - LeaveFunction(2);
> return 0;
>
> -err_notf:
> - ip_vs_genl_unregister();
> err_genl:
> nf_unregister_sockopt(&ip_vs_sockopts);
> err_sock:
> return ret;
> }
>
> +void ip_vs_unregister_nl_ioctl(void)
There is a extra trailing whitespace behind
ip_vs_unregister_nl_ioctl(void)
> +{
> + ip_vs_genl_unregister();
> + nf_unregister_sockopt(&ip_vs_sockopts);
> +}
> +
> +int __init ip_vs_control_init(void)
> +{
> + int idx;
> + int ret;
> +
> + EnterFunction(2);
> +
> + /* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
> + for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++) {
The for loop is funny spaced...
> + INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> + INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> + }
> +
> + smp_wmb(); /* Do we really need it now ? */
> +
> + ret = register_netdevice_notifier(&ip_vs_dst_notifier);
> + if (ret < 0)
> + return ret;
> +
> + LeaveFunction(2);
> + return 0;
> +}
> +
>
> void ip_vs_control_cleanup(void)
> {
> EnterFunction(2);
> unregister_netdevice_notifier(&ip_vs_dst_notifier);
> - ip_vs_genl_unregister();
> - nf_unregister_sockopt(&ip_vs_sockopts);
> LeaveFunction(2);
> }
--
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 1/1] ipvs: kernel oops - do_ip_vs_get_ctl
From: Julian Anastasov @ 2012-04-25 8:56 UTC (permalink / raw)
To: Hans Schillstrom; +Cc: horms, wensong, lvs-devel, netdev, netfilter-devel, hans
In-Reply-To: <1335339884-29825-1-git-send-email-hans.schillstrom@ericsson.com>
Hello,
On Wed, 25 Apr 2012, Hans Schillstrom wrote:
> Change order of init so netns init is ready
> when register ioctl and netlink.
>
> Reported-by: "Ryan O'Hara" <rohara@redhat.com>
> Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
> ---
> include/net/ip_vs.h | 2 +
> net/netfilter/ipvs/ip_vs_core.c | 9 ++++++
> net/netfilter/ipvs/ip_vs_ctl.c | 52 ++++++++++++++++++++++----------------
> 3 files changed, 41 insertions(+), 22 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 7131417..efaf484 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3750,21 +3750,10 @@ void __net_exit ip_vs_control_net_cleanup(struct net *net)
> free_percpu(ipvs->tot_stats.cpustats);
> }
>
> -int __init ip_vs_control_init(void)
> +int ip_vs_register_nl_ioctl(void)
Can we add __init here. Everything else looks good.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ 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