* Re: [Fwd: Re: [PATCH] Multicast packet reassembly can fail]
From: Steve Chen @ 2009-10-28 21:11 UTC (permalink / raw)
To: David Stevens; +Cc: Eric Dumazet, mhuth, netdev, netdev-owner
In-Reply-To: <OFF3C0AE1C.0C392C88-ON8825765D.006F8D81-8825765D.006FF4C0@us.ibm.com>
On Wed, 2009-10-28 at 13:22 -0700, David Stevens wrote:
> netdev-owner@vger.kernel.org wrote on 10/28/2009 11:25:39 AM:
>
> > I see the point you are making. I assumed, probably incorrectly, that
> > since eth0 and eth1 have different IP address. I would get a complete
> > series of fragments for each interface. Perhaps, I should really be
> > looking up the stack to see why packets were dropped. Please correct me
> > if I'm mistaken. The normal behavior is that application should be
> > receiving either 2 (scenario 1) or 1 (scenario 2) packets.
>
> Steve,
> If you didn't join the group on both interfaces, you won't receive
> two copies in the first place; the unjoined NIC won't deliver anything
> up the stack that isn't in it's multicast address filter.
>
> +-DLS
Thanks for the inputs. I'll revisit the issue.
Steve
^ permalink raw reply
* Re: [PATCH] AF_RAW: Augment raw_send_hdrinc to expand skb to fit iphdr->ihl (v2)
From: Eric Dumazet @ 2009-10-28 21:01 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, davem
In-Reply-To: <20091028185947.GA12675@hmsreliant.think-freely.org>
Neil Horman a écrit :
>
>> I believe we should drop the request, since padding it is not what was expected by user.
>
> Yeah, I had a feeling. Ok, version 2, this time drop the invalid frame and
> report it to user space, instead of expanding it:
>
>
> Augment raw_send_hdrinc to correct for incorrect ip header length values
>
> A series of oopses was reported to me recently. Apparently when using AF_RAW
> sockets to send data to peers that were reachable via ipsec encapsulation,
> people could panic or BUG halt their systems.
>
> I've tracked the problem down to user space sending an invalid ip header over an
> AF_RAW socket with IP_HDRINCL set to 1.
>
> Basically what happens is that userspace sends down an ip frame that includes
> only the header (no data), but sets the ip header ihl value to a large number,
> one that is larger than the total amount of data passed to the sendmsg call. In
> raw_send_hdrincl, we allocate an skb based on the size of the data in the msghdr
> that was passed in, but assume the data is all valid. Later during ipsec
> encapsulation, xfrm4_tranport_output moves the entire frame back in the skbuff
> to provide headroom for the ipsec headers. During this operation, the
> skb->transport_header is repointed to a spot computed by
> skb->network_header + the ip header length (ihl). Since so little data was
> passed in relative to the value of ihl provided by the raw socket, we point
> transport header to an unknown location, resulting in various crashes.
>
> This fix for this is pretty straightforward, simply validate the value of of
> iph->ihl when sending over a raw socket. If (iph->ihl*4U) > user data buffer
> size, drop the frame and return -EINVAL. I just confirmed this fixes the
> reported crashes.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* pull request: wireless-2.6 2009-10-28
From: John W. Linville @ 2009-10-28 20:53 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Dave,
Here is a collection of fixes (mostly (almost-)one-liners) intended
for 2.6.32. There are a couple of fixes relating to behavior when an
association failes, as well as spelling fixes, simply endian fixes,
a MAINTAINERS change...I don't think there is anything controversial
here...
Please let me know if there are problems!
Thanks,
John
---
Individual patches are available here:
http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/
---
The following changes since commit 66ed1e5ec1d979e572554643063734a7664261bb:
Eric Dumazet (1):
pktgen: Dont leak kernel memory
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master
Andrey Yurovsky (1):
mac80211: trivial: fix spelling in mesh_hwmp
Bartlomiej Zolnierkiewicz (1):
MAINTAINERS: rt2x00 list is moderated
Benoit PAPILLAULT (1):
rt2x00: Fix crypto in TX frame for rt2800usb
Björn Smedman (1):
mac80211: fix for incorrect sequence number on hostapd injected frames
Holger Schurig (1):
libertas spi: fix sparse errors
Johannes Berg (2):
mac80211: keep auth state when assoc fails
cfg80211: sme: deauthenticate on assoc failure
Larry Finger (1):
b43: Fix Bugzilla #14181 and the bug from the previous 'fix'
Miguel Boton (1):
b43: add 'struct b43_wl' missing declaration
Reinette Chatre (1):
mac80211: fix ibss joining
Roel Kluin (1):
airo: Reorder tests, check bounds before element
MAINTAINERS | 2 +-
drivers/net/wireless/airo.c | 5 ++---
drivers/net/wireless/b43/leds.h | 1 +
drivers/net/wireless/b43/main.c | 1 -
drivers/net/wireless/b43/rfkill.c | 3 ++-
drivers/net/wireless/libertas/if_spi.c | 10 +++++-----
drivers/net/wireless/rt2x00/rt2800usb.c | 2 +-
net/mac80211/ibss.c | 6 ++----
net/mac80211/mesh_hwmp.c | 2 +-
net/mac80211/mlme.c | 3 +--
net/mac80211/tx.c | 2 +-
net/wireless/core.h | 1 +
net/wireless/mlme.c | 9 +++++++++
net/wireless/sme.c | 21 +++++++++++++++++++--
14 files changed, 46 insertions(+), 22 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 70bc7de..cdbbaf5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4280,7 +4280,7 @@ F: drivers/video/aty/aty128fb.c
RALINK RT2X00 WIRELESS LAN DRIVER
P: rt2x00 project
L: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
-L: users-poMEt7QlJxcwIE2E9O76wjtx2kNaKg5H@public.gmane.org
+L: users-poMEt7QlJxcwIE2E9O76wjtx2kNaKg5H@public.gmane.org (moderated for non-subscribers)
W: http://rt2x00.serialmonkey.com/
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/ivd/rt2x00.git
diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index 7116a1a..abf896a 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -4790,9 +4790,8 @@ static int proc_stats_rid_open( struct inode *inode,
static int get_dec_u16( char *buffer, int *start, int limit ) {
u16 value;
int valid = 0;
- for( value = 0; buffer[*start] >= '0' &&
- buffer[*start] <= '9' &&
- *start < limit; (*start)++ ) {
+ for (value = 0; *start < limit && buffer[*start] >= '0' &&
+ buffer[*start] <= '9'; (*start)++) {
valid = 1;
value *= 10;
value += buffer[*start] - '0';
diff --git a/drivers/net/wireless/b43/leds.h b/drivers/net/wireless/b43/leds.h
index 4c56187..32b66d5 100644
--- a/drivers/net/wireless/b43/leds.h
+++ b/drivers/net/wireless/b43/leds.h
@@ -1,6 +1,7 @@
#ifndef B43_LEDS_H_
#define B43_LEDS_H_
+struct b43_wl;
struct b43_wldev;
#ifdef CONFIG_B43_LEDS
diff --git a/drivers/net/wireless/b43/main.c b/drivers/net/wireless/b43/main.c
index df6b26a..86f3582 100644
--- a/drivers/net/wireless/b43/main.c
+++ b/drivers/net/wireless/b43/main.c
@@ -4501,7 +4501,6 @@ static void b43_op_stop(struct ieee80211_hw *hw)
cancel_work_sync(&(wl->beacon_update_trigger));
- wiphy_rfkill_stop_polling(hw->wiphy);
mutex_lock(&wl->mutex);
if (b43_status(dev) >= B43_STAT_STARTED) {
dev = b43_wireless_core_stop(dev);
diff --git a/drivers/net/wireless/b43/rfkill.c b/drivers/net/wireless/b43/rfkill.c
index 7a3218c..ffdce6f 100644
--- a/drivers/net/wireless/b43/rfkill.c
+++ b/drivers/net/wireless/b43/rfkill.c
@@ -33,7 +33,8 @@ bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
& B43_MMIO_RADIO_HWENABLED_HI_MASK))
return 1;
} else {
- if (b43_read16(dev, B43_MMIO_RADIO_HWENABLED_LO)
+ if (b43_status(dev) >= B43_STAT_STARTED &&
+ b43_read16(dev, B43_MMIO_RADIO_HWENABLED_LO)
& B43_MMIO_RADIO_HWENABLED_LO_MASK)
return 1;
}
diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
index cb8be8d..5b3672c 100644
--- a/drivers/net/wireless/libertas/if_spi.c
+++ b/drivers/net/wireless/libertas/if_spi.c
@@ -134,7 +134,7 @@ static void spu_transaction_finish(struct if_spi_card *card)
static int spu_write(struct if_spi_card *card, u16 reg, const u8 *buf, int len)
{
int err = 0;
- u16 reg_out = cpu_to_le16(reg | IF_SPI_WRITE_OPERATION_MASK);
+ __le16 reg_out = cpu_to_le16(reg | IF_SPI_WRITE_OPERATION_MASK);
struct spi_message m;
struct spi_transfer reg_trans;
struct spi_transfer data_trans;
@@ -166,7 +166,7 @@ static int spu_write(struct if_spi_card *card, u16 reg, const u8 *buf, int len)
static inline int spu_write_u16(struct if_spi_card *card, u16 reg, u16 val)
{
- u16 buff;
+ __le16 buff;
buff = cpu_to_le16(val);
return spu_write(card, reg, (u8 *)&buff, sizeof(u16));
@@ -188,7 +188,7 @@ static int spu_read(struct if_spi_card *card, u16 reg, u8 *buf, int len)
{
unsigned int delay;
int err = 0;
- u16 reg_out = cpu_to_le16(reg | IF_SPI_READ_OPERATION_MASK);
+ __le16 reg_out = cpu_to_le16(reg | IF_SPI_READ_OPERATION_MASK);
struct spi_message m;
struct spi_transfer reg_trans;
struct spi_transfer dummy_trans;
@@ -235,7 +235,7 @@ static int spu_read(struct if_spi_card *card, u16 reg, u8 *buf, int len)
/* Read 16 bits from an SPI register */
static inline int spu_read_u16(struct if_spi_card *card, u16 reg, u16 *val)
{
- u16 buf;
+ __le16 buf;
int ret;
ret = spu_read(card, reg, (u8 *)&buf, sizeof(buf));
@@ -248,7 +248,7 @@ static inline int spu_read_u16(struct if_spi_card *card, u16 reg, u16 *val)
* The low 16 bits are read first. */
static int spu_read_u32(struct if_spi_card *card, u16 reg, u32 *val)
{
- u32 buf;
+ __le32 buf;
int err;
err = spu_read(card, reg, (u8 *)&buf, sizeof(buf));
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index a084077..9fe770f 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -1994,7 +1994,7 @@ static void rt2800usb_write_tx_desc(struct rt2x00_dev *rt2x00dev,
rt2x00_set_field32(&word, TXWI_W1_BW_WIN_SIZE, txdesc->ba_size);
rt2x00_set_field32(&word, TXWI_W1_WIRELESS_CLI_ID,
test_bit(ENTRY_TXD_ENCRYPT, &txdesc->flags) ?
- (skbdesc->entry->entry_idx + 1) : 0xff);
+ txdesc->key_idx : 0xff);
rt2x00_set_field32(&word, TXWI_W1_MPDU_TOTAL_BYTE_COUNT,
skb->len - txdesc->l2pad);
rt2x00_set_field32(&word, TXWI_W1_PACKETID,
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 6eaf698..ca8ecce 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -538,13 +538,12 @@ static void ieee80211_sta_find_ibss(struct ieee80211_sub_if_data *sdata)
WLAN_CAPABILITY_PRIVACY,
capability);
+ if (bss) {
#ifdef CONFIG_MAC80211_IBSS_DEBUG
- if (bss)
printk(KERN_DEBUG " sta_find_ibss: selected %pM current "
"%pM\n", bss->cbss.bssid, ifibss->bssid);
#endif /* CONFIG_MAC80211_IBSS_DEBUG */
- if (bss && !memcmp(ifibss->bssid, bss->cbss.bssid, ETH_ALEN)) {
printk(KERN_DEBUG "%s: Selected IBSS BSSID %pM"
" based on configured SSID\n",
sdata->dev->name, bss->cbss.bssid);
@@ -552,8 +551,7 @@ static void ieee80211_sta_find_ibss(struct ieee80211_sub_if_data *sdata)
ieee80211_sta_join_ibss(sdata, bss);
ieee80211_rx_bss_put(local, bss);
return;
- } else if (bss)
- ieee80211_rx_bss_put(local, bss);
+ }
#ifdef CONFIG_MAC80211_IBSS_DEBUG
printk(KERN_DEBUG " did not try to join ibss\n");
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index e12a786..29b82e9 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -259,7 +259,7 @@ static u32 airtime_link_metric_get(struct ieee80211_local *local,
* @hwmp_ie: hwmp information element (PREP or PREQ)
*
* This function updates the path routing information to the originator and the
- * transmitter of a HWMP PREQ or PREP fram.
+ * transmitter of a HWMP PREQ or PREP frame.
*
* Returns: metric to frame originator or 0 if the frame should not be further
* processed
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 8d26e9b..dc5049d 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1457,8 +1457,7 @@ ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
if (status_code != WLAN_STATUS_SUCCESS) {
printk(KERN_DEBUG "%s: AP denied association (code=%d)\n",
sdata->dev->name, status_code);
- list_del(&wk->list);
- kfree(wk);
+ wk->state = IEEE80211_MGD_STATE_IDLE;
return RX_MGMT_CFG80211_ASSOC;
}
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index db4bda6..eaa4118 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1445,7 +1445,7 @@ static void ieee80211_xmit(struct ieee80211_sub_if_data *sdata,
if (tmp_sdata->vif.type != NL80211_IFTYPE_AP)
continue;
if (compare_ether_addr(tmp_sdata->dev->dev_addr,
- hdr->addr2)) {
+ hdr->addr2) == 0) {
dev_hold(tmp_sdata->dev);
dev_put(sdata->dev);
sdata = tmp_sdata;
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 2a33d8b..68b3219 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -358,6 +358,7 @@ int cfg80211_mgd_wext_connect(struct cfg80211_registered_device *rdev,
struct wireless_dev *wdev);
void cfg80211_conn_work(struct work_struct *work);
+void cfg80211_sme_failed_assoc(struct wireless_dev *wdev);
bool cfg80211_sme_failed_reassoc(struct wireless_dev *wdev);
/* internal helpers */
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 79d2eec..0a6b7a0 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -62,6 +62,7 @@ void cfg80211_send_rx_assoc(struct net_device *dev, const u8 *buf, size_t len)
u8 *ie = mgmt->u.assoc_resp.variable;
int i, ieoffs = offsetof(struct ieee80211_mgmt, u.assoc_resp.variable);
struct cfg80211_internal_bss *bss = NULL;
+ bool need_connect_result = true;
wdev_lock(wdev);
@@ -94,6 +95,14 @@ void cfg80211_send_rx_assoc(struct net_device *dev, const u8 *buf, size_t len)
}
WARN_ON(!bss);
+ } else if (wdev->conn) {
+ cfg80211_sme_failed_assoc(wdev);
+ need_connect_result = false;
+ /*
+ * do not call connect_result() now because the
+ * sme will schedule work that does it later.
+ */
+ goto out;
}
if (!wdev->conn && wdev->sme_state == CFG80211_SME_IDLE) {
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 93c3ed3..ece378d 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -26,6 +26,7 @@ struct cfg80211_conn {
CFG80211_CONN_AUTHENTICATING,
CFG80211_CONN_ASSOCIATE_NEXT,
CFG80211_CONN_ASSOCIATING,
+ CFG80211_CONN_DEAUTH_ASSOC_FAIL,
} state;
u8 bssid[ETH_ALEN], prev_bssid[ETH_ALEN];
u8 *ie;
@@ -148,6 +149,12 @@ static int cfg80211_conn_do_work(struct wireless_dev *wdev)
NULL, 0,
WLAN_REASON_DEAUTH_LEAVING);
return err;
+ case CFG80211_CONN_DEAUTH_ASSOC_FAIL:
+ __cfg80211_mlme_deauth(rdev, wdev->netdev, params->bssid,
+ NULL, 0,
+ WLAN_REASON_DEAUTH_LEAVING);
+ /* return an error so that we call __cfg80211_connect_result() */
+ return -EINVAL;
default:
return 0;
}
@@ -158,6 +165,7 @@ void cfg80211_conn_work(struct work_struct *work)
struct cfg80211_registered_device *rdev =
container_of(work, struct cfg80211_registered_device, conn_work);
struct wireless_dev *wdev;
+ u8 bssid[ETH_ALEN];
rtnl_lock();
cfg80211_lock_rdev(rdev);
@@ -173,10 +181,10 @@ void cfg80211_conn_work(struct work_struct *work)
wdev_unlock(wdev);
continue;
}
+ memcpy(bssid, wdev->conn->params.bssid, ETH_ALEN);
if (cfg80211_conn_do_work(wdev))
__cfg80211_connect_result(
- wdev->netdev,
- wdev->conn->params.bssid,
+ wdev->netdev, bssid,
NULL, 0, NULL, 0,
WLAN_STATUS_UNSPECIFIED_FAILURE,
false, NULL);
@@ -337,6 +345,15 @@ bool cfg80211_sme_failed_reassoc(struct wireless_dev *wdev)
return true;
}
+void cfg80211_sme_failed_assoc(struct wireless_dev *wdev)
+{
+ struct wiphy *wiphy = wdev->wiphy;
+ struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+
+ wdev->conn->state = CFG80211_CONN_DEAUTH_ASSOC_FAIL;
+ schedule_work(&rdev->conn_work);
+}
+
void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
const u8 *req_ie, size_t req_ie_len,
const u8 *resp_ie, size_t resp_ie_len,
--
John W. Linville Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org might be all we have. Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 4/6] vlan: Optimize multiple unregistration
From: Eric Dumazet @ 2009-10-28 20:47 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AE8A425.1000600@trash.net>
Patrick McHardy a écrit :
> Eric Dumazet wrote:
>> Use unregister_netdevice_many() to speedup master device unregister.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> ---
>> include/linux/if_vlan.h | 1
>> net/8021q/vlan.c | 49 +++++++++++++++++++++++++-------------
>> net/core/dev.c | 1
>> 3 files changed, 35 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
>> index 8898cbe..71a4870 100644
>> --- a/include/linux/if_vlan.h
>> +++ b/include/linux/if_vlan.h
>> @@ -85,6 +85,7 @@ struct vlan_group {
>> * the vlan is attached to.
>> */
>> unsigned int nr_vlans;
>> + int killall;
>> struct hlist_node hlist; /* linked list */
>> struct net_device **vlan_devices_arrays[VLAN_GROUP_ARRAY_SPLIT_PARTS];
>> struct rcu_head rcu;
>> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
>> index 6b5c9dd..511afe7 100644
>> --- a/net/8021q/vlan.c
>> +++ b/net/8021q/vlan.c
>> @@ -159,11 +159,12 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
>> if (real_dev->features & NETIF_F_HW_VLAN_FILTER)
>> ops->ndo_vlan_rx_kill_vid(real_dev, vlan_id);
>>
>> - vlan_group_set_device(grp, vlan_id, NULL);
>> grp->nr_vlans--;
>>
>> - synchronize_net();
>> -
>> + if (!grp->killall) {
>> + vlan_group_set_device(grp, vlan_id, NULL);
>> + synchronize_net();
>> + }
>> unregister_netdevice_queue(dev, head);
>>
>> /* If the group is now empty, kill off the group. */
>> @@ -183,6 +184,34 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
>> dev_put(real_dev);
>> }
>>
>> +void unregister_vlan_dev_alls(struct vlan_group *grp)
>
> This could be static.
>
>> +{
>> + LIST_HEAD(list);
>> + int i;
>> + struct net_device *vlandev;
>> + struct vlan_group save;
>> +
>> + memcpy(&save, grp, sizeof(save));
>> + memset(&grp->vlan_devices_arrays, 0, sizeof(grp->vlan_devices_arrays));
>
> This shouldn't be necessary since the lower device is already in the
> process of being unregistered. If it was necessary, it could cause
> crashes since the individual pointers are not set to zero atomically.
> Or maybe I'm misunderstanding the purpose entirely :)
Very good point indeed, even if in practice memset() use long word transferts
I'll make a cleanup patch, or do you want to do it ?
^ permalink raw reply
* Re: iproute uses too small of a receive buffer
From: Eric Dumazet @ 2009-10-28 20:38 UTC (permalink / raw)
To: Ben Greear; +Cc: Patrick McHardy, Stephen Hemminger, NetDev
In-Reply-To: <4AE89927.9090405@candelatech.com>
Ben Greear a écrit :
> Second: Why bail on ENOBUFS at all? I don't see how it helps the user
> since they will probably just have to start it again, and will miss more
> messages than keeping going would have.
>
> And, even 1MB may not be enough for some scenarios. So, probably best to
> let users over-ride the initial setting on cmd-line. If not, then use
> a large value to start with.
>
In this case, just dont call setsockopt() at all in "ip" and let system use the
standard/default value (/proc/sys/net/core/rmem_default) that an admin can change
if he wants to handle one million devices :)
^ permalink raw reply
* Re: [Fwd: Re: [PATCH] Multicast packet reassembly can fail]
From: David Stevens @ 2009-10-28 20:22 UTC (permalink / raw)
To: Steve Chen; +Cc: Eric Dumazet, mhuth, netdev, netdev-owner
In-Reply-To: <1256754339.3153.481.camel@linux-1lbu>
netdev-owner@vger.kernel.org wrote on 10/28/2009 11:25:39 AM:
> I see the point you are making. I assumed, probably incorrectly, that
> since eth0 and eth1 have different IP address. I would get a complete
> series of fragments for each interface. Perhaps, I should really be
> looking up the stack to see why packets were dropped. Please correct me
> if I'm mistaken. The normal behavior is that application should be
> receiving either 2 (scenario 1) or 1 (scenario 2) packets.
Steve,
If you didn't join the group on both interfaces, you won't receive
two copies in the first place; the unjoined NIC won't deliver anything
up the stack that isn't in it's multicast address filter.
+-DLS
^ permalink raw reply
* Re: iproute uses too small of a receive buffer
From: Ben Greear @ 2009-10-28 20:21 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Eric Dumazet, Stephen Hemminger, NetDev
In-Reply-To: <4AE8A48A.1060407@trash.net>
On 10/28/2009 01:07 PM, Patrick McHardy wrote:
> Ben Greear wrote:
>> On 10/28/2009 12:50 PM, Patrick McHardy wrote:
>>
>>>> And, even 1MB may not be enough for some scenarios. So, probably
>>>> best to
>>>> let users over-ride the initial setting on cmd-line. If not, then use
>>>> a large value to start with.
>>>
>>> How about this? It uses 1MB as receive buf limit by default (without
>>> increasing /proc/sys/net/core/rmem_max it will be limited by less
>>> however) and allows to specify the size manually using "-rcvbuf X"
>>> (-r is already used, so you need to specify at least -rc).
>>>
>>> Additionally rtnl_listen() continues on ENOBUFS after printing the
>>> error message.
>>
>> Looks good..except:
>>
>> If rmem_max is smaller than 1M, will that cause setsocktopt to
>> fail and thus fail early out of rtnl_open_byproto?
>
> No, the kernel takes the value as a hint and only uses the
> maximum allowable value:
Sweet. No complaints from me then.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* nfs broken in net-next?
From: Yinghai Lu @ 2009-10-28 20:13 UTC (permalink / raw)
To: David Miller; +Cc: Linux Kernel Mailing List, NetDev
pk12-3214-189-102:~ # mount -t nfs 10.6.75.100:/data/shared/pxeboot /x
mount.nfs: rpc.statd is not running but is required for remote locking.
mount.nfs: Either use '-o nolock' to keep locks local, or start statd.
using opensuse11.1
YH
^ permalink raw reply
* Re: [PATCH] Multicast packet reassembly can fail
From: David Stevens @ 2009-10-28 20:12 UTC (permalink / raw)
To: Steve Chen; +Cc: netdev, netdev-owner
In-Reply-To: <1256683583.3153.389.camel@linux-1lbu>
I haven't gone through the entire thread yet, but I should point
out that this appears to break regular IP fragmentation for
unicast packets. There is no restriction whatsoever that
fragments from a remote destination that are actually for
the same datagram need to be routed on the same paths
and received on the same input interface.
For the multicast case, if they are from the same datagram,
it doesn't matter how you got them. If it's a different datagram
with the same ID, which can happen anyway, the checksum
should fail (at least (64K-1) of 64K cases). I don't see a special
case here, other than that you can tell by the interface if it was
actually a distinct datagram with the same ID in the multicast
case (and only in multicast and only if the different interfaces
are not in the same multicast routing domain).
NACK.
+-DLS
^ permalink raw reply
* Re: [PATCH 1/7] bitmap: Introduce bitmap_set, bitmap_clear, bitmap_find_next_zero_area
From: Andrew Morton @ 2009-10-28 20:11 UTC (permalink / raw)
To: Akinobu Mita
Cc: linux-kernel, FUJITA Tomonori, David S. Miller, sparclinux,
Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86,
Greg Kroah-Hartman, Lothar Wassmann, linux-usb, Roland Dreier,
Yevgeny Petrilin, netdev, Tony Luck, Fenghua Yu, linux-ia64,
linux-altix, Joerg Roedel
In-Reply-To: <1256719397-4258-1-git-send-email-akinobu.mita@gmail.com>
Why were these patches resent? What changed?
Everybody who is going to review these patches has already reviewed
them and now they need to review them all again?
^ permalink raw reply
* Re: iproute uses too small of a receive buffer
From: Patrick McHardy @ 2009-10-28 20:07 UTC (permalink / raw)
To: Ben Greear; +Cc: Eric Dumazet, Stephen Hemminger, NetDev
In-Reply-To: <4AE8A3C3.1070003@candelatech.com>
Ben Greear wrote:
> On 10/28/2009 12:50 PM, Patrick McHardy wrote:
>
>>> And, even 1MB may not be enough for some scenarios. So, probably
>>> best to
>>> let users over-ride the initial setting on cmd-line. If not, then use
>>> a large value to start with.
>>
>> How about this? It uses 1MB as receive buf limit by default (without
>> increasing /proc/sys/net/core/rmem_max it will be limited by less
>> however) and allows to specify the size manually using "-rcvbuf X"
>> (-r is already used, so you need to specify at least -rc).
>>
>> Additionally rtnl_listen() continues on ENOBUFS after printing the
>> error message.
>
> Looks good..except:
>
> If rmem_max is smaller than 1M, will that cause setsocktopt to
> fail and thus fail early out of rtnl_open_byproto?
No, the kernel takes the value as a hint and only uses the
maximum allowable value:
case SO_RCVBUF:
/* Don't error on this BSD doesn't and if you think
about it this is right. Otherwise apps have to
play 'guess the biggest size' games. RCVBUF/SNDBUF
are treated in BSD as hints */
if (val > sysctl_rmem_max)
val = sysctl_rmem_max;
> Maybe we should only print errors but not return in that method
> when setsockopt fails?
>
> In another project, I ended up trying ever smaller values until one
> worked in order to get near what the user wanted even if rmem_max
> was configured smaller. Not sure if that is worth doing here or not.
I think it should be fine this way.
^ permalink raw reply
* Re: [PATCH 4/6] vlan: Optimize multiple unregistration
From: Patrick McHardy @ 2009-10-28 20:05 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AE728A9.2080209@gmail.com>
Eric Dumazet wrote:
> Use unregister_netdevice_many() to speedup master device unregister.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> include/linux/if_vlan.h | 1
> net/8021q/vlan.c | 49 +++++++++++++++++++++++++-------------
> net/core/dev.c | 1
> 3 files changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 8898cbe..71a4870 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -85,6 +85,7 @@ struct vlan_group {
> * the vlan is attached to.
> */
> unsigned int nr_vlans;
> + int killall;
> struct hlist_node hlist; /* linked list */
> struct net_device **vlan_devices_arrays[VLAN_GROUP_ARRAY_SPLIT_PARTS];
> struct rcu_head rcu;
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index 6b5c9dd..511afe7 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -159,11 +159,12 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
> if (real_dev->features & NETIF_F_HW_VLAN_FILTER)
> ops->ndo_vlan_rx_kill_vid(real_dev, vlan_id);
>
> - vlan_group_set_device(grp, vlan_id, NULL);
> grp->nr_vlans--;
>
> - synchronize_net();
> -
> + if (!grp->killall) {
> + vlan_group_set_device(grp, vlan_id, NULL);
> + synchronize_net();
> + }
> unregister_netdevice_queue(dev, head);
>
> /* If the group is now empty, kill off the group. */
> @@ -183,6 +184,34 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
> dev_put(real_dev);
> }
>
> +void unregister_vlan_dev_alls(struct vlan_group *grp)
This could be static.
> +{
> + LIST_HEAD(list);
> + int i;
> + struct net_device *vlandev;
> + struct vlan_group save;
> +
> + memcpy(&save, grp, sizeof(save));
> + memset(&grp->vlan_devices_arrays, 0, sizeof(grp->vlan_devices_arrays));
This shouldn't be necessary since the lower device is already in the
process of being unregistered. If it was necessary, it could cause
crashes since the individual pointers are not set to zero atomically.
Or maybe I'm misunderstanding the purpose entirely :)
> + grp->killall = 1;
> +
> + synchronize_net();
> +
> + /* Delete all VLANs for this dev. */
> + for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) {
> + vlandev = vlan_group_get_device(&save, i);
> + if (!vlandev)
> + continue;
> +
> + unregister_vlan_dev(vlandev, &list);
> + if (grp->nr_vlans == 0)
> + break;
> + }
> + unregister_netdevice_many(&list);
> + for (i = 0; i < VLAN_GROUP_ARRAY_SPLIT_PARTS; i++)
> + kfree(save.vlan_devices_arrays[i]);
> +}
> +
> static void vlan_transfer_operstate(const struct net_device *dev,
> struct net_device *vlandev)
> {
> @@ -524,19 +553,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
> break;
>
> case NETDEV_UNREGISTER:
> - /* Delete all VLANs for this dev. */
> - for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) {
> - vlandev = vlan_group_get_device(grp, i);
> - if (!vlandev)
> - continue;
> -
> - /* unregistration of last vlan destroys group, abort
> - * afterwards */
> - if (grp->nr_vlans == 1)
> - i = VLAN_GROUP_ARRAY_LEN;
> -
> - unregister_vlan_dev(vlandev, NULL);
> - }
> + unregister_vlan_dev_alls(grp);
> break;
> }
>
^ permalink raw reply
* Re: iproute uses too small of a receive buffer
From: Ben Greear @ 2009-10-28 20:04 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Eric Dumazet, Stephen Hemminger, NetDev
In-Reply-To: <4AE8A098.8040207@trash.net>
On 10/28/2009 12:50 PM, Patrick McHardy wrote:
>> And, even 1MB may not be enough for some scenarios. So, probably best to
>> let users over-ride the initial setting on cmd-line. If not, then use
>> a large value to start with.
>
> How about this? It uses 1MB as receive buf limit by default (without
> increasing /proc/sys/net/core/rmem_max it will be limited by less
> however) and allows to specify the size manually using "-rcvbuf X"
> (-r is already used, so you need to specify at least -rc).
>
> Additionally rtnl_listen() continues on ENOBUFS after printing the
> error message.
Looks good..except:
If rmem_max is smaller than 1M, will that cause setsocktopt to
fail and thus fail early out of rtnl_open_byproto?
Maybe we should only print errors but not return in that method
when setsockopt fails?
In another project, I ended up trying ever smaller values until one
worked in order to get near what the user wanted even if rmem_max
was configured smaller. Not sure if that is worth doing here or not.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: iproute uses too small of a receive buffer
From: Patrick McHardy @ 2009-10-28 19:50 UTC (permalink / raw)
To: Ben Greear; +Cc: Eric Dumazet, Stephen Hemminger, NetDev
In-Reply-To: <4AE89927.9090405@candelatech.com>
[-- Attachment #1: Type: text/plain, Size: 2006 bytes --]
Ben Greear wrote:
> On 10/28/2009 12:05 PM, Patrick McHardy wrote:
>> Eric Dumazet wrote:
>>> Stephen Hemminger a écrit :
>>>> Just having larger buffer isn't guarantee of success. Allocating
>>>> a huge buffer is not going to work on embedded.
>>>>
>>>
>>> Please note we do not allocate a big buffer, only allow more small skbs
>>> to be queued on socket receive queue.
>>>
>>> If memory is not available, skb allocation will eventually fail
>>> and be reported as well, embedded or not.
>>>
>>> I vote for allowing 1024*1024 bytes instead of 32768,
>>> and eventually user should be warned that it is capped by
>>> /proc/sys/net/core/rmem_max
>>
>> How about this? It will double the receive queue limit on ENOBUFS
>> up to 1024 * 1024b, then bail out with the normal error message on
>> further ENOBUFS.
>>
>> Signed-off-by: Patrick McHardy<kaber@trash.net>
>
> First: This still pretty much guarantees that messages will be lost when
> the program starts (when messages are coming in too large of chunks for
> small buffers)
> If you are debugging something tricky, having lost messages will be
> very annoying!
Yeah, on second thought the probing also doesn't make too much sense
since the memory is only used when its really needed anyways. And its
capped by rmem_max.
> Second: Why bail on ENOBUFS at all? I don't see how it helps the user
> since they will probably just have to start it again, and will miss more
> messages than keeping going would have.
Agreed.
> And, even 1MB may not be enough for some scenarios. So, probably best to
> let users over-ride the initial setting on cmd-line. If not, then use
> a large value to start with.
How about this? It uses 1MB as receive buf limit by default (without
increasing /proc/sys/net/core/rmem_max it will be limited by less
however) and allows to specify the size manually using "-rcvbuf X"
(-r is already used, so you need to specify at least -rc).
Additionally rtnl_listen() continues on ENOBUFS after printing the
error message.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2170 bytes --]
diff --git a/include/libnetlink.h b/include/libnetlink.h
index 0e02468..61da15b 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -17,6 +17,8 @@ struct rtnl_handle
__u32 dump;
};
+extern int rcvbuf;
+
extern int rtnl_open(struct rtnl_handle *rth, unsigned subscriptions);
extern int rtnl_open_byproto(struct rtnl_handle *rth, unsigned subscriptions, int protocol);
extern void rtnl_close(struct rtnl_handle *rth);
diff --git a/ip/ip.c b/ip/ip.c
index 2bd54b2..b4c076a 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -50,7 +50,8 @@ static void usage(void)
" tunnel | maddr | mroute | monitor | xfrm }\n"
" OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[esolve] |\n"
" -f[amily] { inet | inet6 | ipx | dnet | link } |\n"
-" -o[neline] | -t[imestamp] | -b[atch] [filename] }\n");
+" -o[neline] | -t[imestamp] | -b[atch] [filename] |\n"
+" -rc[vbuf] [size]}\n");
exit(-1);
}
@@ -213,6 +214,19 @@ int main(int argc, char **argv)
if (argc <= 1)
usage();
batch_file = argv[1];
+ } else if (matches(opt, "-rcvbuf") == 0) {
+ unsigned int size;
+
+ argc--;
+ argv++;
+ if (argc <= 1)
+ usage();
+ if (get_unsigned(&size, argv[1], 0)) {
+ fprintf(stderr, "Invalid rcvbuf size '%s'\n",
+ argv[1]);
+ exit(-1);
+ }
+ rcvbuf = size;
} else if (matches(opt, "-help") == 0) {
usage();
} else {
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index b68e2fd..5c716ab 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -25,6 +25,8 @@
#include "libnetlink.h"
+int rcvbuf = 1024 * 1024;
+
void rtnl_close(struct rtnl_handle *rth)
{
if (rth->fd >= 0) {
@@ -38,7 +40,6 @@ int rtnl_open_byproto(struct rtnl_handle *rth, unsigned subscriptions,
{
socklen_t addr_len;
int sndbuf = 32768;
- int rcvbuf = 32768;
memset(rth, 0, sizeof(*rth));
@@ -409,6 +410,8 @@ int rtnl_listen(struct rtnl_handle *rtnl,
continue;
fprintf(stderr, "netlink receive error %s (%d)\n",
strerror(errno), errno);
+ if (errno == ENOBUFS)
+ continue;
return -1;
}
if (status == 0) {
^ permalink raw reply related
* Re: iproute uses too small of a receive buffer
From: Ben Greear @ 2009-10-28 19:19 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Eric Dumazet, Stephen Hemminger, NetDev
In-Reply-To: <4AE895E8.60308@trash.net>
On 10/28/2009 12:05 PM, Patrick McHardy wrote:
> Eric Dumazet wrote:
>> Stephen Hemminger a écrit :
>>> Just having larger buffer isn't guarantee of success. Allocating
>>> a huge buffer is not going to work on embedded.
>>>
>>
>> Please note we do not allocate a big buffer, only allow more small skbs
>> to be queued on socket receive queue.
>>
>> If memory is not available, skb allocation will eventually fail
>> and be reported as well, embedded or not.
>>
>> I vote for allowing 1024*1024 bytes instead of 32768,
>> and eventually user should be warned that it is capped by
>> /proc/sys/net/core/rmem_max
>
> How about this? It will double the receive queue limit on ENOBUFS
> up to 1024 * 1024b, then bail out with the normal error message on
> further ENOBUFS.
>
> Signed-off-by: Patrick McHardy<kaber@trash.net>
First: This still pretty much guarantees that messages will be lost when
the program starts (when messages are coming in too large of chunks for small buffers)
If you are debugging something tricky, having lost messages will be
very annoying!
Second: Why bail on ENOBUFS at all? I don't see how it helps the user
since they will probably just have to start it again, and will miss more
messages than keeping going would have.
And, even 1MB may not be enough for some scenarios. So, probably best to
let users over-ride the initial setting on cmd-line. If not, then use
a large value to start with.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* RE: [PATCH] udev: create empty regular files to represent net interfaces
From: Narendra_K @ 2009-10-28 19:15 UTC (permalink / raw)
To: kay.sievers, Matt_Domsch
Cc: dannf, linux-hotplug, netdev, Jordan_Hargrave, Charles_Rose,
bhutchings
In-Reply-To: <ac3eb2510910280123g3c0e3d95wb38a239238906027@mail.gmail.com>
>That all sounds very much like something which will hit us
>back some day. I'm not sure, if udev should publish such dead
>text files in /dev, it does not seem to fit the usual
>APIs/assumptions where /sys and /dev match, and libudev
>provides access to both. It all sounds more like a database
>for a possible netdevname library, which does not need to be
>public in /dev, right?
The char device nodes under /dev/netdev/ do seem to adhere to the
assumption of what is there under /sys and /dev match.
With regards,
Narendra K
^ permalink raw reply
* Re: iproute uses too small of a receive buffer
From: Patrick McHardy @ 2009-10-28 19:05 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Stephen Hemminger, Ben Greear, NetDev
In-Reply-To: <4AE7F859.7020105@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 753 bytes --]
Eric Dumazet wrote:
> Stephen Hemminger a écrit :
>> Just having larger buffer isn't guarantee of success. Allocating
>> a huge buffer is not going to work on embedded.
>>
>
> Please note we do not allocate a big buffer, only allow more small skbs
> to be queued on socket receive queue.
>
> If memory is not available, skb allocation will eventually fail
> and be reported as well, embedded or not.
>
> I vote for allowing 1024*1024 bytes instead of 32768,
> and eventually user should be warned that it is capped by
> /proc/sys/net/core/rmem_max
How about this? It will double the receive queue limit on ENOBUFS
up to 1024 * 1024b, then bail out with the normal error message on
further ENOBUFS.
Signed-off-by: Patrick McHardy <kaber@trash.net>
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 894 bytes --]
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index b68e2fd..e4fda40 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -25,6 +25,8 @@
#include "libnetlink.h"
+static int rcvbuf = 32768;
+
void rtnl_close(struct rtnl_handle *rth)
{
if (rth->fd >= 0) {
@@ -38,7 +40,6 @@ int rtnl_open_byproto(struct rtnl_handle *rth, unsigned subscriptions,
{
socklen_t addr_len;
int sndbuf = 32768;
- int rcvbuf = 32768;
memset(rth, 0, sizeof(*rth));
@@ -407,6 +409,12 @@ int rtnl_listen(struct rtnl_handle *rtnl,
if (status < 0) {
if (errno == EINTR || errno == EAGAIN)
continue;
+ if (errno == ENOBUFS && rcvbuf < 1024 * 1024) {
+ rcvbuf *= 2;
+ if (setsockopt(rtnl->fd, SOL_SOCKET, SO_RCVBUF,
+ &rcvbuf, sizeof(rcvbuf)) == 0)
+ continue;
+ }
fprintf(stderr, "netlink receive error %s (%d)\n",
strerror(errno), errno);
return -1;
^ permalink raw reply related
* Re: [PATCH] AF_RAW: Augment raw_send_hdrinc to expand skb to fit iphdr->ihl (v2)
From: Neil Horman @ 2009-10-28 18:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem, nhorman
In-Reply-To: <4AE889B5.4040301@gmail.com>
On Wed, Oct 28, 2009 at 07:13:09PM +0100, Eric Dumazet wrote:
> Neil Horman a écrit :
> > Augment raw_send_hdrinc to correct for incorrect ip header length values
> >
> > A series of oopses was reported to me recently. Apparently when using AF_RAW
> > sockets to send data to peers that were reachable via ipsec encapsulation,
> > people could panic or BUG halt their systems.
> >
> > I've tracked the problem down to user space sending an invalid ip header over an
> > AF_RAW socket with IP_HDRINCL set to 1.
> >
> > Basically what happens is that userspace sends down an ip frame that includes
> > only the header (no data), but sets the ip header ihl value to a large number,
> > one that is larger than the total amount of data passed to the sendmsg call. In
> > raw_send_hdrincl, we allocate an skb based on the size of the data in the msghdr
> > that was passed in, but assume the data is all valid. Later during ipsec
> > encapsulation, xfrm4_tranport_output moves the entire frame back in the skbuff
> > to provide headroom for the ipsec headers. During this operation, the
> > skb->transport_header is repointed to a spot computed by
> > skb->network_header + the ip header length (ihl). Since so little data was
> > passed in relative to the value of ihl provided by the raw socket, we point
> > transport header to an unknown location, resulting in various crashes.
> >
> > So, what to do about this? My first thought was to simply return -EINVAL, and
> > let user space sort it out. I'm still thinking that might be the best way, but
> > I thought I'd try this first, just in case someone has reason to try to
> > send such a bogus frame through the kernel. This solution simply checks the
> > value of ihl in raw_send_hdrinc and expands the skb to fit, filling the new
> > space with IPOPT_NOOP options. I've confirmed that it fixes the crashes that
> > were reported.
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> >
>
> Thanks a lot for this detailed info, I wish everything could be explained like this !
>
You're welcome, this was a fun one to track down :)
> I believe we should drop the request, since padding it is not what was expected by user.
Yeah, I had a feeling. Ok, version 2, this time drop the invalid frame and
report it to user space, instead of expanding it:
Augment raw_send_hdrinc to correct for incorrect ip header length values
A series of oopses was reported to me recently. Apparently when using AF_RAW
sockets to send data to peers that were reachable via ipsec encapsulation,
people could panic or BUG halt their systems.
I've tracked the problem down to user space sending an invalid ip header over an
AF_RAW socket with IP_HDRINCL set to 1.
Basically what happens is that userspace sends down an ip frame that includes
only the header (no data), but sets the ip header ihl value to a large number,
one that is larger than the total amount of data passed to the sendmsg call. In
raw_send_hdrincl, we allocate an skb based on the size of the data in the msghdr
that was passed in, but assume the data is all valid. Later during ipsec
encapsulation, xfrm4_tranport_output moves the entire frame back in the skbuff
to provide headroom for the ipsec headers. During this operation, the
skb->transport_header is repointed to a spot computed by
skb->network_header + the ip header length (ihl). Since so little data was
passed in relative to the value of ihl provided by the raw socket, we point
transport header to an unknown location, resulting in various crashes.
This fix for this is pretty straightforward, simply validate the value of of
iph->ihl when sending over a raw socket. If (iph->ihl*4U) > user data buffer
size, drop the frame and return -EINVAL. I just confirmed this fixes the
reported crashes.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
raw.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 9ef8c08..4b15354 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -351,13 +351,24 @@ static int raw_send_hdrinc(struct sock *sk, void *from, size_t length,
skb->ip_summed = CHECKSUM_NONE;
skb->transport_header = skb->network_header;
- err = memcpy_fromiovecend((void *)iph, from, 0, length);
- if (err)
- goto error_fault;
+ err = -EFAULT;
+ if (memcpy_fromiovecend((void *)iph, from, 0, length))
+ goto error_free;
- /* We don't modify invalid header */
iphlen = iph->ihl * 4;
- if (iphlen >= sizeof(*iph) && iphlen <= length) {
+
+ /*
+ * We don't want to modify the ip header, but we do need to
+ * be sure that it won't cause problems later along the network
+ * stack. Specifically we want to make sure that iph->ihl is a
+ * sane value. If ihl points beyond the length of the buffer passed
+ * in, reject the frame as invalid
+ */
+ err = -EINVAL;
+ if (iphlen > length)
+ goto error_free;
+
+ if (iphlen >= sizeof(*iph)) {
if (!iph->saddr)
iph->saddr = rt->rt_src;
iph->check = 0;
@@ -380,8 +391,7 @@ static int raw_send_hdrinc(struct sock *sk, void *from, size_t length,
out:
return 0;
-error_fault:
- err = -EFAULT;
+error_free:
kfree_skb(skb);
error:
IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
^ permalink raw reply related
* [net-2.6 PATCH 0/2] qlge: Fixes for qlge.
From: Ron Mercer @ 2009-10-28 18:39 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
Fix EEH and firmware mailbox command processor.
^ permalink raw reply
* [net-2.6 PATCH 1/2] qlge: Fix EEH handling.
From: Ron Mercer @ 2009-10-28 18:39 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256755161-29606-1-git-send-email-ron.mercer@qlogic.com>
Clean up driver resources without touch the hardware. Add pci
save/restore state.
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge_main.c | 78 ++++++++++++++++++++++++++++--------------
1 files changed, 52 insertions(+), 26 deletions(-)
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 48b45df..cea7531 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -3916,6 +3916,7 @@ static int __devinit ql_init_device(struct pci_dev *pdev,
goto err_out;
}
+ pci_save_state(pdev);
qdev->reg_base =
ioremap_nocache(pci_resource_start(pdev, 1),
pci_resource_len(pdev, 1));
@@ -4070,6 +4071,33 @@ static void __devexit qlge_remove(struct pci_dev *pdev)
free_netdev(ndev);
}
+/* Clean up resources without touching hardware. */
+static void ql_eeh_close(struct net_device *ndev)
+{
+ int i;
+ struct ql_adapter *qdev = netdev_priv(ndev);
+
+ if (netif_carrier_ok(ndev)) {
+ netif_carrier_off(ndev);
+ netif_stop_queue(ndev);
+ }
+
+ if (test_bit(QL_ADAPTER_UP, &qdev->flags))
+ cancel_delayed_work_sync(&qdev->asic_reset_work);
+ cancel_delayed_work_sync(&qdev->mpi_reset_work);
+ cancel_delayed_work_sync(&qdev->mpi_work);
+ cancel_delayed_work_sync(&qdev->mpi_idc_work);
+ cancel_delayed_work_sync(&qdev->mpi_port_cfg_work);
+
+ for (i = 0; i < qdev->rss_ring_count; i++)
+ netif_napi_del(&qdev->rx_ring[i].napi);
+
+ clear_bit(QL_ADAPTER_UP, &qdev->flags);
+ ql_tx_ring_clean(qdev);
+ ql_free_rx_buffers(qdev);
+ ql_release_adapter_resources(qdev);
+}
+
/*
* This callback is called by the PCI subsystem whenever
* a PCI bus error is detected.
@@ -4078,17 +4106,21 @@ static pci_ers_result_t qlge_io_error_detected(struct pci_dev *pdev,
enum pci_channel_state state)
{
struct net_device *ndev = pci_get_drvdata(pdev);
- struct ql_adapter *qdev = netdev_priv(ndev);
-
- netif_device_detach(ndev);
- if (state == pci_channel_io_perm_failure)
+ switch (state) {
+ case pci_channel_io_normal:
+ return PCI_ERS_RESULT_CAN_RECOVER;
+ case pci_channel_io_frozen:
+ netif_device_detach(ndev);
+ if (netif_running(ndev))
+ ql_eeh_close(ndev);
+ pci_disable_device(pdev);
+ return PCI_ERS_RESULT_NEED_RESET;
+ case pci_channel_io_perm_failure:
+ dev_err(&pdev->dev,
+ "%s: pci_channel_io_perm_failure.\n", __func__);
return PCI_ERS_RESULT_DISCONNECT;
-
- if (netif_running(ndev))
- ql_adapter_down(qdev);
-
- pci_disable_device(pdev);
+ }
/* Request a slot reset. */
return PCI_ERS_RESULT_NEED_RESET;
@@ -4105,25 +4137,15 @@ static pci_ers_result_t qlge_io_slot_reset(struct pci_dev *pdev)
struct net_device *ndev = pci_get_drvdata(pdev);
struct ql_adapter *qdev = netdev_priv(ndev);
+ pdev->error_state = pci_channel_io_normal;
+
+ pci_restore_state(pdev);
if (pci_enable_device(pdev)) {
QPRINTK(qdev, IFUP, ERR,
"Cannot re-enable PCI device after reset.\n");
return PCI_ERS_RESULT_DISCONNECT;
}
-
pci_set_master(pdev);
-
- netif_carrier_off(ndev);
- ql_adapter_reset(qdev);
-
- /* Make sure the EEPROM is good */
- memcpy(ndev->perm_addr, ndev->dev_addr, ndev->addr_len);
-
- if (!is_valid_ether_addr(ndev->perm_addr)) {
- QPRINTK(qdev, IFUP, ERR, "After reset, invalid MAC address.\n");
- return PCI_ERS_RESULT_DISCONNECT;
- }
-
return PCI_ERS_RESULT_RECOVERED;
}
@@ -4131,17 +4153,21 @@ static void qlge_io_resume(struct pci_dev *pdev)
{
struct net_device *ndev = pci_get_drvdata(pdev);
struct ql_adapter *qdev = netdev_priv(ndev);
+ int err = 0;
- pci_set_master(pdev);
-
+ if (ql_adapter_reset(qdev))
+ QPRINTK(qdev, DRV, ERR, "reset FAILED!\n");
if (netif_running(ndev)) {
- if (ql_adapter_up(qdev)) {
+ err = qlge_open(ndev);
+ if (err) {
QPRINTK(qdev, IFUP, ERR,
"Device initialization failed after reset.\n");
return;
}
+ } else {
+ QPRINTK(qdev, IFUP, ERR,
+ "Device was not running prior to EEH.\n");
}
-
netif_device_attach(ndev);
}
--
1.6.0.2
^ permalink raw reply related
* [net-2.6 PATCH 2/2] qlge: Fix firmware mailbox command timeout.
From: Ron Mercer @ 2009-10-28 18:39 UTC (permalink / raw)
To: davem; +Cc: netdev, ron.mercer
In-Reply-To: <1256755161-29606-1-git-send-email-ron.mercer@qlogic.com>
The mailbox command process would only process a maximum of 5 unrelated
firmware events while waiting for it's command completion status.
It should process an unlimited number of events while waiting for a maximum of 5 seconds.
Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
drivers/net/qlge/qlge.h | 1 +
drivers/net/qlge/qlge_mpi.c | 23 ++++++++++++-----------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index e7285f0..c2383ad 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -95,6 +95,7 @@ enum {
/* Misc. stuff */
MAILBOX_COUNT = 16,
+ MAILBOX_TIMEOUT = 5,
PROC_ADDR_RDY = (1 << 31),
PROC_ADDR_R = (1 << 30),
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index 99e58e3..bcf13c9 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -470,7 +470,8 @@ end:
*/
static int ql_mailbox_command(struct ql_adapter *qdev, struct mbox_params *mbcp)
{
- int status, count;
+ int status;
+ unsigned long count;
/* Begin polled mode for MPI */
@@ -491,9 +492,9 @@ static int ql_mailbox_command(struct ql_adapter *qdev, struct mbox_params *mbcp)
/* Wait for the command to complete. We loop
* here because some AEN might arrive while
* we're waiting for the mailbox command to
- * complete. If more than 5 arrive then we can
+ * complete. If more than 5 seconds expire we can
* assume something is wrong. */
- count = 5;
+ count = jiffies + HZ * MAILBOX_TIMEOUT;
do {
/* Wait for the interrupt to come in. */
status = ql_wait_mbx_cmd_cmplt(qdev);
@@ -517,15 +518,15 @@ static int ql_mailbox_command(struct ql_adapter *qdev, struct mbox_params *mbcp)
MB_CMD_STS_GOOD) ||
((mbcp->mbox_out[0] & 0x0000f000) ==
MB_CMD_STS_INTRMDT))
- break;
- } while (--count);
+ goto done;
+ } while (time_before(jiffies, count));
- if (!count) {
- QPRINTK(qdev, DRV, ERR,
- "Timed out waiting for mailbox complete.\n");
- status = -ETIMEDOUT;
- goto end;
- }
+ QPRINTK(qdev, DRV, ERR,
+ "Timed out waiting for mailbox complete.\n");
+ status = -ETIMEDOUT;
+ goto end;
+
+done:
/* Now we can clear the interrupt condition
* and look at our status.
--
1.6.0.2
^ permalink raw reply related
* Re: [PATCH] Multicast packet reassembly can fail
From: Steve Chen @ 2009-10-28 18:40 UTC (permalink / raw)
To: Rick Jones; +Cc: Mark Huth, netdev
In-Reply-To: <4AE88907.3030206@hp.com>
On Wed, 2009-10-28 at 11:10 -0700, Rick Jones wrote:
> >>If I understand correctly, the idea here is to say that when multiple interfaces
> >>receive fragments of copies of the same IP datagram that both copies will
> >>"survive" and flow up the stack?
> >>
> >>I'm basing that on your description, and an email from Steve that reads:
> >>
> >>
> >>>Actually, the patch tries to prevent packet drop for this exact
> >>>scenario. Please consider the following scenarios
> >>>1. Packet comes in the fragment reassemble code in the following order
> >>>(eth0 frag1), (eth0 frag2), (eth1 frag1), (eth1 frag2)
> >>>Packet from both interfaces get reassembled and gets further processed.
> >>>
> >>>2. Packet can some times arrive in (perhaps other orders as well)
> >>>(eth0 frag1), (eth1 frag1), (eth0 frag2), (eth1 frag2)
> >>>Without this patch, eth0 frag 1/2 are overwritten by eth1 frag1/2, and
> >>>packet from eth1 is dropped in the routing code.
> >>
> >>Doesn't that rather fly in the face of the weak-end-system model followed by Linux?
> >>
> >>I can see where scenario one leads to two IP datagrams making it up the stack,
> >>but I would have thought that was simply an "accident" of the situation that
> >>cannot reasonably be prevented, not justification to cause scenario two to send
> >>two datagrams up the stack.
> >
> >
> > For scenario 2, the routing code drops the 2nd packet. As a result, no
> > packet make it to the application. If someone is willing to suggest an
> > alternative, I can certainly rework the patch and retest.
>
> I'll ask my next potentially Emily Litella question - don't multicast IP
> applications bind to multicast IP addresses and not interfaces? That is to say,
> doesn't the first datagram completed get delivered to all applications on the
> host which have bound to the corresponding multicast IP (and port number...) ?
I actually don't know who Emily Litella is until today. This mailing
list is great not just for learning networking stuff :). In the test
code I received, one of the step to setup is to configure the IP address
of the interface that the application is expecting the packet. It
appears to bind on interface based on that casual observation. I'll
have to study the code in detail to be able to say for sure.
Regards,
Steve
^ permalink raw reply
* Re: [Fwd: Re: [PATCH] Multicast packet reassembly can fail]
From: Steve Chen @ 2009-10-28 18:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: mhuth, netdev
In-Reply-To: <4AE87ECD.7080408@gmail.com>
On Wed, 2009-10-28 at 18:26 +0100, Eric Dumazet wrote:
> Steve Chen a écrit :
> > On Wed, 2009-10-28 at 16:32 +0100, Eric Dumazet wrote:
> >> If each fragment is received twice on host, once by eth0, once by eth1,
> >> should we deliver datagram once or twice ?
> >
> > The application received it once. IIRC the duplicate packet is drop in
> > the routing code.
> >
> >> Once should be enough, even if in the non fragmented case, it will
> >> be delivered twice (kernel cannot detect duplicates, user app might do itself)
> >
> > Routing code drops the duplicate packet for none-fragmented case as
> > well.
>
> Really ? How so ? Receiving two copies of the same packet is legal.
I will have to double check exactly where the packet drop happens. I
thought it was somewhere in routing, but it could be in netfilter.
>
> >
> >>
> >>> For this specific case, src/dst address, protocol, IP ID and fragment
> >>> offset are all identical. The only difference is the ingress interface.
> >>> A good follow up question would be why would anyone in their right mind
> >>> multicast to the same destination? well, I don't know. I can not get
> >>> the people who reported the problem to tell me either. Since someone
> >>> found the need to do this, perhaps others may find it useful too.
> >>>
> >> Then, if a 2000 bytes message is fragmented in two packets, one coming
> >> from eth0, one coming from eth1, I suspect your patch drops the message,
> >> unless eth0/eth1 are part of a bonding device...
> >
> > Actually, the patch tries to prevent packet drop for this exact
> > scenario. Please consider the following scenarios
> > 1. Packet comes in the fragment reassemble code in the following order
> > (eth0 frag1), (eth0 frag2), (eth1 frag1), (eth1 frag2)
> > Packet from both interfaces get reassembled and gets further processed.
>
> Yes your patch does this, so each multicast application receives two copies of the
> same datagram.
>
> >
> > 2. Packet can some times arrive in (perhaps other orders as well)
> > (eth0 frag1), (eth1 frag1), (eth0 frag2), (eth1 frag2)
> > Without this patch, eth0 frag 1/2 are overwritten by eth1 frag1/2, and
> > packet from eth1 is dropped in the routing code.
>
> Really ? how so ? I dont see how it can happen, unless you use RPF ?
>
> current situation should be :
>
> (eth0 frag1) : We create a context, store frag1 in it
> (eth1 frag1) : We find this context, and drop frag1 since we already have the data
> (maybe the bug is here, if we cannot cope with a duplicate ?)
> (eth0 frag2) : We find this context, store frag2 -> complete datagram and deliver it
> (eth1 frag2) : We find context, drop frag2 since datagram was completed.
Yes, this is exactly what is happening in the current code.
>
> (or maybe we create a new context that will timeout later, maybe this is your problem ?)
>
> Net effect : We deliver the datagram correctly.
>
>
> >
> >> That would break common routing setups, using two links to aggregate bandwidth ?
> >
> > I don't believe it would. The aggregate bandwidth will work the same as
> > before. The attributes (src/dst addr, protocol, interface, etc.) should
> > generate a unique hash key. If hash collision should happen with the
> > addition of iif << 5, the code still compare the original src addr along
> > with interface number, so there should be no issues.
>
> What about the obvious :
>
> (eth0 frag1), (eth1 frag2)
>
> Your patch creates two contexts since hashes are different,
> that will timeout and no packet delivered at all
>
I see the point you are making. I assumed, probably incorrectly, that
since eth0 and eth1 have different IP address. I would get a complete
series of fragments for each interface. Perhaps, I should really be
looking up the stack to see why packets were dropped. Please correct me
if I'm mistaken. The normal behavior is that application should be
receiving either 2 (scenario 1) or 1 (scenario 2) packets.
Regards,
Steve
^ permalink raw reply
* Re: wanPMC-CxT1E1
From: Greg KH @ 2009-10-28 18:11 UTC (permalink / raw)
To: Bob Beers; +Cc: netdev
In-Reply-To: <4f6ba3b0910271048n10ff37fek9af191b133892e1e@mail.gmail.com>
On Tue, Oct 27, 2009 at 01:48:53PM -0400, Bob Beers wrote:
> On Mon, Oct 26, 2009 at 4:41 PM, Greg KH <greg@kroah.com> wrote:
> > Getting it to build on 2.6.31 is more important than RHEL5, we can't do
> > anything with an old kernel like that.
>
> ok, so where do I start, I have a system ready to start
> git cloning, and creating patches. I googled for a while
> but didn't find a nice recipe for participating in the -staging
> process.
I'll try to get it to build in the kernel tree today and send you a copy
of the patch to try out.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] AF_RAW: Augment raw_send_hdrinc to expand skb to fit iphdr->ihl
From: Eric Dumazet @ 2009-10-28 18:13 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, davem
In-Reply-To: <20091028173955.GB7422@hmsreliant.think-freely.org>
Neil Horman a écrit :
> Augment raw_send_hdrinc to correct for incorrect ip header length values
>
> A series of oopses was reported to me recently. Apparently when using AF_RAW
> sockets to send data to peers that were reachable via ipsec encapsulation,
> people could panic or BUG halt their systems.
>
> I've tracked the problem down to user space sending an invalid ip header over an
> AF_RAW socket with IP_HDRINCL set to 1.
>
> Basically what happens is that userspace sends down an ip frame that includes
> only the header (no data), but sets the ip header ihl value to a large number,
> one that is larger than the total amount of data passed to the sendmsg call. In
> raw_send_hdrincl, we allocate an skb based on the size of the data in the msghdr
> that was passed in, but assume the data is all valid. Later during ipsec
> encapsulation, xfrm4_tranport_output moves the entire frame back in the skbuff
> to provide headroom for the ipsec headers. During this operation, the
> skb->transport_header is repointed to a spot computed by
> skb->network_header + the ip header length (ihl). Since so little data was
> passed in relative to the value of ihl provided by the raw socket, we point
> transport header to an unknown location, resulting in various crashes.
>
> So, what to do about this? My first thought was to simply return -EINVAL, and
> let user space sort it out. I'm still thinking that might be the best way, but
> I thought I'd try this first, just in case someone has reason to try to
> send such a bogus frame through the kernel. This solution simply checks the
> value of ihl in raw_send_hdrinc and expands the skb to fit, filling the new
> space with IPOPT_NOOP options. I've confirmed that it fixes the crashes that
> were reported.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>
Thanks a lot for this detailed info, I wish everything could be explained like this !
I believe we should drop the request, since padding it is not what was expected by user.
^ 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