* pull request: wireless-2.6 2009-11-02
From: John W. Linville @ 2009-11-02 21:40 UTC (permalink / raw)
To: davem; +Cc: linux-wireless, netdev, linux-kernel
Dave,
Another collection of fixes intended for 2.6.32...several
(almost-)one-liners, a b43 bounce-buffer fix, and a couple of USB IDs.
There are several from Johannes -- I'm sure there was sake involved... :-)
Please let me know if there are problems!
John
---
Individual patches are available here:
http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/
---
The following changes since commit 63ca2d74ea4f9c7a7ac082c915609a7b224908e7:
Ken Kawasaki (1):
pcnet_cs: add cis of PreMax PE-200 ethernet pcmcia card
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master
David Woodhouse (1):
libertas if_usb: Fix crash on 64-bit machines
Johannes Berg (5):
mac80211: fix BSS leak
mac80211: fix addba timer
mac80211: fix reason code output endianness
cfg80211: fix NULL ptr deref
mac80211: check interface is down before type change
Luis R. Rodriguez (1):
ath9k: fix misplaced semicolon on rate control
Michael Buesch (1):
b43: Fix DMA TX bounce buffer copying
Xose Vazquez Perez (1):
rt73usb.c : more ids
Zhu Yi (1):
ipw2200: fix oops on missing firmware
drivers/net/wireless/ath/ath9k/rc.c | 2 +-
drivers/net/wireless/b43/dma.c | 15 +++++++++++++--
drivers/net/wireless/ipw2x00/ipw2100.c | 5 ++++-
drivers/net/wireless/ipw2x00/ipw2200.c | 2 ++
drivers/net/wireless/ipw2x00/libipw.h | 1 +
drivers/net/wireless/ipw2x00/libipw_module.c | 14 +++++++++-----
drivers/net/wireless/libertas/if_usb.c | 2 +-
drivers/net/wireless/rt2x00/rt73usb.c | 5 +++++
net/mac80211/agg-tx.c | 19 ++++++++++++-------
net/mac80211/cfg.c | 6 +++---
net/mac80211/ht.c | 2 +-
net/mac80211/ibss.c | 6 ++++--
net/wireless/sme.c | 7 +++++--
13 files changed, 61 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 16a2717..1895d63 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -679,7 +679,7 @@ static u8 ath_rc_get_highest_rix(struct ath_softc *sc,
return rate;
if (rate_table->info[rate].valid_single_stream &&
- !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG));
+ !(ath_rc_priv->ht_cap & WLAN_RC_DS_FLAG))
return rate;
/* This should not happen */
diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
index 8701034..de4e804 100644
--- a/drivers/net/wireless/b43/dma.c
+++ b/drivers/net/wireless/b43/dma.c
@@ -1157,8 +1157,9 @@ struct b43_dmaring *parse_cookie(struct b43_wldev *dev, u16 cookie, int *slot)
}
static int dma_tx_fragment(struct b43_dmaring *ring,
- struct sk_buff *skb)
+ struct sk_buff **in_skb)
{
+ struct sk_buff *skb = *in_skb;
const struct b43_dma_ops *ops = ring->ops;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
u8 *header;
@@ -1224,8 +1225,14 @@ static int dma_tx_fragment(struct b43_dmaring *ring,
}
memcpy(skb_put(bounce_skb, skb->len), skb->data, skb->len);
+ memcpy(bounce_skb->cb, skb->cb, sizeof(skb->cb));
+ bounce_skb->dev = skb->dev;
+ skb_set_queue_mapping(bounce_skb, skb_get_queue_mapping(skb));
+ info = IEEE80211_SKB_CB(bounce_skb);
+
dev_kfree_skb_any(skb);
skb = bounce_skb;
+ *in_skb = bounce_skb;
meta->skb = skb;
meta->dmaaddr = map_descbuffer(ring, skb->data, skb->len, 1);
if (b43_dma_mapping_error(ring, meta->dmaaddr, skb->len, 1)) {
@@ -1355,7 +1362,11 @@ int b43_dma_tx(struct b43_wldev *dev, struct sk_buff *skb)
* static, so we don't need to store it per frame. */
ring->queue_prio = skb_get_queue_mapping(skb);
- err = dma_tx_fragment(ring, skb);
+ /* dma_tx_fragment might reallocate the skb, so invalidate pointers pointing
+ * into the skb data or cb now. */
+ hdr = NULL;
+ info = NULL;
+ err = dma_tx_fragment(ring, &skb);
if (unlikely(err == -ENOKEY)) {
/* Drop this packet, as we don't have the encryption key
* anymore and must not transmit it unencrypted. */
diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
index 240cff1..a741d37 100644
--- a/drivers/net/wireless/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/ipw2x00/ipw2100.c
@@ -6325,8 +6325,10 @@ static int ipw2100_pci_init_one(struct pci_dev *pci_dev,
fail:
if (dev) {
- if (registered)
+ if (registered) {
+ unregister_ieee80211(priv->ieee);
unregister_netdev(dev);
+ }
ipw2100_hw_stop_adapter(priv);
@@ -6383,6 +6385,7 @@ static void __devexit ipw2100_pci_remove_one(struct pci_dev *pci_dev)
/* Unregister the device first - this results in close()
* being called if the device is open. If we free storage
* first, then close() will crash. */
+ unregister_ieee80211(priv->ieee);
unregister_netdev(dev);
/* ipw2100_down will ensure that there is no more pending work
diff --git a/drivers/net/wireless/ipw2x00/ipw2200.c b/drivers/net/wireless/ipw2x00/ipw2200.c
index 8d58e6e..04341a2 100644
--- a/drivers/net/wireless/ipw2x00/ipw2200.c
+++ b/drivers/net/wireless/ipw2x00/ipw2200.c
@@ -11821,6 +11821,7 @@ static int __devinit ipw_pci_probe(struct pci_dev *pdev,
if (err) {
IPW_ERROR("Failed to register promiscuous network "
"device (error %d).\n", err);
+ unregister_ieee80211(priv->ieee);
unregister_netdev(priv->net_dev);
goto out_remove_sysfs;
}
@@ -11871,6 +11872,7 @@ static void __devexit ipw_pci_remove(struct pci_dev *pdev)
mutex_unlock(&priv->mutex);
+ unregister_ieee80211(priv->ieee);
unregister_netdev(priv->net_dev);
if (priv->rxq) {
diff --git a/drivers/net/wireless/ipw2x00/libipw.h b/drivers/net/wireless/ipw2x00/libipw.h
index bf45391..f42ade6 100644
--- a/drivers/net/wireless/ipw2x00/libipw.h
+++ b/drivers/net/wireless/ipw2x00/libipw.h
@@ -1020,6 +1020,7 @@ static inline int libipw_is_cck_rate(u8 rate)
/* ieee80211.c */
extern void free_ieee80211(struct net_device *dev, int monitor);
extern struct net_device *alloc_ieee80211(int sizeof_priv, int monitor);
+extern void unregister_ieee80211(struct libipw_device *ieee);
extern int libipw_change_mtu(struct net_device *dev, int new_mtu);
extern void libipw_networks_age(struct libipw_device *ieee,
diff --git a/drivers/net/wireless/ipw2x00/libipw_module.c b/drivers/net/wireless/ipw2x00/libipw_module.c
index a0e9f6a..be5b809 100644
--- a/drivers/net/wireless/ipw2x00/libipw_module.c
+++ b/drivers/net/wireless/ipw2x00/libipw_module.c
@@ -235,16 +235,19 @@ void free_ieee80211(struct net_device *dev, int monitor)
libipw_networks_free(ieee);
/* free cfg80211 resources */
- if (!monitor) {
- wiphy_unregister(ieee->wdev.wiphy);
- kfree(ieee->a_band.channels);
- kfree(ieee->bg_band.channels);
+ if (!monitor)
wiphy_free(ieee->wdev.wiphy);
- }
free_netdev(dev);
}
+void unregister_ieee80211(struct libipw_device *ieee)
+{
+ wiphy_unregister(ieee->wdev.wiphy);
+ kfree(ieee->a_band.channels);
+ kfree(ieee->bg_band.channels);
+}
+
#ifdef CONFIG_LIBIPW_DEBUG
static int debug = 0;
@@ -330,3 +333,4 @@ module_init(libipw_init);
EXPORT_SYMBOL(alloc_ieee80211);
EXPORT_SYMBOL(free_ieee80211);
+EXPORT_SYMBOL(unregister_ieee80211);
diff --git a/drivers/net/wireless/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c
index 92bc8c5..3fac4ef 100644
--- a/drivers/net/wireless/libertas/if_usb.c
+++ b/drivers/net/wireless/libertas/if_usb.c
@@ -508,7 +508,7 @@ static int __if_usb_submit_rx_urb(struct if_usb_card *cardp,
/* Fill the receive configuration URB and initialise the Rx call back */
usb_fill_bulk_urb(cardp->rx_urb, cardp->udev,
usb_rcvbulkpipe(cardp->udev, cardp->ep_in),
- (void *) (skb->tail + (size_t) IPFIELD_ALIGN_OFFSET),
+ skb->data + IPFIELD_ALIGN_OFFSET,
MRVDRV_ETH_RX_PACKET_BUFFER_SIZE, callbackfn,
cardp);
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index b8f5ee3..14e7bb2 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -2389,10 +2389,13 @@ static struct usb_device_id rt73usb_device_table[] = {
{ USB_DEVICE(0x13b1, 0x0023), USB_DEVICE_DATA(&rt73usb_ops) },
{ USB_DEVICE(0x13b1, 0x0028), USB_DEVICE_DATA(&rt73usb_ops) },
/* MSI */
+ { USB_DEVICE(0x0db0, 0x4600), USB_DEVICE_DATA(&rt73usb_ops) },
{ USB_DEVICE(0x0db0, 0x6877), USB_DEVICE_DATA(&rt73usb_ops) },
{ USB_DEVICE(0x0db0, 0x6874), USB_DEVICE_DATA(&rt73usb_ops) },
{ USB_DEVICE(0x0db0, 0xa861), USB_DEVICE_DATA(&rt73usb_ops) },
{ USB_DEVICE(0x0db0, 0xa874), USB_DEVICE_DATA(&rt73usb_ops) },
+ /* Ovislink */
+ { USB_DEVICE(0x1b75, 0x7318), USB_DEVICE_DATA(&rt73usb_ops) },
/* Ralink */
{ USB_DEVICE(0x04bb, 0x093d), USB_DEVICE_DATA(&rt73usb_ops) },
{ USB_DEVICE(0x148f, 0x2573), USB_DEVICE_DATA(&rt73usb_ops) },
@@ -2420,6 +2423,8 @@ static struct usb_device_id rt73usb_device_table[] = {
/* Planex */
{ USB_DEVICE(0x2019, 0xab01), USB_DEVICE_DATA(&rt73usb_ops) },
{ USB_DEVICE(0x2019, 0xab50), USB_DEVICE_DATA(&rt73usb_ops) },
+ /* WideTell */
+ { USB_DEVICE(0x7167, 0x3840), USB_DEVICE_DATA(&rt73usb_ops) },
/* Zcom */
{ USB_DEVICE(0x0cde, 0x001c), USB_DEVICE_DATA(&rt73usb_ops) },
/* ZyXEL */
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index bd765f3..b09948c 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -666,26 +666,25 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local,
state = &sta->ampdu_mlme.tid_state_tx[tid];
+ del_timer_sync(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer);
+
spin_lock_bh(&sta->lock);
- if (!(*state & HT_ADDBA_REQUESTED_MSK)) {
- spin_unlock_bh(&sta->lock);
- return;
- }
+ if (!(*state & HT_ADDBA_REQUESTED_MSK))
+ goto timer_still_needed;
if (mgmt->u.action.u.addba_resp.dialog_token !=
sta->ampdu_mlme.tid_tx[tid]->dialog_token) {
- spin_unlock_bh(&sta->lock);
#ifdef CONFIG_MAC80211_HT_DEBUG
printk(KERN_DEBUG "wrong addBA response token, tid %d\n", tid);
#endif /* CONFIG_MAC80211_HT_DEBUG */
- return;
+ goto timer_still_needed;
}
- del_timer_sync(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer);
#ifdef CONFIG_MAC80211_HT_DEBUG
printk(KERN_DEBUG "switched off addBA timer for tid %d \n", tid);
#endif /* CONFIG_MAC80211_HT_DEBUG */
+
if (le16_to_cpu(mgmt->u.action.u.addba_resp.status)
== WLAN_STATUS_SUCCESS) {
u8 curstate = *state;
@@ -699,5 +698,11 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local,
} else {
___ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_INITIATOR);
}
+
+ goto out;
+
+ timer_still_needed:
+ add_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer);
+ out:
spin_unlock_bh(&sta->lock);
}
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 5608f6c..7b5131b 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -72,6 +72,9 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
struct ieee80211_sub_if_data *sdata;
int ret;
+ if (netif_running(dev))
+ return -EBUSY;
+
if (!nl80211_type_check(type))
return -EINVAL;
@@ -81,9 +84,6 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
if (ret)
return ret;
- if (netif_running(sdata->dev))
- return -EBUSY;
-
if (ieee80211_vif_is_mesh(&sdata->vif) && params->mesh_id_len)
ieee80211_sdata_set_mesh_id(sdata,
params->mesh_id_len,
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index 0891bfb..48ef1a2 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -153,7 +153,7 @@ void ieee80211_process_delba(struct ieee80211_sub_if_data *sdata,
if (net_ratelimit())
printk(KERN_DEBUG "delba from %pM (%s) tid %d reason code %d\n",
mgmt->sa, initiator ? "initiator" : "recipient", tid,
- mgmt->u.action.u.delba.reason_code);
+ le16_to_cpu(mgmt->u.action.u.delba.reason_code));
#endif /* CONFIG_MAC80211_HT_DEBUG */
if (initiator == WLAN_BACK_INITIATOR)
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index ca8ecce..f1362f3 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -73,6 +73,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
struct ieee80211_mgmt *mgmt;
u8 *pos;
struct ieee80211_supported_band *sband;
+ struct cfg80211_bss *bss;
u32 bss_change;
u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
@@ -177,8 +178,9 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
mod_timer(&ifibss->timer,
round_jiffies(jiffies + IEEE80211_IBSS_MERGE_INTERVAL));
- cfg80211_inform_bss_frame(local->hw.wiphy, local->hw.conf.channel,
- mgmt, skb->len, 0, GFP_KERNEL);
+ bss = cfg80211_inform_bss_frame(local->hw.wiphy, local->hw.conf.channel,
+ mgmt, skb->len, 0, GFP_KERNEL);
+ cfg80211_put_bss(bss);
cfg80211_ibss_joined(sdata->dev, ifibss->bssid, GFP_KERNEL);
}
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index ece378d..9f0b280 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -165,7 +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];
+ u8 bssid_buf[ETH_ALEN], *bssid = NULL;
rtnl_lock();
cfg80211_lock_rdev(rdev);
@@ -181,7 +181,10 @@ void cfg80211_conn_work(struct work_struct *work)
wdev_unlock(wdev);
continue;
}
- memcpy(bssid, wdev->conn->params.bssid, ETH_ALEN);
+ if (wdev->conn->params.bssid) {
+ memcpy(bssid_buf, wdev->conn->params.bssid, ETH_ALEN);
+ bssid = bssid_buf;
+ }
if (cfg80211_conn_do_work(wdev))
__cfg80211_connect_result(
wdev->netdev, bssid,
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply related
* Re: wanPMC-CxT1E1
From: Bob Beers @ 2009-11-02 21:39 UTC (permalink / raw)
To: Greg KH; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <4f6ba3b0911021335k3054da38x56fe72ac7ea8781c@mail.gmail.com>
On Mon, Nov 2, 2009 at 4:35 PM, Bob Beers <bob.beers@gmail.com> wrote:
> I should be able to fix those things an generate another patch file,
> but my git skills are very weak, so I guess you have the options:
> fix it up by hand, or teach me a little more git, so I can submit
> a better patch.
Here is patch on top of previous patch to fix the two mistakes:
diff --git a/drivers/net/wan/Makefile b/drivers/net/wan/Makefile
index ebc96c7..19d14bc 100644
--- a/drivers/net/wan/Makefile
+++ b/drivers/net/wan/Makefile
@@ -29,7 +29,6 @@ obj-$(CONFIG_DSCC4) += dscc4.o
obj-$(CONFIG_X25_ASY) += x25_asy.o
obj-$(CONFIG_LANMEDIA) += lmc/
-obj-$(CONFIG_CXT1E1) += cxt1e1/
obj-$(CONFIG_DLCI) += dlci.o
obj-$(CONFIG_SDLA) += sdla.o
diff --git a/drivers/staging/cxt1e1/Makefile b/drivers/staging/cxt1e1/Makefile
index edfad71..10020d7 100644
--- a/drivers/staging/cxt1e1/Makefile
+++ b/drivers/staging/cxt1e1/Makefile
@@ -1,4 +1,3 @@
-CONFIG_CXT1E1 = m
obj-$(CONFIG_CXT1E1) += cxt1e1.o
EXTRA_CFLAGS += -DSBE_PMCC4_ENABLE
thanks,
--
-Bob Beers
^ permalink raw reply related
* Re: wanPMC-CxT1E1
From: Bob Beers @ 2009-11-02 21:35 UTC (permalink / raw)
To: Greg KH; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <4f6ba3b0911021316v16ff7431s4dfa939743541df6@mail.gmail.com>
I just remembered that I have another mistake in the patch that I'd
like to confess: I put a 'CONFIG_CXT1E1 = m" at the top
of the staging/cxt1e1/Makefile.
I should be able to fix those things an generate another patch file,
but my git skills are very weak, so I guess you have the options:
fix it up by hand, or teach me a little more git, so I can submit
a better patch.
thanks,
--
-Bob Beers
^ permalink raw reply
* Re: [PATCH net-next-2.6] ifb: RCU locking avoids touching dev refcount
From: Eric Dumazet @ 2009-11-02 21:34 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AEF4B9B.7000205@gmail.com>
Jarek Poplawski a écrit :
> Eric Dumazet wrote, On 11/02/2009 06:45 AM:
>
>> Avoids touching dev refcount in hotpath
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> ---
>> drivers/net/ifb.c | 6 ++++--
>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
>> index 030913f..69c2566 100644
>> --- a/drivers/net/ifb.c
>> +++ b/drivers/net/ifb.c
>> @@ -98,13 +98,15 @@ static void ri_tasklet(unsigned long dev)
>> stats->tx_packets++;
>> stats->tx_bytes +=skb->len;
>>
>> - skb->dev = dev_get_by_index(&init_net, skb->iif);
>> + rcu_read_lock();
>> + skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
>> if (!skb->dev) {
>> + rcu_read_unlock();
>> dev_kfree_skb(skb);
>> stats->tx_dropped++;
>> break;
>> }
>> - dev_put(skb->dev);
>> + rcu_read_unlock();
>
> I wonder if this rcu_read_unlock() isn't too early here. I know, it
> functionally fully replaces the old method, but as a whole it looks
> strange:
>
>> rcu_read_lock();
>> skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
>> if (!skb->dev) {
>> rcu_read_unlock();
>> dev_kfree_skb(skb);
>> stats->tx_dropped++;
>> break;
>> }
>> rcu_read_unlock();
>> skb->iif = _dev->ifindex;
>>
>> if (from & AT_EGRESS) {
>> dp->st_rx_frm_egr++;
>> dev_queue_xmit(skb);
>> } else if (from & AT_INGRESS) {
>> dp->st_rx_frm_ing++;
>> skb_pull(skb, skb->dev->hard_header_len);
>
>
> So, how is skb->dev protected here, above and below? It seems these
> rcu read blocks need extending, don't they?
>
Well, this might be true, but we run under tasklet (softirq) with preemption disabled.
We might move rcu_read_unlock() some lines down to not rely on this.
^ permalink raw reply
* Re: [PATCH net-next-2.6] ifb: RCU locking avoids touching dev refcount
From: Jarek Poplawski @ 2009-11-02 21:14 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List
In-Reply-To: <4AEE71EC.7040208@gmail.com>
Eric Dumazet wrote, On 11/02/2009 06:45 AM:
> Avoids touching dev refcount in hotpath
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> drivers/net/ifb.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
> index 030913f..69c2566 100644
> --- a/drivers/net/ifb.c
> +++ b/drivers/net/ifb.c
> @@ -98,13 +98,15 @@ static void ri_tasklet(unsigned long dev)
> stats->tx_packets++;
> stats->tx_bytes +=skb->len;
>
> - skb->dev = dev_get_by_index(&init_net, skb->iif);
> + rcu_read_lock();
> + skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
> if (!skb->dev) {
> + rcu_read_unlock();
> dev_kfree_skb(skb);
> stats->tx_dropped++;
> break;
> }
> - dev_put(skb->dev);
> + rcu_read_unlock();
I wonder if this rcu_read_unlock() isn't too early here. I know, it
functionally fully replaces the old method, but as a whole it looks
strange:
> rcu_read_lock();
> skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
> if (!skb->dev) {
> rcu_read_unlock();
> dev_kfree_skb(skb);
> stats->tx_dropped++;
> break;
> }
> rcu_read_unlock();
> skb->iif = _dev->ifindex;
>
> if (from & AT_EGRESS) {
> dp->st_rx_frm_egr++;
> dev_queue_xmit(skb);
> } else if (from & AT_INGRESS) {
> dp->st_rx_frm_ing++;
> skb_pull(skb, skb->dev->hard_header_len);
So, how is skb->dev protected here, above and below? It seems these
rcu read blocks need extending, don't they?
Jarek P.
> netif_rx(skb);
> } else
> BUG();
> }
^ permalink raw reply
* Re: wanPMC-CxT1E1
From: Greg KH @ 2009-11-02 20:59 UTC (permalink / raw)
To: Bob Beers; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <4f6ba3b0911021241gb6d9684ga76228013e0f5f14@mail.gmail.com>
On Mon, Nov 02, 2009 at 03:41:44PM -0500, Bob Beers wrote:
> On Thu, Oct 29, 2009 at 8:52 PM, Krzysztof Halasa <khc@pm.waw.pl> wrote:
> > Bob Beers <bob.beers@gmail.com> writes:
> >
> >> 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 gave it a try. At least compiles with few warnings. Not sure about the
> > WORK_INIT() change.
> >
> > Created drivers/net/wan/cxt1e1, moved all relevant SBE's .c and .h
> > there, added a simple Makefile/Kconfig. Quick and dirty. There is a
> > _lot_ of work to be done before it meets the usual kernel standards.
> >
> > It's not in staging/ so the paths need to be corrected but I can't work
> > further on it at this time. Hand-edited but I tried to be careful.
> >
> <snip Krzysztof's diff file>
>
> Ok, I took a shot at it too, It compiles for me and generates the
> cxt1e1.ko file.
> I have made a git clone of kernel, added the directory cxt1e1 under staging,
> added a Kconfig, Makefile, and all the *.[ch] from the tarball's driver/ and
> include/ directories, called git add on cxt1e1 directory and the files
> mentioned above. In the cxt1e1 directory I modified the *.[ch] files as I saw
> fit, and using many hints from Krzysztof
> (I'm also not sure about the changes to INIT_WORK).
> I modified Makefile in staging/ and ran 'git -a commit'.
>
> Here is top part of output from 'git log --stat':
>
> commit d54e08030785153c8c0f4eb4f1cf320d60fff286
> Author: Bob Beers <bob.beers@gmail.com>
> Date: Mon Nov 2 15:06:19 2009 -0500
>
> Add CXT1E1 [1,2,4] channel wan card driver.
>
> drivers/net/wan/Makefile | 1 +
This file shouldn't need to be modified. But I can fix that up by hand.
> drivers/staging/Kconfig | 2 +
> drivers/staging/Makefile | 1 +
> drivers/staging/cxt1e1/Kconfig | 22 +
> drivers/staging/cxt1e1/Makefile | 20 +
> drivers/staging/cxt1e1/comet.c | 566 +++++++
> drivers/staging/cxt1e1/comet.h | 366 +++++
> drivers/staging/cxt1e1/comet_tables.c | 561 +++++++
> drivers/staging/cxt1e1/comet_tables.h | 85 +
> drivers/staging/cxt1e1/functions.c | 366 +++++
> drivers/staging/cxt1e1/hwprobe.c | 400 +++++
> drivers/staging/cxt1e1/libsbew.h | 581 +++++++
> drivers/staging/cxt1e1/linux.c | 1354 ++++++++++++++++
> drivers/staging/cxt1e1/musycc.c | 2180 ++++++++++++++++++++++++++
> drivers/staging/cxt1e1/musycc.h | 460 ++++++
> drivers/staging/cxt1e1/ossiRelease.c | 39 +
> drivers/staging/cxt1e1/pmc93x6_eeprom.c | 559 +++++++
> drivers/staging/cxt1e1/pmc93x6_eeprom.h | 60 +
> drivers/staging/cxt1e1/pmcc4.h | 155 ++
> drivers/staging/cxt1e1/pmcc4_cpld.h | 124 ++
> drivers/staging/cxt1e1/pmcc4_defs.h | 82 +
> drivers/staging/cxt1e1/pmcc4_drv.c | 1855 ++++++++++++++++++++++
> drivers/staging/cxt1e1/pmcc4_ioctls.h | 81 +
> drivers/staging/cxt1e1/pmcc4_private.h | 295 ++++
> drivers/staging/cxt1e1/pmcc4_sysdep.h | 62 +
> drivers/staging/cxt1e1/sbe_bid.h | 61 +
> drivers/staging/cxt1e1/sbe_promformat.h | 157 ++
> drivers/staging/cxt1e1/sbecom_inline_linux.h | 310 ++++
> drivers/staging/cxt1e1/sbecrc.c | 137 ++
> drivers/staging/cxt1e1/sbeid.c | 217 +++
> drivers/staging/cxt1e1/sbeproc.c | 358 +++++
> drivers/staging/cxt1e1/sbeproc.h | 52 +
> drivers/staging/cxt1e1/sbew_ioc.h | 136 ++
> 33 files changed, 11705 insertions(+), 0 deletions(-)
>
> How do I generate the patch file?
git show --pretty=email d54e08030785153c8c0f4eb4f1cf320d60fff286 > my_patch.patch
will do it.
thanks,
greg k-h
^ permalink raw reply
* Re: HTB accuracy on 10GbE
From: Stephen Hemminger @ 2009-11-02 20:53 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Ryousei Takano, Linux Netdev List, takano-ryousei
In-Reply-To: <4AEEFE2E.7090706@trash.net>
On Mon, 02 Nov 2009 16:43:42 +0100
Patrick McHardy <kaber@trash.net> wrote:
> Ryousei Takano wrote:
> > Hi Stephen and all,
> >
> > I have observed a HTB accuracy problem on the Linux kernel 2.6.30 and
> > the Myri-10G 10 GbE NIC.
> > HTB can control the transmission rate at Gigabit speed, however it can
> > not work well at 10 Gigabit speed.
> >
> > I asked Stephen this problem at Japan Linux Symposium. He mentioned a
> > HTB bug related to the timer granularity.
> > I want to know what is happen, and what should be do for fixing it.
> >
> > Any comments and suggestions will be welcome.
> >
> > For more detail, please see the following page:
> > http://code.google.com/p/pspacer/wiki/HTBon10GbE
>
> This is not an easy problem to fix. Userspace, the kernel and the
> netlink API use 32 bit for timing related values, which is too small
> to use more than microsecond resolution. All of them need to be
> converted to use bigger types, additionally some kind of compatibility
> handling to deal with old iproute versions still using microsecond
> resolution is required.
The existing API is a legacy mish-mash. The field is limited to 32 bits,
but it might be possible to use a finer scale.
Maybe if kernel advertised finer resolution through /proc/net/psched
then table could be finer grained. This would maintain compatibility
between kernel and user space. You would need to have new kernel and
new iproute to get nanosecond resolution but older combinations would
still work.
The downside is that by using nanosecond resolution the rates are upper
bounded at 4.2seconds / packet.
^ permalink raw reply
* [PATCH] trivial: remove duplicated MIN macro from tehuti.
From: Thiago Farina @ 2009-11-02 20:45 UTC (permalink / raw)
To: trivial
Cc: baum, andy, davem, shemminger, apkm, ben, yanghy, netdev,
linux-kernel, Thiago Farina
Since the kernel api already has the macro "min",
just use it instead of declaring another one.
Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
drivers/net/tehuti.c | 4 ++--
drivers/net/tehuti.h | 2 --
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tehuti.c b/drivers/net/tehuti.c
index ec9dfb2..8d116a9 100644
--- a/drivers/net/tehuti.c
+++ b/drivers/net/tehuti.c
@@ -1878,7 +1878,7 @@ static void bdx_tx_push_desc_safe(struct bdx_priv *priv, void *data, int size)
udelay(50); /* give hw a chance to clean fifo */
continue;
}
- avail = MIN(avail, size);
+ avail = min(avail, size);
DBG("about to push %d bytes starting %p size %d\n", avail,
data, size);
bdx_tx_push_desc(priv, data, avail);
@@ -1889,7 +1889,7 @@ static void bdx_tx_push_desc_safe(struct bdx_priv *priv, void *data, int size)
}
static const struct net_device_ops bdx_netdev_ops = {
- .ndo_open = bdx_open,
+ .ndo_open = bdx_open,
.ndo_stop = bdx_close,
.ndo_start_xmit = bdx_tx_transmit,
.ndo_validate_addr = eth_validate_addr,
diff --git a/drivers/net/tehuti.h b/drivers/net/tehuti.h
index 4fc875e..1241419 100644
--- a/drivers/net/tehuti.h
+++ b/drivers/net/tehuti.h
@@ -76,8 +76,6 @@
#define FIFO_SIZE 4096
#define FIFO_EXTRA_SPACE 1024
-#define MIN(x, y) ((x) < (y) ? (x) : (y))
-
#if BITS_PER_LONG == 64
# define H32_64(x) (u32) ((u64)(x) >> 32)
# define L32_64(x) (u32) ((u64)(x) & 0xffffffff)
--
1.6.5.1.61.ge79999
^ permalink raw reply related
* Re: wanPMC-CxT1E1
From: Bob Beers @ 2009-11-02 20:41 UTC (permalink / raw)
To: netdev; +Cc: Greg KH, Krzysztof Halasa
In-Reply-To: <m3d4454phz.fsf@intrepid.localdomain>
On Thu, Oct 29, 2009 at 8:52 PM, Krzysztof Halasa <khc@pm.waw.pl> wrote:
> Bob Beers <bob.beers@gmail.com> writes:
>
>> 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 gave it a try. At least compiles with few warnings. Not sure about the
> WORK_INIT() change.
>
> Created drivers/net/wan/cxt1e1, moved all relevant SBE's .c and .h
> there, added a simple Makefile/Kconfig. Quick and dirty. There is a
> _lot_ of work to be done before it meets the usual kernel standards.
>
> It's not in staging/ so the paths need to be corrected but I can't work
> further on it at this time. Hand-edited but I tried to be careful.
>
<snip Krzysztof's diff file>
Ok, I took a shot at it too, It compiles for me and generates the
cxt1e1.ko file.
I have made a git clone of kernel, added the directory cxt1e1 under staging,
added a Kconfig, Makefile, and all the *.[ch] from the tarball's driver/ and
include/ directories, called git add on cxt1e1 directory and the files
mentioned above. In the cxt1e1 directory I modified the *.[ch] files as I saw
fit, and using many hints from Krzysztof
(I'm also not sure about the changes to INIT_WORK).
I modified Makefile in staging/ and ran 'git -a commit'.
Here is top part of output from 'git log --stat':
commit d54e08030785153c8c0f4eb4f1cf320d60fff286
Author: Bob Beers <bob.beers@gmail.com>
Date: Mon Nov 2 15:06:19 2009 -0500
Add CXT1E1 [1,2,4] channel wan card driver.
drivers/net/wan/Makefile | 1 +
drivers/staging/Kconfig | 2 +
drivers/staging/Makefile | 1 +
drivers/staging/cxt1e1/Kconfig | 22 +
drivers/staging/cxt1e1/Makefile | 20 +
drivers/staging/cxt1e1/comet.c | 566 +++++++
drivers/staging/cxt1e1/comet.h | 366 +++++
drivers/staging/cxt1e1/comet_tables.c | 561 +++++++
drivers/staging/cxt1e1/comet_tables.h | 85 +
drivers/staging/cxt1e1/functions.c | 366 +++++
drivers/staging/cxt1e1/hwprobe.c | 400 +++++
drivers/staging/cxt1e1/libsbew.h | 581 +++++++
drivers/staging/cxt1e1/linux.c | 1354 ++++++++++++++++
drivers/staging/cxt1e1/musycc.c | 2180 ++++++++++++++++++++++++++
drivers/staging/cxt1e1/musycc.h | 460 ++++++
drivers/staging/cxt1e1/ossiRelease.c | 39 +
drivers/staging/cxt1e1/pmc93x6_eeprom.c | 559 +++++++
drivers/staging/cxt1e1/pmc93x6_eeprom.h | 60 +
drivers/staging/cxt1e1/pmcc4.h | 155 ++
drivers/staging/cxt1e1/pmcc4_cpld.h | 124 ++
drivers/staging/cxt1e1/pmcc4_defs.h | 82 +
drivers/staging/cxt1e1/pmcc4_drv.c | 1855 ++++++++++++++++++++++
drivers/staging/cxt1e1/pmcc4_ioctls.h | 81 +
drivers/staging/cxt1e1/pmcc4_private.h | 295 ++++
drivers/staging/cxt1e1/pmcc4_sysdep.h | 62 +
drivers/staging/cxt1e1/sbe_bid.h | 61 +
drivers/staging/cxt1e1/sbe_promformat.h | 157 ++
drivers/staging/cxt1e1/sbecom_inline_linux.h | 310 ++++
drivers/staging/cxt1e1/sbecrc.c | 137 ++
drivers/staging/cxt1e1/sbeid.c | 217 +++
drivers/staging/cxt1e1/sbeproc.c | 358 +++++
drivers/staging/cxt1e1/sbeproc.h | 52 +
drivers/staging/cxt1e1/sbew_ioc.h | 136 ++
33 files changed, 11705 insertions(+), 0 deletions(-)
How do I generate the patch file?
thanks,
--
-Bob Beers
^ permalink raw reply
* Re: [net-next-2.6 PATCH v4 3/3] TCPCT part 1c: initial SYN exchange with SYNACK data
From: William Allen Simpson @ 2009-11-02 20:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers, Ilpo Järvinen
In-Reply-To: <4AEF1106.40403@gmail.com>
Eric Dumazet wrote:
> William Allen Simpson a écrit :
>
>> It will be, when we have running code, as I'm loath to publish until I'm
>> certain it *can* be implemented.
>
> This is why RFC is better before coding. To avoid wasting time on experiments
> that have a fatal flaw. Once included in an official kernel, we wont be able
> to change some parameters very easily (think about 253 constant you use)
>
We are talking at cross purposes. IEEE had/has a tendency to publish
without running code. CTIA/TIA/EIA has interim standards without running
code, and had/has rules against even speaking about implementations. Only
IETF expected/expects running code. An RFC is rarely issued before coding.
http://www.iana.org/assignments/tcp-parameters/
253 N RFC3692-style Experiment 1 (*) [RFC4727]
When my code is done, I'll post the completed draft and ask IANA for a
non-experimental number. That's the usual process. My code shouldn't go
into an official release until we know that number.
PS. Rumor has it that Cisco shipped a release with 254 in it. Let's not
do that here.
PPS. Adam used 255, not a correct official experimental number.
^ permalink raw reply
* Re: [PATCH 0/5] Candidate fix for increased number of GFP_ATOMIC failures V2
From: Mel Gorman @ 2009-11-02 20:30 UTC (permalink / raw)
To: Karol Lewandowski
Cc: Frans Pop, Jiri Kosina, Sven Geggus, Tobias Oetiker,
Rafael J. Wysocki, David Miller, Reinette Chatre, Kalle Valo,
David Rientjes, KOSAKI Motohiro, Mohamed Abbas, Jens Axboe,
John W. Linville, Pekka Enberg, Bartlomiej Zolnierkiewicz,
Greg Kroah-Hartman, Stephan von Krawczynski, Kernel Testers List,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org
In-Reply-To: <20091030142350.GA9343-nLtalAL5mPp2RxbNQum0x1nzlInOXLuq@public.gmane.org>
On Fri, Oct 30, 2009 at 03:23:50PM +0100, Karol Lewandowski wrote:
> On Wed, Oct 28, 2009 at 11:59:26AM +0000, Mel Gorman wrote:
> > On Wed, Oct 28, 2009 at 12:42:08PM +0100, Karol Lewandowski wrote:
> > > On Sat, Oct 24, 2009 at 02:46:56PM +0100, Mel LKML wrote:
> > > I've tested patches 1+2+3+4 in my normal usage scenario (do some work,
> > > suspend, do work, suspend, ...) and it failed today after 4 days (== 4
> > > suspend-resume cycles).
> > >
> > > I'll test 1-5 now.
>
> 2.6.32-rc5 with patches 1-5 fails too.
>
>
> > Also, what was the behaviour of the e100 driver when suspending before
> > this commit?
> >
> > 6905b1f1a03a48dcf115a2927f7b87dba8d5e566: Net / e100: Fix suspend of devices that cannot be power managed
>
> This was discussed before with e100 maintainers and Rafael. Reverting
> this patch didn't change anything.
>
Does applying the following on top make any difference?
==== CUT HERE ====
PM: Shrink memory before suspend
This is a partial revert of c6f37f12197ac3bd2e5a35f2f0e195ae63d437de. It
is an outside possibility for fixing the e100 bug where an order-5
allocation is failing during resume. The commit notes that the shrinking
of memory should be unnecessary but maybe it is in error.
Signed-off-by: Mel Gorman <mel-wPRd99KPJ+uzQB+pC5nmwQ@public.gmane.org>
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 6f10dfc..4f6ae64 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -23,6 +23,9 @@ const char *const pm_states[PM_SUSPEND_MAX] = {
[PM_SUSPEND_MEM] = "mem",
};
+/* This is just an arbitrary number */
+#define FREE_PAGE_NUMBER (100)
+
static struct platform_suspend_ops *suspend_ops;
/**
@@ -78,6 +81,7 @@ static int suspend_test(int level)
static int suspend_prepare(void)
{
int error;
+ unsigned int free_pages;
if (!suspend_ops || !suspend_ops->enter)
return -EPERM;
@@ -92,10 +96,24 @@ static int suspend_prepare(void)
if (error)
goto Finish;
- error = suspend_freeze_processes();
+ if (suspend_freeze_processes()) {
+ error = -EAGAIN;
+ goto Thaw;
+ }
+
+ free_pages = global_page_state(NR_FREE_PAGES);
+ if (free_pages < FREE_PAGE_NUMBER) {
+ pr_debug("PM: free some memory\n");
+ shrink_all_memory(FREE_PAGE_NUMBER - free_pages);
+ if (nr_free_pages() < FREE_PAGE_NUMBER) {
+ error = -ENOMEM;
+ printk(KERN_ERR "PM: No enough memory\n");
+ }
+ }
if (!error)
return 0;
+ Thaw:
suspend_thaw_processes();
usermodehelper_enable();
Finish:
^ permalink raw reply related
* Re: [net-next-2.6 PATCH v4 3/3] TCPCT part 1c: initial SYN exchange with SYNACK data
From: William Allen Simpson @ 2009-11-02 20:15 UTC (permalink / raw)
To: Joe Perches
Cc: Linux Kernel Network Developers, Ilpo Järvinen, Eric Dumazet
In-Reply-To: <1257185798.28925.33.camel@Joe-Laptop.home>
Joe Perches wrote:
> On Mon, 2009-11-02 at 13:10 -0500, William Allen Simpson wrote:
>> +static inline struct tcp_extend_values *tcp_xv(const struct request_values *rvp)
>> +{
>> + return (struct tcp_extend_values *)rvp;
>> +}
>>...
> I don't know, but I do have a bias against casting
> const to non-const.
>
Oh dear. That's how everything in both include/linux/tcp.h and
include/net/tcp.h is done.... As I said, we've had really bad luck
using examples from the existing code base.
If somebody else submitted a patch to change all the rest, I'd be
content to follow along.
^ permalink raw reply
* Re: [PATCH] can: Driver for the Microchip MCP251x SPI CAN controllers
From: Wolfgang Grandegger @ 2009-11-02 19:49 UTC (permalink / raw)
To: Christian Pellegrin
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1257175840-28528-1-git-send-email-chripell-VaTbYqLCNhc@public.gmane.org>
I assume this is v2 of the patch.
Christian Pellegrin wrote:
> Signed-off-by: Christian Pellegrin <chripell-VaTbYqLCNhc@public.gmane.org>
> ---
> drivers/net/can/Kconfig | 6 +
> drivers/net/can/Makefile | 1 +
> drivers/net/can/mcp251x.c | 1164 ++++++++++++++++++++++++++++++++++
> include/linux/can/platform/mcp251x.h | 36 +
> 4 files changed, 1207 insertions(+), 0 deletions(-)
> create mode 100644 drivers/net/can/mcp251x.c
> create mode 100644 include/linux/can/platform/mcp251x.h
>
[snip]
> diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c
> new file mode 100644
> index 0000000..9471bb7
> --- /dev/null
> +++ b/drivers/net/can/mcp251x.c
[snip]
> +static void mcp251x_hw_tx(struct spi_device *spi, struct can_frame *frame,
> + int tx_buf_idx)
> +{
> + u32 sid, eid, exide, rtr;
> + u8 buf[SPI_TRANSFER_BUF_LEN];
> +
> + exide = (frame->can_id & CAN_EFF_FLAG) ? 1 : 0; /* Extended ID Enable */
> + if (exide)
> + sid = (frame->can_id & CAN_EFF_MASK) >> 18;
> + else
> + sid = frame->can_id & CAN_SFF_MASK; /* Standard ID */
> + eid = frame->can_id & CAN_EFF_MASK; /* Extended ID */
> + rtr = (frame->can_id & CAN_RTR_FLAG) ? 1 : 0; /* Remote transmission */
> +
> + buf[TXBCTRL_OFF] = INSTRUCTION_LOAD_TXB(tx_buf_idx);
> + buf[TXBSIDH_OFF] = sid >> SIDH_SHIFT;
> + buf[TXBSIDL_OFF] = ((sid & SIDL_SID_MASK) << SIDL_SID_SHIFT) |
> + (exide << SIDL_EXIDE_SHIFT) |
> + ((eid >> SIDL_EID_SHIFT) & SIDL_EID_MASK);
> + buf[TXBEID8_OFF] = GET_BYTE(eid, 1);
> + buf[TXBEID0_OFF] = GET_BYTE(eid, 0);
> + buf[TXBDLC_OFF] = (rtr << DLC_RTR_SHIFT) | frame->can_dlc;
Two spaces before "=".
> + memcpy(buf + TXBDAT_OFF, frame->data, frame->can_dlc);
> + mcp251x_hw_tx_frame(spi, buf, frame->can_dlc, tx_buf_idx);
> + mcp251x_write_reg(spi, TXBCTRL(tx_buf_idx), TXBCTRL_TXREQ);
> +}
> +
[snip]
> +static int mcp251x_open(struct net_device *net)
> +{
> + struct mcp251x_priv *priv = netdev_priv(net);
> + struct spi_device *spi = priv->spi;
> + struct mcp251x_platform_data *pdata = spi->dev.platform_data;
> + int ret;
> +
> + if (pdata->transceiver_enable)
> + pdata->transceiver_enable(1);
> +
> + priv->force_quit = 0;
> + priv->tx_skb = NULL;
> + priv->tx_len = 0;
> +
> + ret = request_irq(spi->irq, mcp251x_can_isr,
> + IRQF_TRIGGER_FALLING, DEVICE_NAME, net);
> + if (ret) {
> + dev_err(&spi->dev, "failed to acquire irq %d\n", spi->irq);
> + return ret;
> + }
Here the transceiver should be switched off!?
> + mcp251x_hw_wakeup(spi);
> + mcp251x_hw_reset(spi);
> + ret = mcp251x_setup(net, priv, spi);
> + if (ret) {
> + free_irq(spi->irq, net);
> + if (pdata->transceiver_enable)
> + pdata->transceiver_enable(0);
> + return ret;
> + }
> + mcp251x_set_normal_mode(spi);
> + netif_wake_queue(net);
> +
> + return 0;
> +}
> +
[snip]
> +static void mcp251x_irq_work_handler(struct work_struct *ws)
> +{
> + struct mcp251x_priv *priv = container_of(ws, struct mcp251x_priv,
> + irq_work);
> + struct spi_device *spi = priv->spi;
> + struct net_device *net = priv->net;
> + u8 txbnctrl;
> + u8 intf;
> + enum can_state new_state;
> +
> + if (priv->after_suspend) {
> + mdelay(10);
> + mcp251x_hw_reset(spi);
> + mcp251x_setup(net, priv, spi);
> + if (priv->after_suspend & AFTER_SUSPEND_RESTART) {
> + mcp251x_set_normal_mode(spi);
> + } else if (priv->after_suspend & AFTER_SUSPEND_UP) {
> + netif_device_attach(net);
> + /* Clean since we lost tx buffer */
> + if (priv->tx_skb || priv->tx_len) {
> + mcp251x_clean(net);
> + netif_wake_queue(net);
> + }
> + mcp251x_set_normal_mode(spi);
> + } else {
> + mcp251x_hw_sleep(spi);
> + }
> + priv->after_suspend = 0;
> + }
> +
> + if (priv->can.restart_ms == 0 && priv->can.state == CAN_STATE_BUS_OFF) {
> + while (!priv->force_quit && !freezing(current) &&
> + (intf = mcp251x_read_reg(spi, CANINTF)))
> + mcp251x_write_bits(spi, CANINTF, intf, 0x00);
Assigning variables within if or while expressions is not allowed. I
wonder why checkpatch did not spot it.
> + return;
> + }
> +
> + while (!priv->force_quit && !freezing(current)) {
> + u8 eflag = mcp251x_read_reg(spi, EFLG);
> + int can_id = 0, data1 = 0;
> +
> + mcp251x_write_reg(spi, EFLG, 0x00);
> +
> + if (priv->restart_tx) {
> + priv->restart_tx = 0;
> + mcp251x_write_reg(spi, TXBCTRL(0), 0);
> + if (priv->tx_skb || priv->tx_len)
> + mcp251x_clean(net);
> + netif_wake_queue(net);
> + can_id |= CAN_ERR_RESTARTED;
> + }
> +
> + if (priv->wake) {
> + /* Wait whilst the device wakes up */
> + mdelay(10);
> + priv->wake = 0;
> + }
> +
> + intf = mcp251x_read_reg(spi, CANINTF);
> + mcp251x_write_bits(spi, CANINTF, intf, 0x00);
> +
> + /* Update can state */
> + if (eflag & EFLG_TXBO) {
> + new_state = CAN_STATE_BUS_OFF;
> + can_id |= CAN_ERR_BUSOFF;
> + } else if (eflag & EFLG_TXEP) {
> + new_state = CAN_STATE_ERROR_PASSIVE;
> + can_id |= CAN_ERR_CRTL;
> + data1 |= CAN_ERR_CRTL_TX_PASSIVE;
> + } else if (eflag & EFLG_RXEP) {
> + new_state = CAN_STATE_ERROR_PASSIVE;
> + can_id |= CAN_ERR_CRTL;
> + data1 |= CAN_ERR_CRTL_RX_PASSIVE;
> + } else if (eflag & EFLG_TXWAR) {
> + new_state = CAN_STATE_ERROR_WARNING;
> + can_id |= CAN_ERR_CRTL;
> + data1 |= CAN_ERR_CRTL_TX_WARNING;
> + } else if (eflag & EFLG_RXWAR) {
> + new_state = CAN_STATE_ERROR_WARNING;
> + can_id |= CAN_ERR_CRTL;
> + data1 |= CAN_ERR_CRTL_RX_WARNING;
> + } else {
> + new_state = CAN_STATE_ERROR_ACTIVE;
> + }
> +
> + /* Update can state statistics */
> + switch (priv->can.state) {
> + case CAN_STATE_ERROR_ACTIVE:
> + if (new_state >= CAN_STATE_ERROR_WARNING &&
> + new_state <= CAN_STATE_BUS_OFF)
> + priv->can.can_stats.error_warning++;
> + case CAN_STATE_ERROR_WARNING: /* fallthrough */
> + if (new_state >= CAN_STATE_ERROR_PASSIVE &&
> + new_state <= CAN_STATE_BUS_OFF)
> + priv->can.can_stats.error_passive++;
> + break;
> + default:
> + break;
> + }
> + priv->can.state = new_state;
> +
> + if ((intf & CANINTF_ERRIF) || (can_id & CAN_ERR_RESTARTED)) {
> + struct sk_buff *skb;
> + struct can_frame *frame;
> +
> + /* Create error frame */
> + skb = alloc_can_err_skb(net, &frame);
> + if (skb) {
> + /* Set error frame flags based on bus state */
> + frame->can_id = can_id;
> + frame->data[1] = data1;
> +
> + /* Update net stats for overflows */
> + if (eflag & (EFLG_RX0OVR | EFLG_RX1OVR)) {
> + if (eflag & EFLG_RX0OVR)
> + net->stats.rx_over_errors++;
> + if (eflag & EFLG_RX1OVR)
> + net->stats.rx_over_errors++;
> + frame->can_id |= CAN_ERR_CRTL;
> + frame->data[1] |=
> + CAN_ERR_CRTL_RX_OVERFLOW;
> + }
> +
> + netif_rx(skb);
> + } else {
> + dev_info(&spi->dev,
> + "cannot allocate error skb\n");
> + }
> + }
> +
> + if (priv->can.state == CAN_STATE_BUS_OFF) {
> + if (priv->can.restart_ms == 0) {
> + can_bus_off(net);
> + mcp251x_hw_sleep(spi);
> + return;
> + }
> + }
> +
> + if (intf == 0)
> + break;
> +
> + if (intf & CANINTF_WAKIF)
> + complete(&priv->awake);
> +
> + if (intf & CANINTF_MERRF) {
> + /* If there are pending Tx buffers, restart queue */
> + txbnctrl = mcp251x_read_reg(spi, TXBCTRL(0));
> + if (!(txbnctrl & TXBCTRL_TXREQ)) {
> + if (priv->tx_skb || priv->tx_len)
> + mcp251x_clean(net);
> + netif_wake_queue(net);
> + }
> + }
> +
> + if (intf & (CANINTF_TX2IF | CANINTF_TX1IF | CANINTF_TX0IF)) {
> + net->stats.tx_packets++;
> + net->stats.tx_bytes += priv->tx_len - 1;
> + if (priv->tx_len) {
> + can_get_echo_skb(net, 0);
> + priv->tx_len = 0;
> + }
> + netif_wake_queue(net);
> + }
> +
> + if (intf & CANINTF_RX0IF)
> + mcp251x_hw_rx(spi, 0);
> +
> + if (intf & CANINTF_RX1IF)
> + mcp251x_hw_rx(spi, 1);
> + }
> +}
> +
> +static const struct net_device_ops mcp251x_netdev_ops = {
> + .ndo_open = mcp251x_open,
> + .ndo_stop = mcp251x_stop,
> + .ndo_start_xmit = mcp251x_hard_start_xmit,
> +};
> +
> +static int __devinit mcp251x_can_probe(struct spi_device *spi)
> +{
> + struct net_device *net;
> + struct mcp251x_priv *priv;
> + struct mcp251x_platform_data *pdata = spi->dev.platform_data;
> + int ret = -ENODEV;
> +
> + if (!pdata)
> + /* Platform data is required for osc freq */
> + goto error_out;
> +
> + /* Allocate can/net device */
> + net = alloc_candev(sizeof(struct mcp251x_priv), TX_ECHO_SKB_MAX);
> + if (!net) {
> + ret = -ENOMEM;
> + goto error_alloc;
> + }
> +
> + net->netdev_ops = &mcp251x_netdev_ops;
> + net->flags |= IFF_ECHO;
Remove spaces, please.
> + priv = netdev_priv(net);
> + priv->can.bittiming_const = &mcp251x_bittiming_const;
> + priv->can.do_set_mode = mcp251x_do_set_mode;
> + priv->can.clock.freq = pdata->oscillator_frequency / 2;
> + priv->can.do_set_bittiming = mcp251x_do_set_bittiming;
> + priv->net = net;
> + dev_set_drvdata(&spi->dev, priv);
> +
> + priv->spi = spi;
> + mutex_init(&priv->spi_lock);
> +
> + /* If requested, allocate DMA buffers */
> + if (mcp251x_enable_dma) {
> + spi->dev.coherent_dma_mask = ~0;
> +
> + /*
> + * Minimum coherent DMA allocation is PAGE_SIZE, so allocate
> + * that much and share it between Tx and Rx DMA buffers.
> + */
> + priv->spi_tx_buf = dma_alloc_coherent(&spi->dev,
> + PAGE_SIZE,
> + &priv->spi_tx_dma,
> + GFP_DMA);
> +
> + if (priv->spi_tx_buf) {
> + priv->spi_rx_buf = (u8 *)(priv->spi_tx_buf +
> + (PAGE_SIZE / 2));
> + priv->spi_rx_dma = (dma_addr_t)(priv->spi_tx_dma +
> + (PAGE_SIZE / 2));
> + } else {
> + /* Fall back to non-DMA */
> + mcp251x_enable_dma = 0;
> + }
> + }
> +
> + /* Allocate non-DMA buffers */
> + if (!mcp251x_enable_dma) {
> + priv->spi_tx_buf = kmalloc(SPI_TRANSFER_BUF_LEN, GFP_KERNEL);
> + if (!priv->spi_tx_buf) {
> + ret = -ENOMEM;
> + goto error_tx_buf;
> + }
> + priv->spi_rx_buf = kmalloc(SPI_TRANSFER_BUF_LEN, GFP_KERNEL);
> + if (!priv->spi_tx_buf) {
> + ret = -ENOMEM;
> + goto error_rx_buf;
> + }
> + }
> +
> + if (pdata->power_enable)
> + pdata->power_enable(1);
> +
> + /* Call out to platform specific setup */
> + if (pdata->board_specific_setup)
> + pdata->board_specific_setup(spi);
> +
> + SET_NETDEV_DEV(net, &spi->dev);
> +
> + priv->wq = create_freezeable_workqueue("mcp251x_wq");
> +
> + INIT_WORK(&priv->tx_work, mcp251x_tx_work_handler);
> + INIT_WORK(&priv->irq_work, mcp251x_irq_work_handler);
> +
> + init_completion(&priv->awake);
> +
> + /* Configure the SPI bus */
> + spi->mode = SPI_MODE_0;
> + spi->bits_per_word = 8;
> + spi_setup(spi);
> +
> + if (!mcp251x_hw_probe(spi)) {
> + dev_info(&spi->dev, "Probe failed\n");
> + goto error_probe;
> + }
> + mcp251x_hw_sleep(spi);
> +
> + if (pdata->transceiver_enable)
> + pdata->transceiver_enable(0);
> +
> + ret = register_candev(net);
> + if (ret >= 0) {
if (!ret) ?
> + dev_info(&spi->dev, "probed\n");
> + return ret;
> + }
Shouldn't the power be switched off?
> +error_probe:
> + if (!mcp251x_enable_dma)
> + kfree(priv->spi_rx_buf);
> +error_rx_buf:
> + if (!mcp251x_enable_dma)
> + kfree(priv->spi_tx_buf);
> +error_tx_buf:
> + free_candev(net);
> + if (mcp251x_enable_dma)
> + dma_free_coherent(&spi->dev, PAGE_SIZE,
> + priv->spi_tx_buf, priv->spi_tx_dma);
> +error_alloc:
> + dev_err(&spi->dev, "probe failed\n");
> +error_out:
> + return ret;
> +}
> +
[snip]
We are close...
Wolfgang.
^ permalink raw reply
* Re: [PATCH net-next-2.6] Driver for the Microchip MCP251x SPI CAN controllers
From: Wolfgang Grandegger @ 2009-11-02 19:28 UTC (permalink / raw)
To: christian pellegrin
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cabda6420911020706r340ef073qea40527f09551a8a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
christian pellegrin wrote:
> On Sun, Nov 1, 2009 at 10:31 AM, Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org> wrote:
>> Hi Christian,
>>
>
> Hi,
>
>> there are a few. In general, please check the usage of {} for if
>
> sorry for missing this: I read your link below: I missed that rule on
> first reading! And I tend to trust checkpatch.pl too much ;-)
>
>> statements and check if "if (ret)" should be used instead of "if (ret <
>> 0)" if 0 means success and !0 failure. I don't have a MCP251x hardware
>
> ok, I misunderstood this to. Now I think it's ok.
>
> I'm replying to this thread with v2 patch. I'm rebasing the
> differences against SVN trunk too, but I'm waiting to send them until
> this patch is accepted in net-next-2.6 since their are only of
> cosmetic nature.
That's fine. of course.
[snip]
>>> +#include <linux/can/core.h>
>> I don't think you need "can/core.h"?
>>
>
> I tried without but there are some dependencies in "can/dev.h" to some
> netdev stuff that are broken if I omit it.
Hm, sounds like a bug. I will check a.s.a.p.
Wolfgang.
^ permalink raw reply
* Re: [PATCH] r8169: partial support and phy init for the 8168d
From: Ben Hutchings @ 2009-11-02 19:06 UTC (permalink / raw)
To: David Miller; +Cc: romieu, netdev, edward_hsu
In-Reply-To: <20091101.224645.262714217.davem@davemloft.net>
On Sun, Nov 01, 2009 at 10:46:45PM -0800, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Sun, 01 Nov 2009 16:24:43 +0000
>
> > I believe this is patching machine code in the PHY. And we do not have
> > source for that code, so it cannot possibly be distributed under GPL.
>
> You don't know if it's machine code or some data values that
> are used to control the PHY's execution.
No, I don't know that this is machine code. But you are setting up
a false opposition: 'data values that are used to control the PHY's
execution' certainly include the machine code that it executes.
> In fact I would really be surprised if they had some cpu interpeting
> code in the 8168d PHY.
Your knowledge of PHYs may be outdated. 1000BASE-T has to be done
with a mixture of analog and digital signal processing, since analog
alone would be too power-hungry. While autonegotiation and power
management could be hardwired it's less risky to put in a micro-
controller and finalise the firmware after the silicon... or even
after release, as seems to have happened here.
> You did do some research about that before making such accusations
> right? :-)
I began with 'I believe'. It's not an accusation.
Ben.
--
Ben Hutchings
Sturgeon's Law: Ninety percent of everything is crap.
^ permalink raw reply
* Re: pull request: wireless-next-2.6 2009-10-28
From: Ivo van Doorn @ 2009-11-02 18:43 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: Randy Dunlap, Luis Correia, John W. Linville, Ingo Molnar,
Johannes Berg, Jarek Poplawski, Bartlomiej Zolnierkiewicz,
Pekka Enberg, David Miller, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4AEF1894.2010809-/UHa2rfvQTnk1uMJSBkQmQ@public.gmane.org>
Well and then another rt2x00 developer discovered this nice little
fight about rt2x00 on the mailinglists...
First for the record, because at the start people where talking about the
maintainership of rt2x00, one thing needs to be straight:
As mentioned in the MAINTAINERS file, the rt2x00 project is listed as maintainer
for the rt2x00 drivers. The rt2x00 drivers include _all_ drivers in the
drivers/net/wireless/rt2x00 folder.
At this time I am hold the position within the rt2x00 team which is making the
decisions about the rt2x00 code and design.
I am also the one that is (N)Acks the patches from others when they are send
to the rt2x00-users or linux-wireless mailinglist.
As for my behavior in discussions:
I am doing my best to listen to all complains regarding the rt2x00 code and design and
improve it if the complainer has a valid point. However, obviously I can disagree with
the complainer and in that case I will explain to that person _why_ I disagree. It is up
to the complainer to convince me that he is right, agree with my response, or whine.
Now as for more specific responses:
On Thursday 29 October 2009, Bartlomiej Zolnierkiewicz wrote:
> rt2800 drivers have their maintainers and I would like to know what they
> are doing besides complaining about users and staging tree..
Working for Avanade, Zarafa and as freelancer for Linux Magazine.
But I guess you mean rt2x00 specific work?
Well that list consists of:
- Listening to people complain
- Responding to those people, because otherwise they complain that they are being ignored.
- Following bug reports, and request testing or additional information if required
- Bugfixing
- Reviewing patches from contributors
- Applying patches from contributors
- Discussing improvements over patches from contributors
Well nothing of this list should be new to you, but apparently you needed some confirmation.
On Thursday 29 October 2009, Johannes Berg wrote:
> On Thu, 2009-10-29 at 07:20 -0700, David Miller wrote:
>
> > In case you're concerned, I actually agree with John and others
> > on this issue, and disagree with your position.
>
> In this particular case, I think it makes more sense to duplicate the
> code _especially_ because it's not working yet. That frees people
> hacking on it of having to worry about breaking other devices.
Thank you Johannes, that is exactly what I was trying to tell Bartlomiej
in the previous discussion.
On Wednesday 28 October 2009, Bartlomiej Zolnierkiewicz wrote:
> I find it rather disappointing that all my review comments regarding
> rt2800pci support were just completely ignored and then the initial
> patch was merged just as it was..
Your code review comments were commented upon with my reasons
why this code duplication exists. I even admitted that when the time is
ready I will remove the code duplication.
On Thursday 29 October 2009, Bartlomiej Zolnierkiewicz wrote:
> Quite the contrary, I'm pretty confident that addressing my review concerns
> would result in better RT28x00 / RT30x0 support in the very near future.
The review concerns regarding the duplicate code would only reduce the
amount of code. It would not magically fix bugs (at least the chance of that
would be quite small).
So far rt2800usb performs better then rt2800pci, and the difference gets
only bigger when I use the exact same register initialization from rt2800usb
in rt2800pci.
But Bartjmoiej knows that the register initialization can be exactly the same,
from his experience with the staging drivers.
So far hasn't been interested in sharing the knowledge in what must be
changed in rt2800pci/usb to make them both work with the same register
initialization.
On Monday 02 November 2009, Randy Dunlap wrote:
> Luis Correia wrote:
> > [huge snip]
> > I've searched on my GMail archives and the only patch Bart has
> > provided so far for the rt2x00 project is this:
> >
> > [PATCH] MAINTAINERS: rt2x00 list is moderated
> >
> > Which, while technically correct, adds nothing to the project.
>
> whatever. That patch still needs to be applied.
And I haven't seen anybody stating the opposite...
Ivo
--
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
* Re: pull request: wireless-next-2.6 2009-10-28
From: John W. Linville @ 2009-11-02 18:29 UTC (permalink / raw)
To: Randy Dunlap
Cc: Luis Correia, Ingo Molnar, Johannes Berg, Jarek Poplawski,
Bartlomiej Zolnierkiewicz, Pekka Enberg, David Miller,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4AEF1894.2010809-/UHa2rfvQTnk1uMJSBkQmQ@public.gmane.org>
On Mon, Nov 02, 2009 at 09:36:20AM -0800, Randy Dunlap wrote:
> Luis Correia wrote:
> > [huge snip]
> >
> > This is going to be my last reply to this thread (even if I get to be insulted).
> >
> > I'm not a coder, I'm a tester and administrator for the rt2x00 project.
> > I'm not happy about the current state of the rt2x00 in-kernel drivers.
> > I tend to agree that more could be done.
> >
> > Even though I'm not a coder, I could be hipotetically responsable for
> > the current situation. It is my job function to motivate people to
> > work on the rt2x00 in-kernel drivers, but apparently I've failed here.
> > It's not an atitude problem, as some have suggested, it's just life.
> >
> > About Bartlomiej, there is only one more thing that I'll say:
> >
> > I've searched on my GMail archives and the only patch Bart has
> > provided so far for the rt2x00 project is this:
> >
> > [PATCH] MAINTAINERS: rt2x00 list is moderated
> >
> > Which, while technically correct, adds nothing to the project.
>
> whatever. That patch still needs to be applied.
And, in fact, it has been...already in linux-2.6:
commit b5654f5e7fc414a6e69b3647db2b043257c9e62e
Author: Bartlomiej Zolnierkiewicz <bzolnier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Mon Oct 26 16:49:50 2009 -0700
MAINTAINERS: rt2x00 list is moderated
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Signed-off-by: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Hth...
John
--
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
* Re: RFC: netdev: allow ethtool physical id to drop rtnl_lock
From: Ben Hutchings @ 2009-11-02 18:29 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, mchan, netdev
In-Reply-To: <20091102085121.7416fa8f@nehalam>
On Mon, 2009-11-02 at 08:51 -0800, Stephen Hemminger wrote:
[...]
> For compatibility, I was thinking of adding a new ethtool hook that
> moves the blinking loop into ethtool.
>
> static int ethtool_phys_blink(struct net_device *dev, u32 secs)
> {
> while (secs > 0) {
> dev->ethtool_ps->phys_led(dev, ETH_LED_ON);
> ...
> dev->ethtool_ps->phys_led(dev, ETH_LED_OFF);
> }
> dev->ethtool_ops->phys_led(dev, ETH_LED_NORMAL);
> }
[...]
We could even force this on unconverted drivers, but using a longer
loop period to allow for slow blinking:
static int ethtool_phys_id_loop(struct net_device *dev, u32 secs)
{
u32 timeout;
int rc;
for (;;) {
timeout = min_t(u32, secs, 5);
secs -= timeout;
rc = dev->ethtool_ops->phys_id(dev, timeout);
if (rc || secs == 0)
return rc;
...
}
}
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] Multicast packet reassembly can fail
From: Steve Chen @ 2009-11-02 18:36 UTC (permalink / raw)
To: Herbert Xu; +Cc: rick.jones2, mhuth, David Stevens, Eric Dumazet, netdev
In-Reply-To: <20091029180450.GA31044@gondor.apana.org.au>
On Thu, 2009-10-29 at 14:04 -0400, Herbert Xu wrote:
> Steve Chen <schen@mvista.com> wrote:
> >
> > 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.
>
> Well if it does bind to the interface then that explains the
> failure. And the fix is "if it hurts, don't do it" :)
>
> Cheers,
The packet drop was tracked to rp_filter. All packets received as
expected after disabling rp_filter. Thank you all for the inputs.
Regards,
Steve
^ permalink raw reply
* Re: [net-next-2.6 PATCH v4 3/3] TCPCT part 1c: initial SYN exchange with SYNACK data
From: Joe Perches @ 2009-11-02 18:16 UTC (permalink / raw)
To: William Allen Simpson
Cc: Linux Kernel Network Developers, Ilpo Järvinen, Eric Dumazet
In-Reply-To: <4AEF207C.9030002@gmail.com>
On Mon, 2009-11-02 at 13:10 -0500, William Allen Simpson wrote:
> Another style question:
>
> +struct tcp_extend_values {
> + u8 cookie_bakery[TCP_COOKIE_MAX];
> + u8 cookie_plus;
> + u8 cookie_in_always:1,
> + cookie_out_never:1;
> +};
> +
> +static inline struct tcp_extend_values *tcp_xv(const struct request_values *rvp)
> +{
> + return (struct tcp_extend_values *)rvp;
> +}
>
> Some examples have "struct request_values" as the first element, others
> don't. I started with it, and then removed it (as essentially nil).
>
> Is there a preference?
I don't know, but I do have a bias against casting
const to non-const.
cheers, Joe
^ permalink raw reply
* Re: [net-next-2.6 PATCH v4 3/3] TCPCT part 1c: initial SYN exchange with SYNACK data
From: William Allen Simpson @ 2009-11-02 18:10 UTC (permalink / raw)
To: Linux Kernel Network Developers
Cc: Ilpo Järvinen, Eric Dumazet, Joe Perches
In-Reply-To: <4AE6E7C0.2050408@gmail.com>
Another style question:
+struct tcp_extend_values {
+ u8 cookie_bakery[TCP_COOKIE_MAX];
+ u8 cookie_plus;
+ u8 cookie_in_always:1,
+ cookie_out_never:1;
+};
+
+static inline struct tcp_extend_values *tcp_xv(const struct request_values *rvp)
+{
+ return (struct tcp_extend_values *)rvp;
+}
Some examples have "struct request_values" as the first element, others
don't. I started with it, and then removed it (as essentially nil).
Is there a preference?
^ permalink raw reply
* Re: [PATCH] cnic: ensure ulp_type is not negative
From: Michael Chan @ 2009-11-02 17:56 UTC (permalink / raw)
To: Roel Kluin; +Cc: netdev@vger.kernel.org, Andrew Morton, LKML, davem
In-Reply-To: <4AEF0E98.5050304@gmail.com>
On Mon, 2009-11-02 at 08:53 -0800, Roel Kluin wrote:
> `ulp_type' is signed, make sure it is not negative
> when we read the array element.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
Looks good to me. Thanks.
Acked-by: Michael Chan <mchan@broadcom.com>
> ---
> drivers/net/cnic.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
> index 3bf1b04..f384b0a 100644
> --- a/drivers/net/cnic.c
> +++ b/drivers/net/cnic.c
> @@ -347,7 +347,7 @@ int cnic_register_driver(int ulp_type, struct cnic_ulp_ops *ulp_ops)
> {
> struct cnic_dev *dev;
>
> - if (ulp_type >= MAX_CNIC_ULP_TYPE) {
> + if (ulp_type < 0 || ulp_type >= MAX_CNIC_ULP_TYPE) {
> printk(KERN_ERR PFX "cnic_register_driver: Bad type %d\n",
> ulp_type);
> return -EINVAL;
> @@ -393,7 +393,7 @@ int cnic_unregister_driver(int ulp_type)
> struct cnic_ulp_ops *ulp_ops;
> int i = 0;
>
> - if (ulp_type >= MAX_CNIC_ULP_TYPE) {
> + if (ulp_type < 0 || ulp_type >= MAX_CNIC_ULP_TYPE) {
> printk(KERN_ERR PFX "cnic_unregister_driver: Bad type %d\n",
> ulp_type);
> return -EINVAL;
> @@ -449,7 +449,7 @@ static int cnic_register_device(struct cnic_dev *dev, int ulp_type,
> struct cnic_local *cp = dev->cnic_priv;
> struct cnic_ulp_ops *ulp_ops;
>
> - if (ulp_type >= MAX_CNIC_ULP_TYPE) {
> + if (ulp_type < 0 || ulp_type >= MAX_CNIC_ULP_TYPE) {
> printk(KERN_ERR PFX "cnic_register_device: Bad type %d\n",
> ulp_type);
> return -EINVAL;
> @@ -490,7 +490,7 @@ static int cnic_unregister_device(struct cnic_dev *dev, int ulp_type)
> struct cnic_local *cp = dev->cnic_priv;
> int i = 0;
>
> - if (ulp_type >= MAX_CNIC_ULP_TYPE) {
> + if (ulp_type < 0 || ulp_type >= MAX_CNIC_ULP_TYPE) {
> printk(KERN_ERR PFX "cnic_unregister_device: Bad type %d\n",
> ulp_type);
> return -EINVAL;
>
^ permalink raw reply
* Re: [net-next-2.6 PATCH v4 2/3] TCPCT part 1b: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS, functions
From: William Allen Simpson @ 2009-11-02 17:54 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev
In-Reply-To: <20091102.025150.73299764.davem@davemloft.net>
David Miller wrote:
> Yes, it is a problem.
>
> You can allocate the bit when you add the feature.
>
Sadly, I've been having very bad luck in my choice of code base, and
reviewing other similar code -- it seemed the common practice was to
specify the interface completely (even those bits not yet used).
I'll remove that line in the next pass.
^ permalink raw reply
* Re: [net-next-2.6 PATCH v4 3/3] TCPCT part 1c: initial SYN exchange with SYNACK data
From: William Allen Simpson @ 2009-11-02 17:50 UTC (permalink / raw)
To: Joe Perches; +Cc: Eric Dumazet, Linux Kernel Network Developers
In-Reply-To: <1257181219.28925.13.camel@Joe-Laptop.home>
Joe Perches wrote:
> Linus wrote a long time back (5+ years):
>
> The reason for "if (x == 8)" comes from the way we're taught to think.
> Arguing against that _fact_ is just totally non-productive, and you have
> to _force_ yourself to write it the other way around.
>
Interesting. I've not been able to Google this quote.
I actually think as I write it, finding the other ragged and hard to
visually review. But apparently it's an issue for ESL or something,
taught to think in another way.
Therefore, I'll re-code as Linus has prescribed. Sadly, I'm having
very bad luck verifying coding examples by checking against the
installed base, as I found thousands of lines following more usual
secure coding practices....
Thank you.
^ permalink raw reply
* Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: Eric Dumazet @ 2009-11-02 17:42 UTC (permalink / raw)
To: William Allen Simpson
Cc: Linux Kernel Developers, Linux Kernel Network Developers
In-Reply-To: <4AEF1534.4090506@gmail.com>
William Allen Simpson a écrit :
> Eric Dumazet wrote:
>> Large part of network code is run by softirq handler, and a softirq
>> handler
>> is not preemptable with another softirq (including itself).
>>
> Thank you. That's helpful to know, as some existing locks have a "bh".
> I've never figured out the ip_local_deliver_finish() context.
>
> Knowing that there can only be one instance of the tcp stack running at
> any one time, and the cpu never changes even after being interrupted, will
> make it much easier to code.
Correction :
There is one instance of sofirq handler running per cpu.
So TCP (or UDP) stack can run simultaneously on several (and eventually all online) cpus.
This is why we still need some locks in various places.
>>
> (No, I've not yet added locks; obviously, I'm still asking about them.)
>
> Unlikely, as it was easy to reproduce by changing one line, without
> *any* of
> my code present. Usually works, but doesn't work with tcpdump running on
> the interface:
Yes, tcpdump has the nasty habit to consume a lot of ram, queuing a copy of
all network traffic on an af_packet socket. (or using a mmap buffer, it depends
on libpcap version you use)
>
> struct sock *tcp_create_openreq_child(struct sock *sk, struct
> request_sock *req, struct sk_buff *skb)
> {
> - struct sock *newsk = inet_csk_clone(sk, req, GFP_ATOMIC);
> + struct sock *newsk = inet_csk_clone(sk, req, GFP_KERNEL);
Here you want a GFP_KERNEL allocation, that is allowed to sleep if there is not
enough available memory. (It's allowed to sleep to let some processes to free
bit of ram, eventually)
But as the caller of tcp_create_openreq_child() runs from {soft}irq context,
its not allowed to sleep at all, even 10 usecs.
Therefore, linux kernel kindly warns you that's its illegal and deadlockable.
Dont change GFP_ATOMIC here, its really there for a valid reason.
And be prepared to get a NULL result from allocation.
If your machine has problems when running tcpdump, maybe it lacks some ram, maybe you could
tune tcpdump socket to not exhaust all LOWMEM.
I see your kernel is 32bits, so you have only 860 MB of kernel memory, called LOWMEM.
I believe last kernels might have some problems in OOM situations...
^ 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