* Re: [PATCH 0/2][net-next] gianfar: xmit - fix errata workaround and cleanup
From: David Miller @ 2013-08-05 19:29 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev, paul.gortmaker
In-Reply-To: <1375712410-11760-1-git-send-email-claudiu.manoil@freescale.com>
From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Mon, 5 Aug 2013 17:20:08 +0300
> Please find these cleanup patches for the gfar_start_xmit() function,
> including a fix for an old errata workaround implementation (eTSEC76,
> for MPC8313 and MPC837x). I find this cleanup quite necessary to make
> future fixes and improvements in xmit (and Tx processing) easier.
Both applied, thanks.
^ permalink raw reply
* [PATCH net-next] bonding: fix send_peer_notif leekage on rtnl lock congestion
From: Veaceslav Falico @ 2013-08-05 19:47 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In bond_mii_monitor()/bond_activebackup_arp_mon(), we verify if we have any
notifications to be sent before trying to get the rtnl_trylock(), and if
we have - we set should_notify_peers and withdraw a notification. However,
if we fail the get the rtnl_trylock(), we don't send out any notification
and don't put the notification back to bond->send_peer_notif.
Fix it by putting back the notification to send_peer_notif in case of
rtnl_trylock() failure.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5697043..fd92738 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2383,7 +2383,10 @@ void bond_mii_monitor(struct work_struct *work)
if (!rtnl_trylock()) {
read_lock(&bond->lock);
delay = 1;
- should_notify_peers = false;
+ if (should_notify_peers) {
+ bond->send_peer_notif++;
+ should_notify_peers = false;
+ }
goto re_arm;
}
@@ -2999,7 +3002,10 @@ void bond_activebackup_arp_mon(struct work_struct *work)
if (!rtnl_trylock()) {
read_lock(&bond->lock);
delta_in_ticks = 1;
- should_notify_peers = false;
+ if (should_notify_peers) {
+ bond->send_peer_notif++;
+ should_notify_peers = false;
+ }
goto re_arm;
}
--
1.8.1.4
^ permalink raw reply related
* low latency/busy poll feedback and bugs
From: Shawn Bohrer @ 2013-08-05 21:22 UTC (permalink / raw)
To: Amir Vadai; +Cc: eliezer.tamir, netdev
I did a little testing of the new low latency/busy poll sockets today
and found a few things that surprised me and at least one bug.
1) I'm testing with a Mellanox ConnectX-3 card. Currently polling
with mlx4_en is broken when GRO is enabled. In
mlx4_en_process_rx_cq() when GRO is enabled skb_mark_napi_id() is
never called. It appears like low latency sockets with GRO is
supposed to work because the following code checks that we are not
ll_polling:
/* This packet is eligible for GRO if it is:
* - DIX Ethernet (type interpretation)
* - TCP/IP (v4)
* - without IP options
* - not an IP fragment
* - no LLS polling in progress
*/
if (!mlx4_en_cq_ll_polling(cq) &&
(dev->features & NETIF_F_GRO)) {
However since we never call skb_mark_napi_id() mlx4_en_cq_ll_polling()
will never be true.
2) Why is LowLatencyRxPackets reported as a TcpExt stat? Perhaps I've
been confused and misguided but I've always assumed those are
statistics related to TCP and this feature is protocol neutral. I'm
not entirely sure where it should be moved to perhaps IpExt?
3) I don't know if this was intentional, an oversight, or simply a
missing feature but UDP multicast currently is not supported. In
order to add support I believe you would need to call
sk_mark_napi_id() in __udp4_lib_mcast_deliver(). Assuming there isn't
some intentional reason this wasn't done I'd be happy to test this and
send a patch.
--
Shawn
--
---------------------------------------------------------------
This email, along with any attachments, is confidential. If you
believe you received this message in error, please contact the
sender immediately and delete all copies of the message.
Thank you.
^ permalink raw reply
* [PATCH net v4 2/6] bnx2x: protect different statistics flows
From: Dmitry Kravkov @ 2013-08-05 21:35 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, Dmitry Kravkov, Ariel Elior
In-Reply-To: <1375738544-8695-1-git-send-email-dmitry@broadcom.com>
Add locking to protect different statistics flows from
running simultaneously.
This in order to serialize statistics requests sent to FW,
otherwise two outstanding queries may cause FW assert.
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 3 +
drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 88 +++++++++++++++++++----
2 files changed, 79 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index d80e34b..c7ffff3 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -1830,6 +1830,9 @@ struct bnx2x {
int fp_array_size;
u32 dump_preset_idx;
+ bool stats_started;
+ unsigned long stats_flags;
+#define BNX2X_STATS_UPDATE_BIT 0
};
/* Tx queues may be less or equal to Rx queues */
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
index a22ad61..bf7f9a7 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
@@ -221,7 +221,8 @@ static int bnx2x_stats_comp(struct bnx2x *bp)
* Statistics service functions
*/
-static void bnx2x_stats_pmf_update(struct bnx2x *bp)
+/* should be called under bnx2x_lock_stat_transition */
+static void __bnx2x_stats_pmf_update(struct bnx2x *bp)
{
struct dmae_command *dmae;
u32 opcode;
@@ -518,7 +519,8 @@ static void bnx2x_func_stats_init(struct bnx2x *bp)
*stats_comp = 0;
}
-static void bnx2x_stats_start(struct bnx2x *bp)
+/* should be called under bnx2x_lock_stat_transition */
+static void __bnx2x_stats_start(struct bnx2x *bp)
{
/* vfs travel through here as part of the statistics FSM, but no action
* is required
@@ -534,13 +536,58 @@ static void bnx2x_stats_start(struct bnx2x *bp)
bnx2x_hw_stats_post(bp);
bnx2x_storm_stats_post(bp);
+
+ bp->stats_started = true;
+}
+
+static bool bnx2x_trylock_stat_transition(struct bnx2x *bp)
+{
+ return !test_and_set_bit(BNX2X_STATS_UPDATE_BIT, &bp->stats_flags);
+}
+
+static void bnx2x_lock_stat_transition(struct bnx2x *bp)
+{
+ int cnt = 10; /* 10-20mSec */
+
+ while (test_and_set_bit(BNX2X_STATS_UPDATE_BIT, &bp->stats_flags)) {
+ if (!cnt) {
+ BNX2X_ERR("timeout waiting for stats lock\n");
+ bnx2x_panic();
+ break;
+ }
+ cnt--;
+ usleep_range(1000, 2000);
+ }
+}
+
+static void bnx2x_unlock_stat_transition(struct bnx2x *bp)
+{
+ smp_mb__before_clear_bit();
+ clear_bit(BNX2X_STATS_UPDATE_BIT, &bp->stats_flags);
+ smp_mb__after_clear_bit();
+}
+
+static void bnx2x_stats_start(struct bnx2x *bp)
+{
+ bnx2x_lock_stat_transition(bp);
+ __bnx2x_stats_start(bp);
+ bnx2x_unlock_stat_transition(bp);
}
static void bnx2x_stats_pmf_start(struct bnx2x *bp)
{
+ bnx2x_lock_stat_transition(bp);
bnx2x_stats_comp(bp);
- bnx2x_stats_pmf_update(bp);
- bnx2x_stats_start(bp);
+ __bnx2x_stats_pmf_update(bp);
+ __bnx2x_stats_start(bp);
+ bnx2x_unlock_stat_transition(bp);
+}
+
+static void bnx2x_stats_pmf_update(struct bnx2x *bp)
+{
+ bnx2x_lock_stat_transition(bp);
+ __bnx2x_stats_pmf_update(bp);
+ bnx2x_unlock_stat_transition(bp);
}
static void bnx2x_stats_restart(struct bnx2x *bp)
@@ -550,8 +597,10 @@ static void bnx2x_stats_restart(struct bnx2x *bp)
*/
if (IS_VF(bp))
return;
+ bnx2x_lock_stat_transition(bp);
bnx2x_stats_comp(bp);
- bnx2x_stats_start(bp);
+ __bnx2x_stats_start(bp);
+ bnx2x_unlock_stat_transition(bp);
}
static void bnx2x_bmac_stats_update(struct bnx2x *bp)
@@ -888,9 +937,7 @@ static int bnx2x_storm_stats_validate_counters(struct bnx2x *bp)
/* Make sure we use the value of the counter
* used for sending the last stats ramrod.
*/
- spin_lock_bh(&bp->stats_lock);
cur_stats_counter = bp->stats_counter - 1;
- spin_unlock_bh(&bp->stats_lock);
/* are storm stats valid? */
if (le16_to_cpu(counters->xstats_counter) != cur_stats_counter) {
@@ -1227,12 +1274,18 @@ static void bnx2x_stats_update(struct bnx2x *bp)
{
u32 *stats_comp = bnx2x_sp(bp, stats_comp);
- if (bnx2x_edebug_stats_stopped(bp))
+ /* we run update from timer context, so give up
+ * if somebody in the middle of transition
+ */
+ if (!bnx2x_trylock_stat_transition(bp))
return;
+ if (bnx2x_edebug_stats_stopped(bp) || !bp->stats_started)
+ goto out;
+
if (IS_PF(bp)) {
if (*stats_comp != DMAE_COMP_VAL)
- return;
+ goto out;
if (bp->port.pmf)
bnx2x_hw_stats_update(bp);
@@ -1242,7 +1295,7 @@ static void bnx2x_stats_update(struct bnx2x *bp)
BNX2X_ERR("storm stats were not updated for 3 times\n");
bnx2x_panic();
}
- return;
+ goto out;
}
} else {
/* vf doesn't collect HW statistics, and doesn't get completions
@@ -1256,7 +1309,7 @@ static void bnx2x_stats_update(struct bnx2x *bp)
/* vf is done */
if (IS_VF(bp))
- return;
+ goto out;
if (netif_msg_timer(bp)) {
struct bnx2x_eth_stats *estats = &bp->eth_stats;
@@ -1267,6 +1320,9 @@ static void bnx2x_stats_update(struct bnx2x *bp)
bnx2x_hw_stats_post(bp);
bnx2x_storm_stats_post(bp);
+
+out:
+ bnx2x_unlock_stat_transition(bp);
}
static void bnx2x_port_stats_stop(struct bnx2x *bp)
@@ -1332,6 +1388,10 @@ static void bnx2x_stats_stop(struct bnx2x *bp)
{
int update = 0;
+ bnx2x_lock_stat_transition(bp);
+
+ bp->stats_started = false;
+
bnx2x_stats_comp(bp);
if (bp->port.pmf)
@@ -1348,6 +1408,8 @@ static void bnx2x_stats_stop(struct bnx2x *bp)
bnx2x_hw_stats_post(bp);
bnx2x_stats_comp(bp);
}
+
+ bnx2x_unlock_stat_transition(bp);
}
static void bnx2x_stats_do_nothing(struct bnx2x *bp)
@@ -1376,15 +1438,17 @@ static const struct {
void bnx2x_stats_handle(struct bnx2x *bp, enum bnx2x_stats_event event)
{
enum bnx2x_stats_state state;
+ void (*action)(struct bnx2x *bp);
if (unlikely(bp->panic))
return;
spin_lock_bh(&bp->stats_lock);
state = bp->stats_state;
bp->stats_state = bnx2x_stats_stm[state][event].next_state;
+ action = bnx2x_stats_stm[state][event].action;
spin_unlock_bh(&bp->stats_lock);
- bnx2x_stats_stm[state][event].action(bp);
+ action(bp);
if ((event != STATS_EVENT_UPDATE) || netif_msg_timer(bp))
DP(BNX2X_MSG_STATS, "state %d -> event %d -> state %d\n",
--
1.8.1.4
^ permalink raw reply related
* [PATCH net v4 1/6] bnx2x: properly initialize statistic counters
From: Dmitry Kravkov @ 2013-08-05 21:35 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, Dmitry Kravkov, Ariel Elior
In-Reply-To: <1375738544-8695-1-git-send-email-dmitry@broadcom.com>
This prevent second statistics query be sent before first one is complete.
This is required since two outstanding queries may cause FW assert.
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
index 98366ab..a22ad61 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c
@@ -1589,12 +1589,17 @@ void bnx2x_memset_stats(struct bnx2x *bp)
void bnx2x_stats_init(struct bnx2x *bp)
{
- int /*abs*/port = BP_PORT(bp);
+ struct stats_counter *counters = &bp->fw_stats_data->storm_counters;
int mb_idx = BP_FW_MB_IDX(bp);
+ int port = BP_PORT(bp);
bp->stats_pending = 0;
bp->executer_idx = 0;
bp->stats_counter = 0;
+ counters->xstats_counter = cpu_to_le16(0xFFFF);
+ counters->tstats_counter = cpu_to_le16(0xFFFF);
+ counters->ustats_counter = cpu_to_le16(0xFFFF);
+ counters->cstats_counter = cpu_to_le16(0xFFFF);
/* port and func stats for management */
if (!BP_NOMCP(bp)) {
--
1.8.1.4
^ permalink raw reply related
* [PATCH net v4 0/6] bnx2x: fixes
From: Dmitry Kravkov @ 2013-08-05 21:35 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, Dmitry Kravkov
Hello Dave
Please consider applying the series of bnx2x fixes to net:
* statistics may cause FW assert
* missing fairness configuration in DCB flow
* memory leak in sriov related part
* Illegal PTE access
* Pagefault crash in shutdown flow with cnic
v1->v2
* fixed sparse error pointed by Joe Perches
* added missing signed-off from Sergei Shtylyov
v2->v3
* added missing signed-off from Sergei Shtylyov
* fixed formatting from Sergei Shtylyov
v3->v4
* patch 1/6: fixed declaration order
* patch 2/6 replaced with: protect flows using set_bit constraints
Thanks
Dmitry
^ permalink raw reply
* [PATCH net v4 6/6] bnx2x: prevent crash in shutdown flow with CNIC
From: Dmitry Kravkov @ 2013-08-05 21:35 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, Yuval Mintz, Dmitry Kravkov, Ariel Elior
In-Reply-To: <1375738544-8695-1-git-send-email-dmitry@broadcom.com>
From: Yuval Mintz <yuvalmin@broadcom.com>
There might be a crash as during shutdown flow CNIC might try
to access resources already freed by bnx2x.
Change bnx2x_close() into dev_close() in __bnx2x_remove (shutdown flow)
to guarantee CNIC is notified of the device's change of status.
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 5c22081..abafb30 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -12837,8 +12837,7 @@ static void __bnx2x_remove(struct pci_dev *pdev,
unregister_netdev(dev);
} else {
rtnl_lock();
- if (netif_running(dev))
- bnx2x_close(dev);
+ dev_close(dev);
rtnl_unlock();
}
--
1.8.1.4
^ permalink raw reply related
* [PATCH net v4 4/6] bnx2x: fix memory leak in VF
From: Dmitry Kravkov @ 2013-08-05 21:35 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, Ariel Elior, Dmitry Kravkov
In-Reply-To: <1375738544-8695-1-git-send-email-dmitry@broadcom.com>
From: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
index 95861ef..44104fb 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c
@@ -3463,7 +3463,7 @@ int bnx2x_vf_pci_alloc(struct bnx2x *bp)
alloc_mem_err:
BNX2X_PCI_FREE(bp->vf2pf_mbox, bp->vf2pf_mbox_mapping,
sizeof(struct bnx2x_vf_mbx_msg));
- BNX2X_PCI_FREE(bp->vf2pf_mbox, bp->vf2pf_mbox_mapping,
+ BNX2X_PCI_FREE(bp->vf2pf_mbox, bp->pf2vf_bulletin_mapping,
sizeof(union pf_vf_bulletin));
return -ENOMEM;
}
--
1.8.1.4
^ permalink raw reply related
* [PATCH net v4 3/6] bnx2x: update fairness parameters following DCB negotiation
From: Dmitry Kravkov @ 2013-08-05 21:35 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, Dmitry Kravkov, Ariel Elior
In-Reply-To: <1375738544-8695-1-git-send-email-dmitry@broadcom.com>
ETS can be enabled as a result of DCB negotiation, then
fairness must be recalculated after each negotiation.
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 2 ++
drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c | 4 ++++
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 30 ++++++++++++++----------
3 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index c7ffff3..062480a 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -2454,4 +2454,6 @@ enum bnx2x_pci_bus_speed {
BNX2X_PCI_LINK_SPEED_5000 = 5000,
BNX2X_PCI_LINK_SPEED_8000 = 8000
};
+
+void bnx2x_set_local_cmng(struct bnx2x *bp);
#endif /* bnx2x.h */
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
index 0c94df4..f9122f2 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c
@@ -753,6 +753,10 @@ void bnx2x_dcbx_set_params(struct bnx2x *bp, u32 state)
bnx2x_pfc_set_pfc(bp);
bnx2x_dcbx_update_ets_params(bp);
+
+ /* ets may affect cmng configuration: reinit it in hw */
+ bnx2x_set_local_cmng(bp);
+
bnx2x_dcbx_resume_hw_tx(bp);
return;
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index e06186c..90a84af 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -2476,7 +2476,7 @@ static void bnx2x_cmng_fns_init(struct bnx2x *bp, u8 read_cfg, u8 cmng_type)
input.port_rate = bp->link_vars.line_speed;
- if (cmng_type == CMNG_FNS_MINMAX) {
+ if (cmng_type == CMNG_FNS_MINMAX && input.port_rate) {
int vn;
/* read mf conf from shmem */
@@ -2533,6 +2533,21 @@ static void storm_memset_cmng(struct bnx2x *bp,
}
}
+/* init cmng mode in HW according to local configuration */
+void bnx2x_set_local_cmng(struct bnx2x *bp)
+{
+ int cmng_fns = bnx2x_get_cmng_fns_mode(bp);
+
+ if (cmng_fns != CMNG_FNS_NONE) {
+ bnx2x_cmng_fns_init(bp, false, cmng_fns);
+ storm_memset_cmng(bp, &bp->cmng, BP_PORT(bp));
+ } else {
+ /* rate shaping and fairness are disabled */
+ DP(NETIF_MSG_IFUP,
+ "single function mode without fairness\n");
+ }
+}
+
/* This function is called upon link interrupt */
static void bnx2x_link_attn(struct bnx2x *bp)
{
@@ -2568,17 +2583,8 @@ static void bnx2x_link_attn(struct bnx2x *bp)
bnx2x_stats_handle(bp, STATS_EVENT_LINK_UP);
}
- if (bp->link_vars.link_up && bp->link_vars.line_speed) {
- int cmng_fns = bnx2x_get_cmng_fns_mode(bp);
-
- if (cmng_fns != CMNG_FNS_NONE) {
- bnx2x_cmng_fns_init(bp, false, cmng_fns);
- storm_memset_cmng(bp, &bp->cmng, BP_PORT(bp));
- } else
- /* rate shaping and fairness are disabled */
- DP(NETIF_MSG_IFUP,
- "single function mode without fairness\n");
- }
+ if (bp->link_vars.link_up && bp->link_vars.line_speed)
+ bnx2x_set_local_cmng(bp);
__bnx2x_link_report(bp);
--
1.8.1.4
^ permalink raw reply related
* [PATCH net v4 5/6] bnx2x: fix PTE write access error
From: Dmitry Kravkov @ 2013-08-05 21:35 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, Barak Witkowsky, Dmitry Kravkov, Ariel Elior
In-Reply-To: <1375738544-8695-1-git-send-email-dmitry@broadcom.com>
From: Barak Witkowsky <barak@broadcom.com>
PTE write access error might occur in MF_ALLOWED mode when IOMMU
is active. The patch adds rmmod HSI indicating to MFW to stop
running queries which might trigger this failure.
Signed-off-by: Barak Witkowsky <barak@broadcom.com>
Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 1 +
drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h | 5 +++++
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 9 +++++++++
3 files changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
index 062480a..b82793e 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
@@ -1502,6 +1502,7 @@ struct bnx2x {
#define BC_SUPPORTS_DCBX_MSG_NON_PMF (1 << 21)
#define IS_VF_FLAG (1 << 22)
#define INTERRUPTS_ENABLED_FLAG (1 << 23)
+#define BC_SUPPORTS_RMMOD_CMD (1 << 24)
#define BP_NOMCP(bp) ((bp)->flags & NO_MCP_FLAG)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
index 5018e52..32767f6 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h
@@ -1300,6 +1300,9 @@ struct drv_func_mb {
#define DRV_MSG_CODE_EEE_RESULTS_ACK 0xda000000
+ #define DRV_MSG_CODE_RMMOD 0xdb000000
+ #define REQ_BC_VER_4_RMMOD_CMD 0x0007080f
+
#define DRV_MSG_CODE_SET_MF_BW 0xe0000000
#define REQ_BC_VER_4_SET_MF_BW 0x00060202
#define DRV_MSG_CODE_SET_MF_BW_ACK 0xe1000000
@@ -1372,6 +1375,8 @@ struct drv_func_mb {
#define FW_MSG_CODE_EEE_RESULS_ACK 0xda100000
+ #define FW_MSG_CODE_RMMOD_ACK 0xdb100000
+
#define FW_MSG_CODE_SET_MF_BW_SENT 0xe0000000
#define FW_MSG_CODE_SET_MF_BW_DONE 0xe1000000
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 90a84af..5c22081 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -10368,6 +10368,10 @@ static void bnx2x_get_common_hwinfo(struct bnx2x *bp)
bp->flags |= (val >= REQ_BC_VER_4_DCBX_ADMIN_MSG_NON_PMF) ?
BC_SUPPORTS_DCBX_MSG_NON_PMF : 0;
+
+ bp->flags |= (val >= REQ_BC_VER_4_RMMOD_CMD) ?
+ BC_SUPPORTS_RMMOD_CMD : 0;
+
boot_mode = SHMEM_RD(bp,
dev_info.port_feature_config[BP_PORT(bp)].mba_config) &
PORT_FEATURE_MBA_BOOT_AGENT_TYPE_MASK;
@@ -12823,6 +12827,11 @@ static void __bnx2x_remove(struct pci_dev *pdev,
bnx2x_dcbnl_update_applist(bp, true);
#endif
+ if (IS_PF(bp) &&
+ !BP_NOMCP(bp) &&
+ (bp->flags & BC_SUPPORTS_RMMOD_CMD))
+ bnx2x_fw_command(bp, DRV_MSG_CODE_RMMOD, 0);
+
/* Close the interface - either directly or implicitly */
if (remove_netdev) {
unregister_netdev(dev);
--
1.8.1.4
^ permalink raw reply related
* Re: [net-next,1/3] bonding: fix vlan 0 addition and removal
From: Veaceslav Falico @ 2013-08-05 21:51 UTC (permalink / raw)
To: nikolay; +Cc: netdev, fubar, andy, davem, kaber
In-Reply-To: <1375709304-16778-2-git-send-email-nikolay@redhat.com>
On Mon, Aug 05, 2013 at 03:28:22PM +0200, nikolay@redhat.com wrote:
...snip...
>This is fixed by forbidding the addition/removal of vlan 0 through the
>bond's ndo_vlan_rx_add/kill_vid functions, and adding/removing it only when
>vlan 0 is in fact being created (or destroyed) on top of a bond interface
>in the bond's netdev handling function.
Isn't that a bit too intrusive/hacky? I don't think we should treat vlan id
0 somehow differently in terms of adding/removing, though I might be
wrong...
Maybe we should just fix the bond_vlan_used() function? Something like
this (I've done only basic testing, can do more thorough if needed), though
it's also not a really clean fix:
>From 1c89abefebe90568ed52d2df59fcfdd650bc4696 Mon Sep 17 00:00:00 2001
From: Veaceslav Falico <vfalico@redhat.com>
Date: Mon, 5 Aug 2013 23:29:12 +0200
Subject: [PATCH] bonding: add vlan_uses_dev_rcu() and make bond_vlan_used() use it
Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
id 0, and always returns true if 8021q is loaded. This creates several bad
situations - some warnings in __bond_release_one() because it thinks that
we still have vlans while removing, sending LB packets with vlan id 0 and,
possibly, other caused by vlan id 0.
Fix it by adding a new call, vlan_uses_dev_rcu(), which is the same as
vlan_uses_dev(), but uses rcu_dereference() instead of rtnl, and thus we
can use it in bond_vlan_used() wrapped in rcu_read_lock().
Also, use the pure vlan_uses_dev() in __bond_release_one() cause the rtnl
lock is held there.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Patrick McHardy <kaber@trash.net>
CC: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_alb.c | 1 -
drivers/net/bonding/bond_main.c | 3 +--
drivers/net/bonding/bonding.h | 10 +++++++++-
include/linux/if_vlan.h | 6 ++++++
net/8021q/vlan_core.c | 11 +++++++++++
5 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 3a5db7b..2684329 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -34,7 +34,6 @@
#include <linux/if_arp.h>
#include <linux/if_ether.h>
#include <linux/if_bonding.h>
-#include <linux/if_vlan.h>
#include <linux/in.h>
#include <net/ipx.h>
#include <net/arp.h>
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5697043..b8c36ac 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -69,7 +69,6 @@
#include <net/arp.h>
#include <linux/mii.h>
#include <linux/ethtool.h>
-#include <linux/if_vlan.h>
#include <linux/if_bonding.h>
#include <linux/jiffies.h>
#include <linux/preempt.h>
@@ -1976,7 +1975,7 @@ static int __bond_release_one(struct net_device *bond_dev,
bond_set_carrier(bond);
eth_hw_addr_random(bond_dev);
- if (bond_vlan_used(bond)) {
+ if (vlan_uses_dev(bond_dev)) {
pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n",
bond_dev->name, bond_dev->name);
pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n",
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 4bf52d5..cb49313 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -23,6 +23,7 @@
#include <linux/netpoll.h>
#include <linux/inetdevice.h>
#include <linux/etherdevice.h>
+#include <linux/if_vlan.h>
#include "bond_3ad.h"
#include "bond_alb.h"
@@ -267,9 +268,16 @@ struct bonding {
#endif /* CONFIG_DEBUG_FS */
};
+/* use vlan_uses_dev() if under rtnl */
static inline bool bond_vlan_used(struct bonding *bond)
{
- return !list_empty(&bond->vlan_list);
+ bool ret;
+
+ rcu_read_lock();
+ ret = vlan_uses_dev_rcu(bond->dev);
+ rcu_read_unlock();
+
+ return ret;
}
#define bond_slave_get_rcu(dev) \
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 715c343..1fcea36 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -101,6 +101,7 @@ extern void vlan_vids_del_by_dev(struct net_device *dev,
const struct net_device *by_dev);
extern bool vlan_uses_dev(const struct net_device *dev);
+extern bool vlan_uses_dev_rcu(const struct net_device *dev);
#else
static inline struct net_device *
__vlan_find_dev_deep(struct net_device *real_dev,
@@ -155,6 +156,11 @@ static inline bool vlan_uses_dev(const struct net_device *dev)
{
return false;
}
+
+static inline bool vlan_uses_dev_rcu(const struct net_device *dev)
+{
+ return false;
+}
#endif
static inline bool vlan_hw_offload_capable(netdev_features_t features,
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 4a78c4d..52e3fb3 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -399,3 +399,14 @@ bool vlan_uses_dev(const struct net_device *dev)
return vlan_info->grp.nr_vlan_devs ? true : false;
}
EXPORT_SYMBOL(vlan_uses_dev);
+
+bool vlan_uses_dev_rcu(const struct net_device *dev)
+{
+ struct vlan_info *vlan_info;
+
+ vlan_info = rcu_dereference(dev->vlan_info);
+ if (!vlan_info)
+ return false;
+ return vlan_info->grp.nr_vlan_devs ? true : false;
+}
+EXPORT_SYMBOL(vlan_uses_dev_rcu);
--
1.8.1.4
^ permalink raw reply related
* Re: [net-next, 2/3] bonding: change the bond's vlan syncing functions with the standard ones
From: Veaceslav Falico @ 2013-08-05 21:56 UTC (permalink / raw)
To: nikolay; +Cc: netdev, fubar, andy, davem
In-Reply-To: <1375709304-16778-3-git-send-email-nikolay@redhat.com>
On Mon, Aug 05, 2013 at 03:28:23PM +0200, nikolay@redhat.com wrote:
>From: Nikolay Aleksandrov <Nikolay Aleksandrov nikolay@redhat.com>
>
>Now we have vlan_vids_add/del_by_dev() which serve the same purpose as
>bond's bond_add/del_vlans_on_slave() with the good side effect of
>reverting the changes if one of the additions fails.
>
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>
>---
>drivers/net/bonding/bond_main.c | 35 +++++------------------------------
> 1 file changed, 5 insertions(+), 30 deletions(-)
-25, awesome, great one :)
Acked-by: Veaceslav Falico <vfalico@redhat.com>
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 5df8bcd..ed1d261 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -521,33 +521,6 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
> return __bond_vlan_rx_kill_vid(bond_dev, proto, vid);
> }
>
>-static void bond_add_vlans_on_slave(struct bonding *bond, struct net_device *slave_dev)
>-{
>- struct vlan_entry *vlan;
>- int res;
>-
>- list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
>- res = vlan_vid_add(slave_dev, htons(ETH_P_8021Q),
>- vlan->vlan_id);
>- if (res)
>- pr_warning("%s: Failed to add vlan id %d to device %s\n",
>- bond->dev->name, vlan->vlan_id,
>- slave_dev->name);
>- }
>-}
>-
>-static void bond_del_vlans_from_slave(struct bonding *bond,
>- struct net_device *slave_dev)
>-{
>- struct vlan_entry *vlan;
>-
>- list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
>- if (!vlan->vlan_id)
>- continue;
>- vlan_vid_del(slave_dev, htons(ETH_P_8021Q), vlan->vlan_id);
>- }
>-}
>-
> /*------------------------------- Link status -------------------------------*/
>
> /*
>@@ -1656,7 +1629,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> dev_mc_add(slave_dev, lacpdu_multicast);
> }
>
>- bond_add_vlans_on_slave(bond, slave_dev);
>+ if (vlan_vids_add_by_dev(slave_dev, bond_dev))
>+ pr_warn("%s: couldn't add bond vlan ids to %s",
>+ bond_dev->name, slave_dev->name);
>
> write_lock_bh(&bond->lock);
>
>@@ -1832,7 +1807,7 @@ err_detach:
> if (!USES_PRIMARY(bond->params.mode))
> bond_hw_addr_flush(bond_dev, slave_dev);
>
>- bond_del_vlans_from_slave(bond, slave_dev);
>+ vlan_vids_del_by_dev(slave_dev, bond_dev);
> write_lock_bh(&bond->lock);
> bond_detach_slave(bond, new_slave);
> if (bond->primary_slave == new_slave)
>@@ -2028,7 +2003,7 @@ static int __bond_release_one(struct net_device *bond_dev,
> /* must do this from outside any spinlocks */
> bond_destroy_slave_symlinks(bond_dev, slave_dev);
>
>- bond_del_vlans_from_slave(bond, slave_dev);
>+ vlan_vids_del_by_dev(slave_dev, bond_dev);
>
> /* If the mode USES_PRIMARY, then this cases was handled above by
> * bond_change_active_slave(..., NULL)
^ permalink raw reply
* Re: [net-next,3/3] bonding: unwind on bond_add_vlan add failure
From: Veaceslav Falico @ 2013-08-05 21:59 UTC (permalink / raw)
To: nikolay; +Cc: netdev, fubar, andy, davem
In-Reply-To: <1375709304-16778-4-git-send-email-nikolay@redhat.com>
On Mon, Aug 05, 2013 at 03:28:24PM +0200, nikolay@redhat.com wrote:
>From: Nikolay Aleksandrov <Nikolay Aleksandrov nikolay@redhat.com>
>
>In case of bond_add_vlan() failure currently we'll have the vlan's
>refcnt bumped up in all slaves, but it will never go down because it
>failed to get added to the bond, so properly unwind the added vlan if
>bond_add_vlan fails.
>
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
In case patch 1/3 from series goes in... Otherwise I think it will be
needed to patch bond_vlan_rx_add_vid().
Acked-by: Veaceslav Falico <vfalico@redhat.com>
>
>---
>drivers/net/bonding/bond_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index ed1d261..0f9ca7e 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -456,13 +456,13 @@ static int __bond_vlan_rx_add_vid(struct net_device *bond_dev,
> if (res) {
> pr_err("%s: Error: Failed to add vlan id %d\n",
> bond_dev->name, vid);
>- return res;
>+ goto unwind;
> }
>
> return 0;
>
> unwind:
>- /* unwind from head to the slave that failed */
>+ /* unwind from the slave that failed */
> bond_for_each_slave_continue_reverse(bond, slave)
> vlan_vid_del(slave->dev, proto, vid);
>
^ permalink raw reply
* [PATCH] lto, wan/sbni: Make inline assembler symbols visible and assembler global
From: Andi Kleen @ 2013-08-05 22:18 UTC (permalink / raw)
To: netdev; +Cc: Andi Kleen
From: Andi Kleen <ak@linux.intel.com>
- Inline assembler defining C callable code has to be global
- The function has to be visible
Do this in wan/sbni
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
drivers/net/wan/sbni.c | 6 +++---
drivers/net/wan/sbni.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wan/sbni.c b/drivers/net/wan/sbni.c
index d43f4ef..b914ab3 100644
--- a/drivers/net/wan/sbni.c
+++ b/drivers/net/wan/sbni.c
@@ -160,7 +160,7 @@ static int scandone __initdata = 0;
static int num __initdata = 0;
static unsigned char rxl_tab[];
-static u32 crc32tab[];
+__visible u32 sbni_crc32tab[];
/* A list of all installed devices, for removing the driver module. */
static struct net_device *sbni_cards[ SBNI_MAX_NUM_CARDS ];
@@ -1563,7 +1563,7 @@ calc_crc32( u32 crc, u8 *p, u32 len )
"xorl %%ebx, %%ebx\n"
"movl %2, %%esi\n"
"movl %3, %%ecx\n"
- "movl $crc32tab, %%edi\n"
+ "movl $sbni_crc32tab, %%edi\n"
"shrl $2, %%ecx\n"
"jz 1f\n"
@@ -1645,7 +1645,7 @@ calc_crc32( u32 crc, u8 *p, u32 len )
#endif /* ASM_CRC */
-static u32 crc32tab[] __attribute__ ((aligned(8))) = {
+__visible u32 sbni_crc32tab[] __attribute__ ((aligned(8))) = {
0xD202EF8D, 0xA505DF1B, 0x3C0C8EA1, 0x4B0BBE37,
0xD56F2B94, 0xA2681B02, 0x3B614AB8, 0x4C667A2E,
0xDCD967BF, 0xABDE5729, 0x32D70693, 0x45D03605,
diff --git a/drivers/net/wan/sbni.h b/drivers/net/wan/sbni.h
index 8426451..7e6d980 100644
--- a/drivers/net/wan/sbni.h
+++ b/drivers/net/wan/sbni.h
@@ -132,7 +132,7 @@ struct sbni_flags {
/*
* CRC-32 stuff
*/
-#define CRC32(c,crc) (crc32tab[((size_t)(crc) ^ (c)) & 0xff] ^ (((crc) >> 8) & 0x00FFFFFF))
+#define CRC32(c,crc) (sbni_crc32tab[((size_t)(crc) ^ (c)) & 0xff] ^ (((crc) >> 8) & 0x00FFFFFF))
/* CRC generator 0xEDB88320 */
/* CRC remainder 0x2144DF1C */
/* CRC initial value 0x00000000 */
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next] net: Add low-latency/polling support for UDP multicast
From: Shawn Bohrer @ 2013-08-05 22:16 UTC (permalink / raw)
To: davem; +Cc: eliezer.tamir, netdev, Amir Vadai, tomk, Shawn Bohrer
In-Reply-To: <20130805212257.GB6904@sbohrermbp13-local.rgmadvisors.com>
Set the napi id for each socket in the multicast path to enable
low-latency/polling support.
Signed-off-by: Shawn Bohrer <sbohrer@rgmadvisors.com>
---
net/ipv4/udp.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 766e6ba..0d0da17 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1596,6 +1596,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
dif = skb->dev->ifindex;
sk = udp_v4_mcast_next(net, sk, uh->dest, daddr, uh->source, saddr, dif);
while (sk) {
+ sk_mark_napi_id(sk, skb);
stack[count++] = sk;
sk = udp_v4_mcast_next(net, sk_nulls_next(sk), uh->dest,
daddr, uh->source, saddr, dif);
--
1.7.7.6
--
---------------------------------------------------------------
This email, along with any attachments, is confidential. If you
believe you received this message in error, please contact the
sender immediately and delete all copies of the message.
Thank you.
^ permalink raw reply related
* Re: [PATCH net-next 0/2] fix bonding neighbour setup handling
From: David Miller @ 2013-08-05 22:25 UTC (permalink / raw)
To: nikolay; +Cc: vfalico, netdev, fubar, andy, ebiederm, joe
In-Reply-To: <51FFAD54.7090501@redhat.com>
From: Nikolay Aleksandrov <nikolay@redhat.com>
Date: Mon, 05 Aug 2013 15:49:08 +0200
> Since the cat is out of the bag about this bug, as Vaeceslav discovered it
> independently and wasn't aware that there's a CVE number pending because it
> poses a security threat since the dereferenced first_slave pointer is
> taken from the struct vlan_dev_priv's ingress_priority map array which is
> user-controllable and any memory address can be dereferenced in that way,
> and taking after that first_slave->dev->netdev_ops and calling a function
> from the ops is making it even easier. Of course for that to happen the
> user must have CAP_NET_ADMIN.
> I've tested these patches and they apply cleanly on -net as well, so please
> queue them for -net and stable.
This is why I absolutely detest closed work on bugs, and prefer
everything be discussed and implemented openly here on this list,
without exceptions, and regardless of perceived "severity" of the bug.
Applied to net and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] fib_trie: remove potential out of bound access
From: David Miller @ 2013-08-05 22:27 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, andreyknvl, dvyukov
In-Reply-To: <1375726729.4457.45.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 05 Aug 2013 11:18:49 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> AddressSanitizer [1] dynamic checker pointed a potential
> out of bound access in leaf_walk_rcu()
>
> We could allocate one more slot in tnode_new() to leave the prefetch()
> in-place but it looks not worth the pain.
>
> Bug added in commit 82cfbb008572b ("[IPV4] fib_trie: iterator recode")
>
> [1] :
> https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
I question the validity of the prefetch anyways, even without the
out-of-bounds concerns.
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply
* [PATCH] bridge: don't try to update timers in case of broken MLD queries
From: Linus Lüssing @ 2013-08-05 22:32 UTC (permalink / raw)
To: bridge
Cc: Paul Bolle, Herbert Xu, netdev, linux-kernel, Adam Baker,
Stephen Hemminger, Linus Lüssing, David S. Miller, Cong Wang
Currently we are reading an uninitialized value for the max_delay
variable when snooping an MLD query message of invalid length and would
update our timers with that.
Fixing this by simply ignoring such broken MLD queries (just like we do
for IGMP already).
This is a regression introduced by:
"bridge: disable snooping if there is no querier" (b00589af3b04)
Reported-by: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Linus Lüssing <linus.luessing@web.de>
---
net/bridge/br_multicast.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 61c5e81..08e576a 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1195,7 +1195,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
if (max_delay)
group = &mld->mld_mca;
- } else if (skb->len >= sizeof(*mld2q)) {
+ } else {
if (!pskb_may_pull(skb, sizeof(*mld2q))) {
err = -EINVAL;
goto out;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] lto, wan/sbni: Make inline assembler symbols visible and assembler global
From: David Miller @ 2013-08-05 22:33 UTC (permalink / raw)
To: andi; +Cc: netdev, ak
In-Reply-To: <1375741094-8166-1-git-send-email-andi@firstfloor.org>
From: Andi Kleen <andi@firstfloor.org>
Date: Mon, 5 Aug 2013 15:18:14 -0700
> From: Andi Kleen <ak@linux.intel.com>
>
> - Inline assembler defining C callable code has to be global
> - The function has to be visible
>
> Do this in wan/sbni
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
Please just make it use the standard lib/ CRC routines instead.
Thanks.
^ permalink raw reply
* Re: [PATCHv3] bridge: disable snooping if there is no querier
From: Linus Lüssing @ 2013-08-05 22:40 UTC (permalink / raw)
To: Paul Bolle
Cc: herbert, netdev, bridge, linux-kernel, linux, stephen,
linus.luessing, David Miller, amwang
In-Reply-To: <1375692066.25148.14.camel@x61.thuisdomein>
Thanks for the very quick reporting! Looks like my gcc was and still
isn't as supportive... You and the lists should have received a patch
to fix that.
Cheers, Linus
On Mon, Aug 05, 2013 at 10:41:06AM +0200, Paul Bolle wrote:
> On Wed, 2013-07-31 at 17:40 -0700, David Miller wrote:
> > > If there is no querier on a link then we won't get periodic reports and
> > > therefore won't be able to learn about multicast listeners behind ports,
> > > potentially leading to lost multicast packets, especially for multicast
> > > listeners that joined before the creation of the bridge.
> > >
> > > These lost multicast packets can appear since c5c23260594
> > > ("bridge: Add multicast_querier toggle and disable queries by default")
> > > in particular.
> > >
> > > With this patch we are flooding multicast packets if our querier is
> > > disabled and if we didn't detect any other querier.
> > >
> > > A grace period of the Maximum Response Delay of the querier is added to
> > > give multicast responses enough time to arrive and to be learned from
> > > before disabling the flooding behaviour again.
> > >
> > > Signed-off-by: Linus Lüssing <linus.luessing@web.de>
> >
> > Looks good, applied, thanks Linus.
>
> 0) This patch is part of v3.11-rc4 as commit b00589af3b0. It introduced
> a GCC warning:
> net/bridge/br_multicast.c: In function ‘br_multicast_rcv’:
> net/bridge/br_multicast.c:1081:36: warning: ‘max_delay’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> net/bridge/br_multicast.c:1178:16: note: ‘max_delay’ was declared here
>
> 1) Summarized, the code reads:
>
> unsigned long max_delay;
>
> if (skb->len == sizeof(*mld))
> max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
> else if (skb->len >= sizeof(*mld2q))
> max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
>
> br_multicast_query_received(br, port, !ipv6_addr_any(&ip6h->saddr),
> max_delay);
>
> So GCC notices that max_delay is still uninitialized if skb->len is
> neither equal to sizeof(*mld) nor equal or bigger than sizeof(*mld2q).
> To me it looks GCC is right here. At least, it is not obvious that
> max_delay will actually not be used in br_multicast_query_received() if
> it still is uninitialized.
>
> 2) I'm entirely unfamiliar to this code. So I can't say how this warning
> might be silenced.
>
>
> Paul Bolle
>
^ permalink raw reply
* Re: [PATCH] fib_trie: remove potential out of bound access
From: Stephen Hemminger @ 2013-08-05 22:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Andrey Konovalov, Dmitry Vyukov
In-Reply-To: <1375726729.4457.45.camel@edumazet-glaptop>
On Mon, 05 Aug 2013 11:18:49 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> AddressSanitizer [1] dynamic checker pointed a potential
> out of bound access in leaf_walk_rcu()
>
> We could allocate one more slot in tnode_new() to leave the prefetch()
> in-place but it looks not worth the pain.
>
> Bug added in commit 82cfbb008572b ("[IPV4] fib_trie: iterator recode")
>
> [1] :
> https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
Isn't prefetch supposed to always be safe, even out of bounds; even prefetch(NULL).
Although I really doubt prefetch helps in in this code anyway.
^ permalink raw reply
* Re: [PATCH] bridge: don't try to update timers in case of broken MLD queries
From: Stephen Hemminger @ 2013-08-05 22:42 UTC (permalink / raw)
To: Linus Lüssing
Cc: Paul Bolle, Cong Wang, netdev, bridge, linux-kernel, Adam Baker,
David S. Miller, Herbert Xu
In-Reply-To: <1375741925-22179-1-git-send-email-linus.luessing@web.de>
On Tue, 6 Aug 2013 00:32:05 +0200
Linus Lüssing <linus.luessing@web.de> wrote:
> Currently we are reading an uninitialized value for the max_delay
> variable when snooping an MLD query message of invalid length and would
> update our timers with that.
>
> Fixing this by simply ignoring such broken MLD queries (just like we do
> for IGMP already).
>
> This is a regression introduced by:
> "bridge: disable snooping if there is no querier" (b00589af3b04)
>
> Reported-by: Paul Bolle <pebolle@tiscali.nl>
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>
> ---
> net/bridge/br_multicast.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 61c5e81..08e576a 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1195,7 +1195,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
> max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
> if (max_delay)
> group = &mld->mld_mca;
> - } else if (skb->len >= sizeof(*mld2q)) {
> + } else {
> if (!pskb_may_pull(skb, sizeof(*mld2q))) {
> err = -EINVAL;
> goto out;
Why not use else if here, other than that looks great.
^ permalink raw reply
* Re: [PATCH] bridge: don't try to update timers in case of broken MLD queries
From: David Miller @ 2013-08-05 22:44 UTC (permalink / raw)
To: linus.luessing
Cc: pebolle, amwang, netdev, bridge, linux-kernel, linux, stephen,
herbert
In-Reply-To: <1375741925-22179-1-git-send-email-linus.luessing@web.de>
From: Linus Lüssing <linus.luessing@web.de>
Date: Tue, 6 Aug 2013 00:32:05 +0200
> Currently we are reading an uninitialized value for the max_delay
> variable when snooping an MLD query message of invalid length and would
> update our timers with that.
>
> Fixing this by simply ignoring such broken MLD queries (just like we do
> for IGMP already).
>
> This is a regression introduced by:
> "bridge: disable snooping if there is no querier" (b00589af3b04)
>
> Reported-by: Paul Bolle <pebolle@tiscali.nl>
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] fib_trie: remove potential out of bound access
From: Eric Dumazet @ 2013-08-05 23:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, netdev, Andrey Konovalov, Dmitry Vyukov
In-Reply-To: <20130805154111.119ac5ac@nehalam.linuxnetplumber.net>
On Mon, 2013-08-05 at 15:41 -0700, Stephen Hemminger wrote:
> On Mon, 05 Aug 2013 11:18:49 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > From: Eric Dumazet <edumazet@google.com>
> >
> > AddressSanitizer [1] dynamic checker pointed a potential
> > out of bound access in leaf_walk_rcu()
> >
> > We could allocate one more slot in tnode_new() to leave the prefetch()
> > in-place but it looks not worth the pain.
> >
> > Bug added in commit 82cfbb008572b ("[IPV4] fib_trie: iterator recode")
> >
> > [1] :
> > https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel
> >
> > Reported-by: Andrey Konovalov <andreyknvl@google.com>
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Dmitry Vyukov <dvyukov@google.com>
>
> Isn't prefetch supposed to always be safe, even out of bounds; even prefetch(NULL).
> Although I really doubt prefetch helps in in this code anyway.
prefetch(...) was not the problem here.
The problem was X = array[N] with N being >= size(array)
^ permalink raw reply
* Re: [PATCH] lto, wan/sbni: Make inline assembler symbols visible and assembler global
From: Andi Kleen @ 2013-08-05 23:05 UTC (permalink / raw)
To: David Miller; +Cc: andi, netdev, ak
In-Reply-To: <20130805.153323.276242626385124232.davem@davemloft.net>
On Mon, Aug 05, 2013 at 03:33:23PM -0700, David Miller wrote:
> From: Andi Kleen <andi@firstfloor.org>
> Date: Mon, 5 Aug 2013 15:18:14 -0700
>
> > From: Andi Kleen <ak@linux.intel.com>
> >
> > - Inline assembler defining C callable code has to be global
> > - The function has to be visible
> >
> > Do this in wan/sbni
> >
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
>
> Please just make it use the standard lib/ CRC routines instead.
The problem is I don't have this hardware, and I don't think
I can do such a change without testing it.
FWIW my main interest here is to avoid this thing breaking
my allyesconfig build.
Could you please consider adding it?
If not I'll have to disable the driver in the LTO patchkit.
-Andi
^ 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