* Re: [PATCH] vlan: Fix the ingress VLAN_FLAG_REORDER_HDR check v2
From: Changli Gao @ 2011-06-03 3:59 UTC (permalink / raw)
To: padmanabh ratnakar
Cc: Eric W. Biederman, David Miller, shemminger, greearb,
nicolas.2p.debian, jpirko, netdev, kaber, fubar, eric.dumazet,
andy, jesse
In-Reply-To: <BANLkTikjdz+LkavRiuqdDNS6+djAer-a0A@mail.gmail.com>
On Fri, Jun 3, 2011 at 11:34 AM, padmanabh ratnakar
<pratnakarlx@gmail.com> wrote:
> Doesnt __vlan_put_tag()/vlan_insert_tag() depend on skb->data pointing
> to ethernet header.
> Is'nt the skb->data pointing past ethernet header in netif_receive_skb().
> Am I missing anything?
Yes, you are right. skb->data should be adjusted before feeding to
vlan_insert_tag(), or we make vlan_insert_tag() rely on
skb->mac_header instead.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH] vlan: Fix the ingress VLAN_FLAG_REORDER_HDR check v2
From: padmanabh ratnakar @ 2011-06-03 3:34 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David Miller, shemminger, greearb, nicolas.2p.debian, jpirko,
xiaosuo, netdev, kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <m11uzcidvq.fsf_-_@fess.ebiederm.org>
Doesnt __vlan_put_tag()/vlan_insert_tag() depend on skb->data pointing
to ethernet header.
Is'nt the skb->data pointing past ethernet header in netif_receive_skb().
Am I missing anything?
Regards,
Padmanabh
On Thu, Jun 2, 2011 at 6:33 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> Testing of VLAN_FLAG_REORDER_HDR does not belong in vlan_untag
> but rather in vlan_do_receive. Otherwise the vlan header
> will not be properly put on the packet in the case of
> vlan header accelleration.
>
> As we remove the check from vlan_check_reorder_header
> rename it vlan_reorder_header to keep the naming clean.
>
> Fix up the skb->pkt_type early so we don't look at the packet
> after adding the vlan tag, which guarantees we don't goof
> and look at the wrong field.
>
> Use a simple if statement instead of a complicated switch
> statement to decided that we need to increment rx_stats
> for a multicast packet.
>
> Hopefully at somepoint we will just declare the case where
> VLAN_FLAG_REORDER_HDR is cleared as unsupported and remove
> the code. Until then this keeps it working correctly.
>
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
> include/linux/if_vlan.h | 25 ++++++++++++++++++++--
> net/8021q/vlan_core.c | 50 ++++++++++++++++++++++------------------------
> 2 files changed, 46 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 290bd8a..821f0e3 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -220,7 +220,7 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb,
> }
>
> /**
> - * __vlan_put_tag - regular VLAN tag inserting
> + * vlan_insert_tag - regular VLAN tag inserting
> * @skb: skbuff to tag
> * @vlan_tci: VLAN TCI to insert
> *
> @@ -229,8 +229,10 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb,
> *
> * Following the skb_unshare() example, in case of error, the calling function
> * doesn't have to worry about freeing the original skb.
> + *
> + * Does not change skb->protocol so this function can be used during receive.
> */
> -static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
> +static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb, u16 vlan_tci)
> {
> struct vlan_ethhdr *veth;
>
> @@ -250,8 +252,25 @@ static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
> /* now, the TCI */
> veth->h_vlan_TCI = htons(vlan_tci);
>
> - skb->protocol = htons(ETH_P_8021Q);
> + return skb;
> +}
>
> +/**
> + * __vlan_put_tag - regular VLAN tag inserting
> + * @skb: skbuff to tag
> + * @vlan_tci: VLAN TCI to insert
> + *
> + * Inserts the VLAN tag into @skb as part of the payload
> + * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
> + *
> + * Following the skb_unshare() example, in case of error, the calling function
> + * doesn't have to worry about freeing the original skb.
> + */
> +static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
> +{
> + skb = vlan_insert_tag(skb, vlan_tci);
> + if (skb)
> + skb->protocol = htons(ETH_P_8021Q);
> return skb;
> }
>
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 41495dc..735c818 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -23,6 +23,20 @@ bool vlan_do_receive(struct sk_buff **skbp)
> return false;
>
> skb->dev = vlan_dev;
> + if (skb->pkt_type == PACKET_OTHERHOST) {
> + /* Our lower layer thinks this is not local, let's make sure.
> + * This allows the VLAN to have a different MAC than the
> + * underlying device, and still route correctly. */
> + if (!compare_ether_addr(eth_hdr(skb)->h_dest,
> + vlan_dev->dev_addr))
> + skb->pkt_type = PACKET_HOST;
> + }
> +
> + if (unlikely(!(vlan_dev_info(vlan_dev)->flags & VLAN_FLAG_REORDER_HDR))) {
> + skb = *skbp = vlan_insert_tag(skb, skb->vlan_tci);
> + if (!skb)
> + return false;
> + }
> skb->priority = vlan_get_ingress_priority(vlan_dev, skb->vlan_tci);
> skb->vlan_tci = 0;
>
> @@ -31,22 +45,8 @@ bool vlan_do_receive(struct sk_buff **skbp)
> u64_stats_update_begin(&rx_stats->syncp);
> rx_stats->rx_packets++;
> rx_stats->rx_bytes += skb->len;
> -
> - switch (skb->pkt_type) {
> - case PACKET_BROADCAST:
> - break;
> - case PACKET_MULTICAST:
> + if (skb->pkt_type == PACKET_MULTICAST)
> rx_stats->rx_multicast++;
> - break;
> - case PACKET_OTHERHOST:
> - /* Our lower layer thinks this is not local, let's make sure.
> - * This allows the VLAN to have a different MAC than the
> - * underlying device, and still route correctly. */
> - if (!compare_ether_addr(eth_hdr(skb)->h_dest,
> - vlan_dev->dev_addr))
> - skb->pkt_type = PACKET_HOST;
> - break;
> - }
> u64_stats_update_end(&rx_stats->syncp);
>
> return true;
> @@ -89,17 +89,15 @@ gro_result_t vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
> }
> EXPORT_SYMBOL(vlan_gro_frags);
>
> -static struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
> +static struct sk_buff *vlan_reorder_header(struct sk_buff *skb)
> {
> - if (vlan_dev_info(skb->dev)->flags & VLAN_FLAG_REORDER_HDR) {
> - if (skb_cow(skb, skb_headroom(skb)) < 0)
> - skb = NULL;
> - if (skb) {
> - /* Lifted from Gleb's VLAN code... */
> - memmove(skb->data - ETH_HLEN,
> - skb->data - VLAN_ETH_HLEN, 12);
> - skb->mac_header += VLAN_HLEN;
> - }
> + if (skb_cow(skb, skb_headroom(skb)) < 0)
> + skb = NULL;
> + if (skb) {
> + /* Lifted from Gleb's VLAN code... */
> + memmove(skb->data - ETH_HLEN,
> + skb->data - VLAN_ETH_HLEN, 12);
> + skb->mac_header += VLAN_HLEN;
> }
> return skb;
> }
> @@ -161,7 +159,7 @@ struct sk_buff *vlan_untag(struct sk_buff *skb)
> skb_pull_rcsum(skb, VLAN_HLEN);
> vlan_set_encap_proto(skb, vhdr);
>
> - skb = vlan_check_reorder_header(skb);
> + skb = vlan_reorder_header(skb);
> if (unlikely(!skb))
> goto err_free;
>
> --
> 1.7.5.1.217.g4e3aa
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH 5/5 v2] iwlegacy: add missing null check
From: Greg Dietsche @ 2011-06-03 3:24 UTC (permalink / raw)
To: sgruszka, linville
Cc: kernel-janitors, linux-wireless, netdev, linux-kernel,
Greg Dietsche
In-Reply-To: <20110603030746.GA4834@joana>
lq_sta has other null checks in this function.
assuming they are correct, this additional null check
should be added too.
Incorporating suggestion from Gustavo Padovan.
Signed-off-by: Greg Dietsche <Gregory.Dietsche@cuw.edu>
---
drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
index 24d1499..9b65153 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
@@ -2275,6 +2275,9 @@ iwl4965_rs_get_rate(void *priv_r, struct ieee80211_sta *sta, void *priv_sta,
if (rate_control_send_low(sta, priv_sta, txrc))
return;
+ if (!lq_sta)
+ return;
+
rate_idx = lq_sta->last_txrate_idx;
if (lq_sta->last_rate_n_flags & RATE_MCS_HT_MSK) {
--
1.7.2.5
^ permalink raw reply related
* Just 4 U
From: BballPlayaIY25 @ 2011-06-03 3:15 UTC (permalink / raw)
To: netdev
Meet Hot Local Singles Free Join.! <a href="http://korta.nu/3dkun">Click
Here</a>
^ permalink raw reply
* Re: [PATCH 5/5] iwlegacy: add missing null check
From: Gustavo F. Padovan @ 2011-06-03 3:07 UTC (permalink / raw)
To: Greg Dietsche
Cc: sgruszka, linville, kernel-janitors, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1307066770-27309-5-git-send-email-Gregory.Dietsche@cuw.edu>
Hi Greg,
* Greg Dietsche <Gregory.Dietsche@cuw.edu> [2011-06-02 21:06:10 -0500]:
> lq_sta has other null checks in this funcion.
> assuming they are correct, this additional null check
> should be added too.
>
> Signed-off-by: Greg Dietsche <Gregory.Dietsche@cuw.edu>
> ---
> drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 66 ++++++++++++++-------------
> 1 files changed, 34 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> index 24d1499..a475aac 100644
> --- a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> +++ b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
> @@ -2275,40 +2275,42 @@ iwl4965_rs_get_rate(void *priv_r, struct ieee80211_sta *sta, void *priv_sta,
> if (rate_control_send_low(sta, priv_sta, txrc))
> return;
>
> - rate_idx = lq_sta->last_txrate_idx;
> -
> - if (lq_sta->last_rate_n_flags & RATE_MCS_HT_MSK) {
> - rate_idx -= IWL_FIRST_OFDM_RATE;
> - /* 6M and 9M shared same MCS index */
> - rate_idx = (rate_idx > 0) ? (rate_idx - 1) : 0;
> - if (iwl4965_rs_extract_rate(lq_sta->last_rate_n_flags) >=
> - IWL_RATE_MIMO2_6M_PLCP)
> - rate_idx = rate_idx + MCS_INDEX_PER_STREAM;
> - info->control.rates[0].flags = IEEE80211_TX_RC_MCS;
> - if (lq_sta->last_rate_n_flags & RATE_MCS_SGI_MSK)
> - info->control.rates[0].flags |=
> - IEEE80211_TX_RC_SHORT_GI;
> - if (lq_sta->last_rate_n_flags & RATE_MCS_DUP_MSK)
> - info->control.rates[0].flags |=
> - IEEE80211_TX_RC_DUP_DATA;
> - if (lq_sta->last_rate_n_flags & RATE_MCS_HT40_MSK)
> - info->control.rates[0].flags |=
> - IEEE80211_TX_RC_40_MHZ_WIDTH;
> - if (lq_sta->last_rate_n_flags & RATE_MCS_GF_MSK)
> - info->control.rates[0].flags |=
> - IEEE80211_TX_RC_GREEN_FIELD;
> - } else {
> - /* Check for invalid rates */
> - if ((rate_idx < 0) || (rate_idx >= IWL_RATE_COUNT_LEGACY) ||
> - ((sband->band == IEEE80211_BAND_5GHZ) &&
> - (rate_idx < IWL_FIRST_OFDM_RATE)))
> - rate_idx = rate_lowest_index(sband, sta);
> - /* On valid 5 GHz rate, adjust index */
> - else if (sband->band == IEEE80211_BAND_5GHZ)
> + if (lq_sta) {
Then do
if (!lq_sta)
return;
to avoid an exatra level of indentation.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* [PATCH 5/5] iwlegacy: add missing null check
From: Greg Dietsche @ 2011-06-03 2:06 UTC (permalink / raw)
To: sgruszka, linville
Cc: kernel-janitors, linux-wireless, netdev, linux-kernel,
Greg Dietsche
In-Reply-To: <1307066770-27309-1-git-send-email-Gregory.Dietsche@cuw.edu>
lq_sta has other null checks in this funcion.
assuming they are correct, this additional null check
should be added too.
Signed-off-by: Greg Dietsche <Gregory.Dietsche@cuw.edu>
---
drivers/net/wireless/iwlegacy/iwl-4965-rs.c | 66 ++++++++++++++-------------
1 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
index 24d1499..a475aac 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965-rs.c
@@ -2275,40 +2275,42 @@ iwl4965_rs_get_rate(void *priv_r, struct ieee80211_sta *sta, void *priv_sta,
if (rate_control_send_low(sta, priv_sta, txrc))
return;
- rate_idx = lq_sta->last_txrate_idx;
-
- if (lq_sta->last_rate_n_flags & RATE_MCS_HT_MSK) {
- rate_idx -= IWL_FIRST_OFDM_RATE;
- /* 6M and 9M shared same MCS index */
- rate_idx = (rate_idx > 0) ? (rate_idx - 1) : 0;
- if (iwl4965_rs_extract_rate(lq_sta->last_rate_n_flags) >=
- IWL_RATE_MIMO2_6M_PLCP)
- rate_idx = rate_idx + MCS_INDEX_PER_STREAM;
- info->control.rates[0].flags = IEEE80211_TX_RC_MCS;
- if (lq_sta->last_rate_n_flags & RATE_MCS_SGI_MSK)
- info->control.rates[0].flags |=
- IEEE80211_TX_RC_SHORT_GI;
- if (lq_sta->last_rate_n_flags & RATE_MCS_DUP_MSK)
- info->control.rates[0].flags |=
- IEEE80211_TX_RC_DUP_DATA;
- if (lq_sta->last_rate_n_flags & RATE_MCS_HT40_MSK)
- info->control.rates[0].flags |=
- IEEE80211_TX_RC_40_MHZ_WIDTH;
- if (lq_sta->last_rate_n_flags & RATE_MCS_GF_MSK)
- info->control.rates[0].flags |=
- IEEE80211_TX_RC_GREEN_FIELD;
- } else {
- /* Check for invalid rates */
- if ((rate_idx < 0) || (rate_idx >= IWL_RATE_COUNT_LEGACY) ||
- ((sband->band == IEEE80211_BAND_5GHZ) &&
- (rate_idx < IWL_FIRST_OFDM_RATE)))
- rate_idx = rate_lowest_index(sband, sta);
- /* On valid 5 GHz rate, adjust index */
- else if (sband->band == IEEE80211_BAND_5GHZ)
+ if (lq_sta) {
+ rate_idx = lq_sta->last_txrate_idx;
+
+ if (lq_sta->last_rate_n_flags & RATE_MCS_HT_MSK) {
rate_idx -= IWL_FIRST_OFDM_RATE;
- info->control.rates[0].flags = 0;
+ /* 6M and 9M shared same MCS index */
+ rate_idx = (rate_idx > 0) ? (rate_idx - 1) : 0;
+ if (iwl4965_rs_extract_rate(lq_sta->last_rate_n_flags) >=
+ IWL_RATE_MIMO2_6M_PLCP)
+ rate_idx = rate_idx + MCS_INDEX_PER_STREAM;
+ info->control.rates[0].flags = IEEE80211_TX_RC_MCS;
+ if (lq_sta->last_rate_n_flags & RATE_MCS_SGI_MSK)
+ info->control.rates[0].flags |=
+ IEEE80211_TX_RC_SHORT_GI;
+ if (lq_sta->last_rate_n_flags & RATE_MCS_DUP_MSK)
+ info->control.rates[0].flags |=
+ IEEE80211_TX_RC_DUP_DATA;
+ if (lq_sta->last_rate_n_flags & RATE_MCS_HT40_MSK)
+ info->control.rates[0].flags |=
+ IEEE80211_TX_RC_40_MHZ_WIDTH;
+ if (lq_sta->last_rate_n_flags & RATE_MCS_GF_MSK)
+ info->control.rates[0].flags |=
+ IEEE80211_TX_RC_GREEN_FIELD;
+ } else {
+ /* Check for invalid rates */
+ if ((rate_idx < 0) || (rate_idx >= IWL_RATE_COUNT_LEGACY) ||
+ ((sband->band == IEEE80211_BAND_5GHZ) &&
+ (rate_idx < IWL_FIRST_OFDM_RATE)))
+ rate_idx = rate_lowest_index(sband, sta);
+ /* On valid 5 GHz rate, adjust index */
+ else if (sband->band == IEEE80211_BAND_5GHZ)
+ rate_idx -= IWL_FIRST_OFDM_RATE;
+ info->control.rates[0].flags = 0;
+ }
+ info->control.rates[0].idx = rate_idx;
}
- info->control.rates[0].idx = rate_idx;
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH 4/5] iwlegacy: propagate error return value
From: Greg Dietsche @ 2011-06-03 2:06 UTC (permalink / raw)
To: sgruszka, linville
Cc: kernel-janitors, linux-wireless, netdev, linux-kernel,
Greg Dietsche
In-Reply-To: <1307066770-27309-1-git-send-email-Gregory.Dietsche@cuw.edu>
propogate the return value from iwl4965_get_tx_atten_grp instead
of implicitly returning -EINVAL in the error case.
Signed-off-by: Greg Dietsche <Gregory.Dietsche@cuw.edu>
---
drivers/net/wireless/iwlegacy/iwl-4965.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/iwlegacy/iwl-4965.c b/drivers/net/wireless/iwlegacy/iwl-4965.c
index 06dcee5..79e7ce6 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965.c
@@ -915,7 +915,7 @@ static int iwl4965_fill_txpower_tbl(struct iwl_priv *priv, u8 band, u16 channel,
if (txatten_grp < 0) {
IWL_ERR(priv, "Can't find txatten group for channel %d.\n",
channel);
- return -EINVAL;
+ return txatten_grp;
}
IWL_DEBUG_TXPOWER(priv, "channel %d belongs to txatten group %d\n",
--
1.7.2.5
^ permalink raw reply related
* [PATCH 3/5] iwlegacy: return -EINVAL instead of -1
From: Greg Dietsche @ 2011-06-03 2:06 UTC (permalink / raw)
To: sgruszka, linville
Cc: kernel-janitors, linux-wireless, netdev, linux-kernel,
Greg Dietsche
In-Reply-To: <1307066770-27309-1-git-send-email-Gregory.Dietsche@cuw.edu>
Cleanup the code to return -EINVAL instead of -1
Signed-off-by: Greg Dietsche <Gregory.Dietsche@cuw.edu>
---
drivers/net/wireless/iwlegacy/iwl-4965.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/iwlegacy/iwl-4965.c b/drivers/net/wireless/iwlegacy/iwl-4965.c
index ffda05c..06dcee5 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965.c
@@ -496,7 +496,7 @@ static s32 iwl4965_get_tx_atten_grp(u16 channel)
channel <= CALIB_IWL_TX_ATTEN_GR4_LCH)
return CALIB_CH_GROUP_4;
- return -1;
+ return -EINVAL;
}
static u32 iwl4965_get_sub_band(const struct iwl_priv *priv, u32 channel)
--
1.7.2.5
^ permalink raw reply related
* [PATCH 2/5] iwlegacy: remove unecessary if statement
From: Greg Dietsche @ 2011-06-03 2:06 UTC (permalink / raw)
To: sgruszka, linville
Cc: kernel-janitors, linux-wireless, netdev, linux-kernel,
Greg Dietsche
In-Reply-To: <1307066770-27309-1-git-send-email-Gregory.Dietsche@cuw.edu>
the code always returns ret regardless, so if(ret) check is unecessary.
Signed-off-by: Greg Dietsche <Gregory.Dietsche@cuw.edu>
---
drivers/net/wireless/iwlegacy/iwl-4965.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/iwlegacy/iwl-4965.c b/drivers/net/wireless/iwlegacy/iwl-4965.c
index f5433c7..ffda05c 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965.c
@@ -1185,8 +1185,6 @@ static int iwl4965_send_rxon_assoc(struct iwl_priv *priv,
ret = iwl_legacy_send_cmd_pdu_async(priv, REPLY_RXON_ASSOC,
sizeof(rxon_assoc), &rxon_assoc, NULL);
- if (ret)
- return ret;
return ret;
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH 1/5] iwlegacy: remove unreachable code
From: Greg Dietsche @ 2011-06-03 2:06 UTC (permalink / raw)
To: sgruszka, linville
Cc: kernel-janitors, linux-wireless, netdev, linux-kernel,
Greg Dietsche
return; at the end of the function is unecessary.
Signed-off-by: Greg Dietsche <Gregory.Dietsche@cuw.edu>
---
drivers/net/wireless/iwlegacy/iwl-eeprom.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/iwlegacy/iwl-eeprom.c b/drivers/net/wireless/iwlegacy/iwl-eeprom.c
index cb346d1..5bf3f49 100644
--- a/drivers/net/wireless/iwlegacy/iwl-eeprom.c
+++ b/drivers/net/wireless/iwlegacy/iwl-eeprom.c
@@ -316,7 +316,6 @@ static void iwl_legacy_init_band_reference(const struct iwl_priv *priv,
break;
default:
BUG();
- return;
}
}
--
1.7.2.5
^ permalink raw reply related
* Re: [TRIVIAL PATCH next 15/15] net: Convert vmalloc/memset to vzalloc
From: Joe Perches @ 2011-06-03 1:35 UTC (permalink / raw)
To: David Miller
Cc: pablo, kaber, andy.grover, trivial, netfilter-devel, netfilter,
coreteam, netdev, linux-kernel, rds-devel
In-Reply-To: <20110602.143927.1073346459383432955.davem@davemloft.net>
On Thu, 2011-06-02 at 14:39 -0700, David Miller wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Thu, 02 Jun 2011 16:49:53 +0200
> > Are you going to take this patch? it includes one chunk which is out of
> > netfilter scope.
> I think the trivial folks submit these things via their own tree
> and are only looking for ACKs from us.
> Acked-by: David S. Miller <davem@davemloft.net>
Getting trivial patches applied between the various
maintainers and the trivial tree is most often a bit
uncoordinated and haphazard.
Doing micropatches for each subsystem maintainer
does sometimes appear to be detrimental to getting
any treewide modification applied.
I've experimented with both micropatches and
treewide patches to gauge what happens.
I'm not sure either approach is better or worse.
In any case, it commonly takes several submittals
for these sort of trivial patches to get applied
either by the subtree maintainer or by Jiri.
Jiri often waits a few weeks for patches to get
picked up by any subtree maintainer before looking
at his trivial emails. Sometimes he acks what's
left, sometimes what's left just disappears into
the ethervoid waiting for the submitter to try
again.
No worries, that's just the way it is.
^ permalink raw reply
* Re: [net-next,02/13] tg3: Cleanup transmit error path
From: Alex Williamson @ 2011-06-03 1:18 UTC (permalink / raw)
To: Matt Carlson; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20110602230139.GD5068@mcarlson.broadcom.com>
On Thu, 2011-06-02 at 16:01 -0700, Matt Carlson wrote:
> On Thu, Jun 02, 2011 at 03:18:35PM -0700, Alex Williamson wrote:
> > On Thu, 2011-05-19 at 12:12 +0000, Matt Carlson wrote:
> > > This patch consolidates the skb cleanup code into a function named
> > > tg3_skb_error_unmap(). The modification addresses a long-standing bug
> > > where pci_unmap_single() was incorrectly being called instead of
> > > pci_unmap_page() in tigon3_dma_hwbug_workaround().
> >
> > This patch doesn't behave well when an iommu (VT-d) is involved.
> > Booting an X58 chipset based system with intel_iommu=on with 3.0.0-rc1
> > results in the warning below. Looks like the driver is trying to unmap
> > 0x0. Eventually something worse happens and the system reports a few
> > iommu faults from the device before panicing. Neither problem is
> > observed if 432aa7ed is reverted. tg3 device in use is a BCM5755.
> > Thanks,
> >
> > Alex
> >
> > WARNING: at drivers/pci/intel-iommu.c:2888 intel_unmap_page+0x15c/0x180()
> > Hardware name: 4157CTO
> > Driver unmaps unmatched page at PFN 0
> > Modules linked in: nfs lockd auth_rpcgss nfs_acl ipt_MASQUERADE iptable_nat nf_nat iptable_mangle tun autofs4 sunrpc cpufreq_ondemand acpi_cpufreq freq_table mperf bridge stp llc ipv6 dm_mirror dm_region_hash dm_log kvm_intel kvm sg microcode serio_raw wmi i2c_i801 i2c_core iTCO_wdt iTCO_vendor_support tg3 ext4 mbcache jbd2 raid1 raid456 async_pq async_xor xor async_memcpy async_raid6_recov raid6_pq async_tx sr_mod cdrom sd_mod crc_t10dif floppy dm_mod [last unloaded: scsi_wait_scan]
> > Pid: 2604, comm: libvirtd Not tainted 3.0.0-rc1+ #184
> > Call Trace:
> > [<ffffffff8105c7df>] warn_slowpath_common+0x7f/0xc0
> > [<ffffffff8105c8d6>] warn_slowpath_fmt+0x46/0x50
> > [<ffffffff81245b8b>] ? find_iova+0x5b/0xa0
> > [<ffffffff8124a60c>] intel_unmap_page+0x15c/0x180
> > [<ffffffffa0128d48>] tg3_skb_error_unmap+0xb8/0x130 [tg3]
> Does this patch fix the problem?
Yes it does, probably explains the subsequent iommu faults if that off
by one unmapping sometimes finds a non-zero address to unmap. Thanks,
Alex
Tested-by: Alex Williamson <alex.williamson@redhat.com>
> Subject: [PATCH] tg3: Fix tg3_skb_error_unmap()
>
> This function attempts to free one fragment beyond the number of
> fragments that were actually mapped. This patch brings back the limit
> to the correct spot.
>
> Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> ---
> drivers/net/tg3.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index 8ece39b..3d239ab 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -5771,7 +5771,7 @@ static void tg3_skb_error_unmap(struct tg3_napi *tnapi,
> dma_unmap_addr(txb, mapping),
> skb_headlen(skb),
> PCI_DMA_TODEVICE);
> - for (i = 0; i <= last; i++) {
> + for (i = 0; i < last; i++) {
> skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
>
> entry = NEXT_TX(entry);
> --
> 1.7.3.4
>
>
^ permalink raw reply
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device
From: Neil Horman @ 2011-06-03 1:16 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1307045604.2812.41.camel@bwh-desktop>
On Thu, Jun 02, 2011 at 09:13:24PM +0100, Ben Hutchings wrote:
> On Thu, 2011-06-02 at 15:46 -0400, Neil Horman wrote:
> > On Thu, Jun 02, 2011 at 08:09:49PM +0100, Ben Hutchings wrote:
> > > On Thu, 2011-06-02 at 14:56 -0400, Neil Horman wrote:
> > > > On Thu, Jun 02, 2011 at 07:35:53PM +0100, Ben Hutchings wrote:
> > > > > On Thu, 2011-06-02 at 14:03 -0400, Neil Horman wrote:
> > > > > > The bonding driver is multiqueue enabled, in which each queue represents a slave
> > > > > > to enable optional steering of output frames to given slaves against the default
> > > > > > output policy. However, it needs to reset the skb->queue_mapping prior to
> > > > > > queuing to the physical device or the physical slave (if it is multiqueue) could
> > > > > > wind up transmitting on an unintended tx queue (one that was reserved for
> > > > > > specific traffic classes for instance)
> > > [...]
> > > > > So far as I can see, this has no effect, because dev_queue_xmit() always
> > > > > sets queue_mapping (in dev_pick_tx()).
> > > > >
> > > > it resets the queue mapping exactly as you would expect it to. bonding is a
> > > > multiqueue enabled device and selects a potentially non-zero queue based on the
> > > > output of bond_select_queue.
> > > >
> > > > > What is the problem you're seeing?
> > > > >
> > > > The problem is exctly that. dev_pick_tx() on the bond device sets the
> > > > queue_mapping as per the result of bond_select_queue (the ndo_select_queue
> > > > method for the bonding driver). The implementation there is based on the use of
> > > > tc with bonding, so that output slaves can be selected for certain types of
> > > > traffic. But when that mechanism is used, skb->queue_mapping is preserved when
> > > > the bonding driver queues the frame to the underlying slave. This denies the
> > > > slave (if its also a multiqueue device) the opportunity to reselect the queue
> > > > properly, because of this call path:
> > > >
> > > > bond_queue_xmit
> > > > dev_queue_xmit(slave_dev)
> > > > dev_pick_tx()
> > > > skb_tx_hash()
> > > > __skb_tx_hash()
> > > >
> > > > __skb_tx_hash sees that skb_queue_recorded returns true, and assigns a hardware queue mapping
> > > > based on what the bonding driver chose using its own internal logic. Since
> > > > bonding uses the multiqueue infrastructure to do slave output selection without
> > > > any regard for slave output queue selection, it seems to me we should really
> > > > reset the queue mapping to zero so the slave device can pick its own tx queue.
> > >
> > > So you're effectively clearing the *RX queue* number (as this is before
> > > dev_pick_tx()) in order to influence TX queue selection.
> > >
> > No, you're not seeing it properly. In bonding (as with all stacked devices) we
> > make two passes through dev_pick_tx.
>
> Only if the stacked device has its own software queues and schedulers;
> e.g. VLAN devices don't.
>
Every net_device has a default fifo qdisc, and as such, each net_device when
called with dev_queue_xmit calls dev_pick_tx, and that includes vlans. It just
so happens that vlans are a single queue device and come out of dev_pick_tx with
a queue_mapping of 0. Thats all moot of course, since we're talking about
bonding.
> > 1) The first time we call dev_pick_tx is when the network stack calls
> > dev_queue_xmit on the bond device. here we call ndo_select_queue, which calls
> > bond_select_queue. This method tells the stack which queue to enqueue the skb
> > on for the bond device. We can use tc's skbedit action to select a particular
> > queue on the bond device for various classes of traffic, and those queues
> > correspond to individual slave devices.
> >
> > 2) the second time we call dev_pick_tx is when the bonding bond_queue_xmit
> > routine (which is the bonding drivers ndo_start_xmit method, called after the
> > frist dev_pick_tx), calls dev_queue_xmit, passing the slave net_device pointer).
> > In this case we do whatever the slave device has configured (either reset the
> > queue to zero, or call the slaves ndo_select queue method).
> >
> > What I'm fixing is the fact that the bonding drivers queue_mappnig value is
> > 'leaking' down to the slave.
>
> And this is because dev_queue_xmit() assumes that it's a record of the
> RX queue number.
>
Yes, correct. stacked devices provide the opportuninty for queue_mapping to no
longer be that rx queue number, but rather a tx queue mapping. This is a
potentential problem with all stacked devices, but bonding is the only thing
showing it right now because its the only stacked device that can set a non-zero
queue_mapping value.
> The differing interpretation of queue_mapping for RX and TX is annoying
> and I think we should change the initial value of queue_mapping to -1.
> But that's a separate issue.
>
Agreed.
> > Lets say, for example we're bonding two multiqueue devices, and have the bonding
> > driver configured to send all traffic to 192.168.1.1 over the first slave (which
> > we can accomplish using an appropriate tc rule on the bond device, setting
> > frames with that dest ip to have a skb->queue_mapping of 1).
> >
> > In the above example, frames bound for 192.168.1.1 have skb->queue_mapping set
> > to 1 when they enter bond_queue_xmit. bond_queue_xmit, calls
> > dev_queue_xmit(slave, skb), which calls dev_pick_tx(slave, skb). If we (for
> > simplicity's sake) assume the slave has no ndo_select_queue method, dev_pick_tx
> > calls __skb_tx_hash to determine the output queue. But the bonding driver
> > already set skb->queue_mapping to 1 (because it wanted this frame output on the
> > first slave, not because it wanted to transmit the frame on the slaves tx queue
> > 1). Nevertheless, __skb_tx_hash() sees that skb_rx_queue_recorded returns true
> > here and as a result, calls skb_rx_get_queue, which returns 0. Because of that
> > we wind up transmitting on hardware queue 0 all the time, which is not at all
> > what we want. What we want is for the bonding driver to be able to use
> > queue_mapping for its own purposes, and then allow the physical device to make
> > its own output queue selection independently.
> [...]
>
> I think I understand - the bonding device is effectively single-queue
> and shouldn't record an RX queue number. But I think you should define
Well, no, its a multiqueue device, it just happens to treat slaves as queues,
rather than internal hardware objects. But I suppose you're saying the same
thing that dave is, in that we should restore the origional queue_mapping,
rather than clearing it to preserve the input semantics when we reach the
physical device. I can do that.
Neil
^ permalink raw reply
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device
From: Neil Horman @ 2011-06-03 1:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, fubar, andy
In-Reply-To: <20110602.130710.1904222486883754792.davem@davemloft.net>
On Thu, Jun 02, 2011 at 01:07:10PM -0700, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Thu, 2 Jun 2011 14:03:19 -0400
>
> > The bonding driver is multiqueue enabled, in which each queue represents a slave
> > to enable optional steering of output frames to given slaves against the default
> > output policy. However, it needs to reset the skb->queue_mapping prior to
> > queuing to the physical device or the physical slave (if it is multiqueue) could
> > wind up transmitting on an unintended tx queue (one that was reserved for
> > specific traffic classes for instance)
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>
> Since, as I mentioned, the idea when we are forwarding and bridging is that
> we use the input receive classification to influence the spread on transmit,
> I think things like this bonding case should remember the rxhash setting
> before they override it and then restore that value right before invoking
> dev_queue_xmit().
>
Ok, I can respin the patch to do that. I'll handle it in the AM.
Thanks
Neil
^ permalink raw reply
* Re: [PATCH] TODO FLAG_POINTTOPOINT => FLAG_WWAN? usbnet/cdc_ncm: mark ncm devices as "mobile broadband devices" with FLAG_WWAN
From: Valdis.Kletnieks @ 2011-06-03 0:58 UTC (permalink / raw)
To: Dan Williams
Cc: Alexey ORISHKO, Stefan Metzmacher, Oliver Neukum,
Greg Kroah-Hartman, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1307054132.21633.25.camel@dcbw.foobar.com>
[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]
On Thu, 02 Jun 2011 17:35:31 CDT, Dan Williams said:
> On Thu, 2011-06-02 at 17:29 -0500, Dan Williams wrote:
> > On Wed, 2011-06-01 at 12:20 +0200, Alexey ORISHKO wrote:
> > > > -----Original Message-----
> > > > From: netdev-owner@vger.kernel.org [mailto:netdev-
> > > > owner@vger.kernel.org] On Behalf Of Stefan Metzmacher
> > > > Sent: Wednesday, June 01, 2011 12:09 PM
> > >
> > >
> > > > - .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
> > > > + .flags = FLAG_WWAN | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
> > >
> > > This patch will introduce incompatibility with already existing
> > > applications, which track usbX devices. As a result, end user
> > > application will stop working.
I'm interpreting this as "devices which currently often get named usbX by udev,
and currently have FLAG_POINTTOPOINT set". People are getting hung up
on the usbX part, not what the *real* problem is.
> A follow-on to this is that if you really care about specific devices,
> your application can use udev rules to "tag" specific interfaces based
> on USB VID/PID/GUID or other device attributes, and check for those tags
> in your program. Use udev (good) or netlink (good) or SIOCGIFCONF (bad)
> to enumerate the various network interfaces on the system and pick the
I think Alexey's point was that the patch will hose up programs that
currently do the netlink or SIOCGIFCONF thing and look for FLAG_POINTTOPOINT.
[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply
* Re: inet_diag insufficient validation?
From: Dan Rosenberg @ 2011-06-03 0:50 UTC (permalink / raw)
To: davem; +Cc: kuznet, netdev, security
In-Reply-To: <1306942849.3150.10.camel@dan>
On Wed, 2011-06-01 at 11:40 -0400, Dan Rosenberg wrote:
> Finally, I can't seem to find any validation that the reported length of
> the netlink message header doesn't exceed the skb length, as checked in
> some other netlink receive functions, which could result in reading
> beyond the bounds of the socket data. I could just be missing something
> here though.
>
And for the second time, I was missing something - this validation
happens in netlink_rcv_skb().
That leaves the infinite loop in bytecode auditing, which I've confirmed
via reproducer.
-Dan
^ permalink raw reply
* Re: [PATCH] vlan: Fix the ingress VLAN_FLAG_REORDER_HDR check v2
From: Changli Gao @ 2011-06-02 23:18 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David Miller, shemminger, greearb, nicolas.2p.debian, jpirko,
netdev, kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <m1ipsob6f8.fsf@fess.ebiederm.org>
On Thu, Jun 2, 2011 at 11:26 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Changli Gao <xiaosuo@gmail.com> writes:
>
>> On Thu, Jun 2, 2011 at 9:03 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>>
>>> -static struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
>>> +static struct sk_buff *vlan_reorder_header(struct sk_buff *skb)
>>> {
>>> - if (vlan_dev_info(skb->dev)->flags & VLAN_FLAG_REORDER_HDR) {
>>> - if (skb_cow(skb, skb_headroom(skb)) < 0)
>>> - skb = NULL;
>>> - if (skb) {
>>> - /* Lifted from Gleb's VLAN code... */
>>> - memmove(skb->data - ETH_HLEN,
>>> - skb->data - VLAN_ETH_HLEN, 12);
>>> - skb->mac_header += VLAN_HLEN;
>>> - }
>>> + if (skb_cow(skb, skb_headroom(skb)) < 0)
>>> + skb = NULL;
>>> + if (skb) {
>>
>> I think an else branch maybe more readable here.
>
> Probably most readable would be simply returning NULL immediately on
> error. In this patch I just removed the if statement, and I would like
> to avoid mixing different bug fixes in the same patch if possible.
>
OK. It is minor.
>>> + /* Lifted from Gleb's VLAN code... */
>>> + memmove(skb->data - ETH_HLEN,
>>> + skb->data - VLAN_ETH_HLEN, 12);
>>> + skb->mac_header += VLAN_HLEN;
>>
>> skb->mac_len should be adjusted too.
>
> Given how vlan_untag is called at the moment it does appear
> that the skb->mac_len = skb->network_header - skb->mac_header
> in __netif_receive_skb that used to handle this for is no longer
> doing this for us.
>
> My feel is that either we need to do all of the header resets and the
> vlan_untagging together. So we either need this all together before or
> after the another_round label:
>
> So the proper fix is probably something like this.
>
OK, it is right. Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [net-next,02/13] tg3: Cleanup transmit error path
From: Matt Carlson @ 2011-06-02 23:01 UTC (permalink / raw)
To: Alex Williamson
Cc: Matthew Carlson, davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <1307053115.3656.19.camel@x201>
On Thu, Jun 02, 2011 at 03:18:35PM -0700, Alex Williamson wrote:
> On Thu, 2011-05-19 at 12:12 +0000, Matt Carlson wrote:
> > This patch consolidates the skb cleanup code into a function named
> > tg3_skb_error_unmap(). The modification addresses a long-standing bug
> > where pci_unmap_single() was incorrectly being called instead of
> > pci_unmap_page() in tigon3_dma_hwbug_workaround().
>
> This patch doesn't behave well when an iommu (VT-d) is involved.
> Booting an X58 chipset based system with intel_iommu=on with 3.0.0-rc1
> results in the warning below. Looks like the driver is trying to unmap
> 0x0. Eventually something worse happens and the system reports a few
> iommu faults from the device before panicing. Neither problem is
> observed if 432aa7ed is reverted. tg3 device in use is a BCM5755.
> Thanks,
>
> Alex
>
> WARNING: at drivers/pci/intel-iommu.c:2888 intel_unmap_page+0x15c/0x180()
> Hardware name: 4157CTO
> Driver unmaps unmatched page at PFN 0
> Modules linked in: nfs lockd auth_rpcgss nfs_acl ipt_MASQUERADE iptable_nat nf_nat iptable_mangle tun autofs4 sunrpc cpufreq_ondemand acpi_cpufreq freq_table mperf bridge stp llc ipv6 dm_mirror dm_region_hash dm_log kvm_intel kvm sg microcode serio_raw wmi i2c_i801 i2c_core iTCO_wdt iTCO_vendor_support tg3 ext4 mbcache jbd2 raid1 raid456 async_pq async_xor xor async_memcpy async_raid6_recov raid6_pq async_tx sr_mod cdrom sd_mod crc_t10dif floppy dm_mod [last unloaded: scsi_wait_scan]
> Pid: 2604, comm: libvirtd Not tainted 3.0.0-rc1+ #184
> Call Trace:
> [<ffffffff8105c7df>] warn_slowpath_common+0x7f/0xc0
> [<ffffffff8105c8d6>] warn_slowpath_fmt+0x46/0x50
> [<ffffffff81245b8b>] ? find_iova+0x5b/0xa0
> [<ffffffff8124a60c>] intel_unmap_page+0x15c/0x180
> [<ffffffffa0128d48>] tg3_skb_error_unmap+0xb8/0x130 [tg3]
> [<ffffffffa0136041>] tg3_start_xmit+0x661/0xde0 [tg3]
> [<ffffffffa01363dd>] ? tg3_start_xmit+0x9fd/0xde0 [tg3]
> [<ffffffff8138d7fe>] dev_hard_start_xmit+0x31e/0x6c0
> [<ffffffff813a9f5f>] sch_direct_xmit+0xef/0x1c0
> [<ffffffff8138dd38>] dev_queue_xmit+0x198/0x5f0
> [<ffffffffa028d2ac>] br_dev_queue_push_xmit+0x6c/0xa0 [bridge]
> [<ffffffffa028d478>] br_forward_finish+0x58/0x60 [bridge]
> [<ffffffffa028d4f8>] __br_deliver+0x78/0xf0 [bridge]
> [<ffffffff814367a9>] ? packet_rcv+0x59/0x450
> [<ffffffffa028d5a5>] br_deliver+0x35/0x40 [bridge]
> [<ffffffffa028b8c4>] br_dev_xmit+0x114/0x180 [bridge]
> [<ffffffff8138d7fe>] dev_hard_start_xmit+0x31e/0x6c0
> [<ffffffff8138df95>] dev_queue_xmit+0x3f5/0x5f0
> [<ffffffff813d0c2e>] ip_finish_output+0x16e/0x340
> [<ffffffff813d1110>] ip_output+0xb0/0xc0
> [<ffffffff813cfdc3>] ? __ip_local_out+0xa3/0xb0
> [<ffffffff813d0129>] ip_local_out+0x29/0x30
> [<ffffffff813d0604>] ip_queue_xmit+0x164/0x410
> [<ffffffff813e5e5c>] tcp_transmit_skb+0x41c/0x910
> [<ffffffff813e86f7>] tcp_write_xmit+0x1e7/0x9d0
> [<ffffffff813e8f46>] __tcp_push_pending_frames+0x26/0xc0
> [<ffffffff813d77be>] tcp_push+0x6e/0x90
> [<ffffffff813db4f9>] tcp_sendmsg+0x759/0xc00
> [<ffffffff813fdaa8>] inet_sendmsg+0x48/0xb0
> [<ffffffff811cd7f3>] ? selinux_socket_sendmsg+0x23/0x30
> [<ffffffff81377e52>] sock_sendmsg+0xe2/0x120
> [<ffffffff8100a7e4>] ? __switch_to+0x194/0x320
> [<ffffffff81377ed1>] kernel_sendmsg+0x41/0x60
Does this patch fix the problem?
Subject: [PATCH] tg3: Fix tg3_skb_error_unmap()
This function attempts to free one fragment beyond the number of
fragments that were actually mapped. This patch brings back the limit
to the correct spot.
Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
---
drivers/net/tg3.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 8ece39b..3d239ab 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -5771,7 +5771,7 @@ static void tg3_skb_error_unmap(struct tg3_napi *tnapi,
dma_unmap_addr(txb, mapping),
skb_headlen(skb),
PCI_DMA_TODEVICE);
- for (i = 0; i <= last; i++) {
+ for (i = 0; i < last; i++) {
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
entry = NEXT_TX(entry);
--
1.7.3.4
^ permalink raw reply related
* [PATCH] qlcnic: Fix bug in FW queue dump
From: Anirban Chakraborty @ 2011-06-02 22:42 UTC (permalink / raw)
To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Anirban Chakraborty
Due to a change in FW template, a bug was introduced in dump queue entries. This is
fixed by reinitializing queue address before looping for each que dump operation.
Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
drivers/net/qlcnic/qlcnic_hw.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
index e965661..a5d9fbf 100644
--- a/drivers/net/qlcnic/qlcnic_hw.c
+++ b/drivers/net/qlcnic/qlcnic_hw.c
@@ -1406,6 +1406,7 @@ qlcnic_dump_que(struct qlcnic_adapter *adapter, struct qlcnic_dump_entry *entry,
for (loop = 0; loop < que->no_ops; loop++) {
QLCNIC_WR_DUMP_REG(que->sel_addr, base, que_id);
+ addr = que->read_addr;
for (i = 0; i < cnt; i++) {
QLCNIC_RD_DUMP_REG(addr, base, &data);
*buffer++ = cpu_to_le32(data);
--
1.7.4.1
^ permalink raw reply related
* RE: [PATCH] TODO FLAG_POINTTOPOINT => FLAG_WWAN? usbnet/cdc_ncm: mark ncm devices as "mobile broadband devices" with FLAG_WWAN
From: Dan Williams @ 2011-06-02 22:35 UTC (permalink / raw)
To: Alexey ORISHKO
Cc: Stefan Metzmacher, Oliver Neukum, Greg Kroah-Hartman,
linux-usb@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1307053743.21633.22.camel@dcbw.foobar.com>
On Thu, 2011-06-02 at 17:29 -0500, Dan Williams wrote:
> On Wed, 2011-06-01 at 12:20 +0200, Alexey ORISHKO wrote:
> > > -----Original Message-----
> > > From: netdev-owner@vger.kernel.org [mailto:netdev-
> > > owner@vger.kernel.org] On Behalf Of Stefan Metzmacher
> > > Sent: Wednesday, June 01, 2011 12:09 PM
> >
> >
> > > - .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
> > > + .flags = FLAG_WWAN | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
> >
> > This patch will introduce incompatibility with already existing
> > applications, which track usbX devices. As a result, end user
> > application will stop working.
>
> Applications should *never* track devices based solely on a device name
> prefix. What do they do when the device gets renamed either by udev
> rules or the user? It's simply broken. Device names are not stable API
> and they can and do change at will. Applications that expect them to
> have a stable prefix are simply broken.
A follow-on to this is that if you really care about specific devices,
your application can use udev rules to "tag" specific interfaces based
on USB VID/PID/GUID or other device attributes, and check for those tags
in your program. Use udev (good) or netlink (good) or SIOCGIFCONF (bad)
to enumerate the various network interfaces on the system and pick the
one you want using the udev tags instead of hardcoding stuff that will
inevitably break, as you've seen here. Yeah, it's more code. But hey,
it doesn't break when people do something you don't expect!
Dan
^ permalink raw reply
* RE: [PATCH] TODO FLAG_POINTTOPOINT => FLAG_WWAN? usbnet/cdc_ncm: mark ncm devices as "mobile broadband devices" with FLAG_WWAN
From: Dan Williams @ 2011-06-02 22:29 UTC (permalink / raw)
To: Alexey ORISHKO
Cc: Stefan Metzmacher, Oliver Neukum, Greg Kroah-Hartman,
linux-usb@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <2AC7D4AD8BA1C640B4C60C61C8E520153E3C07E583@EXDCVYMBSTM006.EQ1STM.local>
On Wed, 2011-06-01 at 12:20 +0200, Alexey ORISHKO wrote:
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org [mailto:netdev-
> > owner@vger.kernel.org] On Behalf Of Stefan Metzmacher
> > Sent: Wednesday, June 01, 2011 12:09 PM
>
>
> > - .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
> > + .flags = FLAG_WWAN | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
>
> This patch will introduce incompatibility with already existing
> applications, which track usbX devices. As a result, end user
> application will stop working.
Applications should *never* track devices based solely on a device name
prefix. What do they do when the device gets renamed either by udev
rules or the user? It's simply broken. Device names are not stable API
and they can and do change at will. Applications that expect them to
have a stable prefix are simply broken.
Dan
^ permalink raw reply
* Re: [net-next,02/13] tg3: Cleanup transmit error path
From: Alex Williamson @ 2011-06-02 22:18 UTC (permalink / raw)
To: Matt Carlson; +Cc: davem, netdev
In-Reply-To: <1305843176-32358-3-git-send-email-mcarlson@broadcom.com>
On Thu, 2011-05-19 at 12:12 +0000, Matt Carlson wrote:
> This patch consolidates the skb cleanup code into a function named
> tg3_skb_error_unmap(). The modification addresses a long-standing bug
> where pci_unmap_single() was incorrectly being called instead of
> pci_unmap_page() in tigon3_dma_hwbug_workaround().
This patch doesn't behave well when an iommu (VT-d) is involved.
Booting an X58 chipset based system with intel_iommu=on with 3.0.0-rc1
results in the warning below. Looks like the driver is trying to unmap
0x0. Eventually something worse happens and the system reports a few
iommu faults from the device before panicing. Neither problem is
observed if 432aa7ed is reverted. tg3 device in use is a BCM5755.
Thanks,
Alex
WARNING: at drivers/pci/intel-iommu.c:2888 intel_unmap_page+0x15c/0x180()
Hardware name: 4157CTO
Driver unmaps unmatched page at PFN 0
Modules linked in: nfs lockd auth_rpcgss nfs_acl ipt_MASQUERADE iptable_nat nf_nat iptable_mangle tun autofs4 sunrpc cpufreq_ondemand acpi_cpufreq freq_table mperf bridge stp llc ipv6 dm_mirror dm_region_hash dm_log kvm_intel kvm sg microcode serio_raw wmi i2c_i801 i2c_core iTCO_wdt iTCO_vendor_support tg3 ext4 mbcache jbd2 raid1 raid456 async_pq async_xor xor async_memcpy async_raid6_recov raid6_pq async_tx sr_mod cdrom sd_mod crc_t10dif floppy dm_mod [last unloaded: scsi_wait_scan]
Pid: 2604, comm: libvirtd Not tainted 3.0.0-rc1+ #184
Call Trace:
[<ffffffff8105c7df>] warn_slowpath_common+0x7f/0xc0
[<ffffffff8105c8d6>] warn_slowpath_fmt+0x46/0x50
[<ffffffff81245b8b>] ? find_iova+0x5b/0xa0
[<ffffffff8124a60c>] intel_unmap_page+0x15c/0x180
[<ffffffffa0128d48>] tg3_skb_error_unmap+0xb8/0x130 [tg3]
[<ffffffffa0136041>] tg3_start_xmit+0x661/0xde0 [tg3]
[<ffffffffa01363dd>] ? tg3_start_xmit+0x9fd/0xde0 [tg3]
[<ffffffff8138d7fe>] dev_hard_start_xmit+0x31e/0x6c0
[<ffffffff813a9f5f>] sch_direct_xmit+0xef/0x1c0
[<ffffffff8138dd38>] dev_queue_xmit+0x198/0x5f0
[<ffffffffa028d2ac>] br_dev_queue_push_xmit+0x6c/0xa0 [bridge]
[<ffffffffa028d478>] br_forward_finish+0x58/0x60 [bridge]
[<ffffffffa028d4f8>] __br_deliver+0x78/0xf0 [bridge]
[<ffffffff814367a9>] ? packet_rcv+0x59/0x450
[<ffffffffa028d5a5>] br_deliver+0x35/0x40 [bridge]
[<ffffffffa028b8c4>] br_dev_xmit+0x114/0x180 [bridge]
[<ffffffff8138d7fe>] dev_hard_start_xmit+0x31e/0x6c0
[<ffffffff8138df95>] dev_queue_xmit+0x3f5/0x5f0
[<ffffffff813d0c2e>] ip_finish_output+0x16e/0x340
[<ffffffff813d1110>] ip_output+0xb0/0xc0
[<ffffffff813cfdc3>] ? __ip_local_out+0xa3/0xb0
[<ffffffff813d0129>] ip_local_out+0x29/0x30
[<ffffffff813d0604>] ip_queue_xmit+0x164/0x410
[<ffffffff813e5e5c>] tcp_transmit_skb+0x41c/0x910
[<ffffffff813e86f7>] tcp_write_xmit+0x1e7/0x9d0
[<ffffffff813e8f46>] __tcp_push_pending_frames+0x26/0xc0
[<ffffffff813d77be>] tcp_push+0x6e/0x90
[<ffffffff813db4f9>] tcp_sendmsg+0x759/0xc00
[<ffffffff813fdaa8>] inet_sendmsg+0x48/0xb0
[<ffffffff811cd7f3>] ? selinux_socket_sendmsg+0x23/0x30
[<ffffffff81377e52>] sock_sendmsg+0xe2/0x120
[<ffffffff8100a7e4>] ? __switch_to+0x194/0x320
[<ffffffff81377ed1>] kernel_sendmsg+0x41/0x60
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: allow all slave speeds
From: David Miller @ 2011-06-02 21:44 UTC (permalink / raw)
To: jpirko; +Cc: netdev, fubar, andy
In-Reply-To: <1306960593-16347-1-git-send-email-jpirko@redhat.com>
From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 1 Jun 2011 22:36:33 +0200
> No need to check for 10, 100, 1000, 10000 explicitly. Just make this
> generic and check for invalid values only (similar check is in ethtool
> userspace app). This enables correct speed handling for slave devices
> with "nonstandard" speeds.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [TRIVIAL PATCH next 15/15] net: Convert vmalloc/memset to vzalloc
From: David Miller @ 2011-06-02 21:39 UTC (permalink / raw)
To: pablo
Cc: joe, kaber, andy.grover, trivial, netfilter-devel, netfilter,
coreteam, netdev, linux-kernel, rds-devel
In-Reply-To: <4DE7A311.5000406@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 02 Jun 2011 16:49:53 +0200
> Are you going to take this patch? it includes one chunck which is out of
> netfilter scope.
I think the trivial folks submit these things via their own tree
and are only looking for ACKs from us.
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH v2] sungem: Spring cleaning and GRO support
From: David Miller @ 2011-06-02 21:14 UTC (permalink / raw)
To: benh; +Cc: netdev, ruediger.herbst, bhamilton04
In-Reply-To: <1306912630.29297.34.camel@pasglop>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Wed, 01 Jun 2011 17:17:10 +1000
> It still needs testing on sparc and maybe a bit more torturing on
> powerpc. I've done some cursory link up/down tests on a g5 and a
> powerbook g4 here, and some suspend/resume tests, so far with
> success, but I'll do more tomorrow just in case.
I'll give this a quick spin on one of my sparc64 boxes that has
a sungem in it. If all goes well I'll just apply this to net-next-2.6
Thanks!
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox