Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net 2/3] qed: Fix overriding of supported autoneg value.
From: Leon Romanovsky @ 2017-05-04  7:20 UTC (permalink / raw)
  To: Sudarsana Reddy Kalluru; +Cc: davem, netdev, Yuval.Mintz
In-Reply-To: <20170504065209.26106-3-sudarsana.kalluru@cavium.com>

[-- Attachment #1: Type: text/plain, Size: 2912 bytes --]

On Wed, May 03, 2017 at 11:52:08PM -0700, Sudarsana Reddy Kalluru wrote:
> Driver currently uses advertised-autoneg value to populate the
> supported-autoneg field. When advertised field is updated, user gets
> the same value for supported field. Supported-autoneg value need to be
> populated from the link capabilities value returned by the MFW.
>
> Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
> ---
>  drivers/net/ethernet/qlogic/qed/qed_dev.c  | 3 +++
>  drivers/net/ethernet/qlogic/qed/qed_main.c | 6 +++++-
>  drivers/net/ethernet/qlogic/qed/qed_mcp.h  | 1 +
>  3 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
> index 5c6874a..bb70522 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
> @@ -2536,6 +2536,9 @@ static int qed_hw_get_nvm_info(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
>  		DP_NOTICE(p_hwfn, "Unknown Speed in 0x%08x\n", link_temp);
>  	}
>
> +	p_hwfn->mcp_info->link_capabilities.default_speed_autoneg =
> +		link->speed.autoneg;
> +
>  	link_temp &= NVM_CFG1_PORT_DRV_FLOW_CONTROL_MASK;
>  	link_temp >>= NVM_CFG1_PORT_DRV_FLOW_CONTROL_OFFSET;
>  	link->pause.autoneg = !!(link_temp &
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
> index a5eef1a..b7ad36b 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_main.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
> @@ -1372,7 +1372,7 @@ static void qed_fill_link(struct qed_hwfn *hwfn,
>
>  	/* TODO - at the moment assume supported and advertised speed equal */
>  	if_link->supported_caps = QED_LM_FIBRE_BIT;
> -	if (params.speed.autoneg)
> +	if (link_caps.default_speed_autoneg)
>  		if_link->supported_caps |= QED_LM_Autoneg_BIT;
>  	if (params.pause.autoneg ||
>  	    (params.pause.forced_rx && params.pause.forced_tx))
> @@ -1382,6 +1382,10 @@ static void qed_fill_link(struct qed_hwfn *hwfn,
>  		if_link->supported_caps |= QED_LM_Pause_BIT;
>
>  	if_link->advertised_caps = if_link->supported_caps;
> +	if (params.speed.autoneg)
> +		if_link->advertised_caps |= QED_LM_Autoneg_BIT;
> +	else
> +		if_link->advertised_caps &= ~QED_LM_Autoneg_BIT;
>  	if (params.speed.advertised_speeds &
>  	    NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_1G)
>  		if_link->advertised_caps |= QED_LM_1000baseT_Half_BIT |
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> index 5ae35d6..937496d 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> +++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> @@ -61,6 +61,7 @@ struct qed_mcp_link_params {
>
>  struct qed_mcp_link_capabilities {
>  	u32 speed_capabilities;
> +	bool default_speed_autoneg;	/* In Mb/s */

bool variable in Mb/s ?????

>  };
>
>  struct qed_mcp_link_state {
> --
> 1.8.3.1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH net 3/3] qede: Fix possible misconfiguration of advertised autoneg value.
From: Sudarsana Reddy Kalluru @ 2017-05-04  6:52 UTC (permalink / raw)
  To: davem; +Cc: netdev, Yuval.Mintz
In-Reply-To: <20170504065209.26106-1-sudarsana.kalluru@cavium.com>

Fail the configuration of advertised speed-autoneg value if the config
update is not supported.

Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
---
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index b22753c..172b292 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -493,6 +493,11 @@ static int qede_set_link_ksettings(struct net_device *dev,
 	params.override_flags |= QED_LINK_OVERRIDE_SPEED_ADV_SPEEDS;
 	params.override_flags |= QED_LINK_OVERRIDE_SPEED_AUTONEG;
 	if (base->autoneg == AUTONEG_ENABLE) {
+		if (!(current_link.supported_caps & QED_LM_Autoneg_BIT)) {
+			DP_INFO(edev, "Auto negotiation is not supported\n");
+			return -EOPNOTSUPP;
+		}
+
 		params.autoneg = true;
 		params.forced_speed = 0;
 		QEDE_ETHTOOL_TO_DRV_CAPS(params.adv_speeds, cmd, advertising)
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net 1/3] qed*: Fix possible overflow for status block id field.
From: Sudarsana Reddy Kalluru @ 2017-05-04  6:52 UTC (permalink / raw)
  To: davem; +Cc: netdev, Yuval.Mintz
In-Reply-To: <20170504065209.26106-1-sudarsana.kalluru@cavium.com>

Value for status block id could be more than 256 in 100G mode, need to
update its data type from u8 to u16.

Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_dev.c       | 8 ++++----
 drivers/net/ethernet/qlogic/qed/qed_dev_api.h   | 4 ++--
 drivers/net/ethernet/qlogic/qed/qed_main.c      | 2 +-
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 5 ++---
 include/linux/qed/qed_if.h                      | 2 +-
 5 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 5f31140..5c6874a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -3586,7 +3586,7 @@ static int qed_set_coalesce(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
 }
 
 int qed_set_rxq_coalesce(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
-			 u16 coalesce, u8 qid, u16 sb_id)
+			 u16 coalesce, u16 qid, u16 sb_id)
 {
 	struct ustorm_eth_queue_zone eth_qzone;
 	u8 timeset, timer_res;
@@ -3607,7 +3607,7 @@ int qed_set_rxq_coalesce(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
 	}
 	timeset = (u8)(coalesce >> timer_res);
 
-	rc = qed_fw_l2_queue(p_hwfn, (u16)qid, &fw_qid);
+	rc = qed_fw_l2_queue(p_hwfn, qid, &fw_qid);
 	if (rc)
 		return rc;
 
@@ -3628,7 +3628,7 @@ int qed_set_rxq_coalesce(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
 }
 
 int qed_set_txq_coalesce(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
-			 u16 coalesce, u8 qid, u16 sb_id)
+			 u16 coalesce, u16 qid, u16 sb_id)
 {
 	struct xstorm_eth_queue_zone eth_qzone;
 	u8 timeset, timer_res;
@@ -3649,7 +3649,7 @@ int qed_set_txq_coalesce(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
 	}
 	timeset = (u8)(coalesce >> timer_res);
 
-	rc = qed_fw_l2_queue(p_hwfn, (u16)qid, &fw_qid);
+	rc = qed_fw_l2_queue(p_hwfn, qid, &fw_qid);
 	if (rc)
 		return rc;
 
diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev_api.h b/drivers/net/ethernet/qlogic/qed/qed_dev_api.h
index cefe3ee..12d16c0 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev_api.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev_api.h
@@ -454,7 +454,7 @@ int qed_final_cleanup(struct qed_hwfn *p_hwfn,
  * @return int
  */
 int qed_set_rxq_coalesce(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
-			 u16 coalesce, u8 qid, u16 sb_id);
+			 u16 coalesce, u16 qid, u16 sb_id);
 
 /**
  * @brief qed_set_txq_coalesce - Configure coalesce parameters for a Tx queue
@@ -471,7 +471,7 @@ int qed_set_rxq_coalesce(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
  * @return int
  */
 int qed_set_txq_coalesce(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
-			 u16 coalesce, u8 qid, u16 sb_id);
+			 u16 coalesce, u16 qid, u16 sb_id);
 
 const char *qed_hw_get_resc_name(enum qed_resources res_id);
 #endif
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 59992cf..a5eef1a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1521,7 +1521,7 @@ static void qed_get_coalesce(struct qed_dev *cdev, u16 *rx_coal, u16 *tx_coal)
 }
 
 static int qed_set_coalesce(struct qed_dev *cdev, u16 rx_coal, u16 tx_coal,
-			    u8 qid, u16 sb_id)
+			    u16 qid, u16 sb_id)
 {
 	struct qed_hwfn *hwfn;
 	struct qed_ptt *ptt;
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 4dcfe96..b22753c 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -706,8 +706,7 @@ static int qede_set_coalesce(struct net_device *dev,
 {
 	struct qede_dev *edev = netdev_priv(dev);
 	int i, rc = 0;
-	u16 rxc, txc;
-	u8 sb_id;
+	u16 rxc, txc, sb_id;
 
 	if (!netif_running(dev)) {
 		DP_INFO(edev, "Interface is down\n");
@@ -729,7 +728,7 @@ static int qede_set_coalesce(struct net_device *dev,
 	for_each_queue(i) {
 		sb_id = edev->fp_array[i].sb_info->igu_sb_id;
 		rc = edev->ops->common->set_coalesce(edev->cdev, rxc, txc,
-						     (u8)i, sb_id);
+						     (u16)i, sb_id);
 		if (rc) {
 			DP_INFO(edev, "Set coalesce error, rc = %d\n", rc);
 			return rc;
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index 5544d7b..c70ac13 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -635,7 +635,7 @@ struct qed_common_ops {
  * @return 0 on success, error otherwise.
  */
 	int (*set_coalesce)(struct qed_dev *cdev, u16 rx_coal, u16 tx_coal,
-			    u8 qid, u16 sb_id);
+			    u16 qid, u16 sb_id);
 
 /**
  * @brief set_led - Configure LED mode
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net 2/3] qed: Fix overriding of supported autoneg value.
From: Sudarsana Reddy Kalluru @ 2017-05-04  6:52 UTC (permalink / raw)
  To: davem; +Cc: netdev, Yuval.Mintz
In-Reply-To: <20170504065209.26106-1-sudarsana.kalluru@cavium.com>

Driver currently uses advertised-autoneg value to populate the
supported-autoneg field. When advertised field is updated, user gets
the same value for supported field. Supported-autoneg value need to be
populated from the link capabilities value returned by the MFW.

Signed-off-by: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>
---
 drivers/net/ethernet/qlogic/qed/qed_dev.c  | 3 +++
 drivers/net/ethernet/qlogic/qed/qed_main.c | 6 +++++-
 drivers/net/ethernet/qlogic/qed/qed_mcp.h  | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 5c6874a..bb70522 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -2536,6 +2536,9 @@ static int qed_hw_get_nvm_info(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 		DP_NOTICE(p_hwfn, "Unknown Speed in 0x%08x\n", link_temp);
 	}
 
+	p_hwfn->mcp_info->link_capabilities.default_speed_autoneg =
+		link->speed.autoneg;
+
 	link_temp &= NVM_CFG1_PORT_DRV_FLOW_CONTROL_MASK;
 	link_temp >>= NVM_CFG1_PORT_DRV_FLOW_CONTROL_OFFSET;
 	link->pause.autoneg = !!(link_temp &
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index a5eef1a..b7ad36b 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1372,7 +1372,7 @@ static void qed_fill_link(struct qed_hwfn *hwfn,
 
 	/* TODO - at the moment assume supported and advertised speed equal */
 	if_link->supported_caps = QED_LM_FIBRE_BIT;
-	if (params.speed.autoneg)
+	if (link_caps.default_speed_autoneg)
 		if_link->supported_caps |= QED_LM_Autoneg_BIT;
 	if (params.pause.autoneg ||
 	    (params.pause.forced_rx && params.pause.forced_tx))
@@ -1382,6 +1382,10 @@ static void qed_fill_link(struct qed_hwfn *hwfn,
 		if_link->supported_caps |= QED_LM_Pause_BIT;
 
 	if_link->advertised_caps = if_link->supported_caps;
+	if (params.speed.autoneg)
+		if_link->advertised_caps |= QED_LM_Autoneg_BIT;
+	else
+		if_link->advertised_caps &= ~QED_LM_Autoneg_BIT;
 	if (params.speed.advertised_speeds &
 	    NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_1G)
 		if_link->advertised_caps |= QED_LM_1000baseT_Half_BIT |
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index 5ae35d6..937496d 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -61,6 +61,7 @@ struct qed_mcp_link_params {
 
 struct qed_mcp_link_capabilities {
 	u32 speed_capabilities;
+	bool default_speed_autoneg;	/* In Mb/s */
 };
 
 struct qed_mcp_link_state {
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net 0/3] qed*: Bug fix series.
From: Sudarsana Reddy Kalluru @ 2017-05-04  6:52 UTC (permalink / raw)
  To: davem; +Cc: netdev, Yuval.Mintz, Sudarsana Reddy Kalluru

From: Sudarsana Reddy Kalluru <Sudarsana.Kalluru@cavium.com>

The series contains minor bug fixes for qed/qede drivers.

Please consider applying it to 'net' branch.

Sudarsana Reddy Kalluru (3):
  qed*: Fix possible overflow for status block id field.
  qed: Fix overriding of supported autoneg value.
  qede: Fix possible misconfiguration of advertised autoneg value.

 drivers/net/ethernet/qlogic/qed/qed_dev.c       | 11 +++++++----
 drivers/net/ethernet/qlogic/qed/qed_dev_api.h   |  4 ++--
 drivers/net/ethernet/qlogic/qed/qed_main.c      |  8 ++++++--
 drivers/net/ethernet/qlogic/qed/qed_mcp.h       |  1 +
 drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 10 +++++++---
 include/linux/qed/qed_if.h                      |  2 +-
 6 files changed, 24 insertions(+), 12 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [PATCH] cfg80211: make RATE_INFO_BW_20 the default
From: Johannes Berg @ 2017-05-04  6:42 UTC (permalink / raw)
  To: linux-wireless; +Cc: Linus Torvalds, netdev, Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Due to the way I did the RX bitrate conversions in mac80211 with
spatch, going setting flags to setting the value, many drivers now
don't set the bandwidth value for 20 MHz, since with the flags it
wasn't necessary to (there was no 20 MHz flag, only the others.)

Rather than go through and try to fix up all the drivers, instead
renumber the enum so that 20 MHz, which is the typical bandwidth,
actually has the value 0, making those drivers all work again.

If VHT was hit used with a driver not reporting it, e.g. iwlmvm,
this manifested in hitting the bandwidth warning in
cfg80211_calculate_bitrate_vht().

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 6e90f1a4950f..15d6599b8bc6 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1013,9 +1013,9 @@ enum rate_info_flags {
  * @RATE_INFO_BW_160: 160 MHz bandwidth
  */
 enum rate_info_bw {
+	RATE_INFO_BW_20 = 0,
 	RATE_INFO_BW_5,
 	RATE_INFO_BW_10,
-	RATE_INFO_BW_20,
 	RATE_INFO_BW_40,
 	RATE_INFO_BW_80,
 	RATE_INFO_BW_160,
-- 
2.11.0

^ permalink raw reply related

* Re: new warning at net/wireless/util.c:1236
From: Johannes Berg @ 2017-05-04  6:41 UTC (permalink / raw)
  To: Linus Torvalds, David Miller
  Cc: Linux Wireless List, Network Development,
	Linux Kernel Mailing List
In-Reply-To: <CA+55aFxyp5gpUZEYHXLwyhbK6DMWf4RoVykGxq9FPtO=j4yJsw@mail.gmail.com>

Hi,

> Things still work, but when it starts warning, it generates a *lot*
> of noise (I got 36 of these within about ten minutes).

Yeah, that's kinda dumb - I just sent a patch to make that just warn
once and actually report the configuration.

> I have no idea what triggered it, because when I rebooted (not
> because of this issue, but just to reboot into a newer kernel) I
> don't see it again.

That's odd. Perhaps you connected to a different wireless network on
the next boot? I'm pretty sure I found the cause for this, and will
send out a patch in a minute, but that wouldn't go away with a
different kernel.

johannes

^ permalink raw reply

* Re: [PATCH v4 net-next 08/10] net/ncsi: Support NCSI packet generation
From: Gavin Shan @ 2017-05-04  6:31 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Gavin Shan, netdev, joe, kubakici, f.fainelli, davem
In-Reply-To: <20170503125254.GF8029@lunn.ch>

On Wed, May 03, 2017 at 02:52:54PM +0200, Andrew Lunn wrote:
>On Wed, May 03, 2017 at 02:44:39PM +1000, Gavin Shan wrote:
>> This introduces /sys/kernel/debug/ncsi/eth0/pkt. The debugfs entry
>> can accept parameters to produce NCSI command packet. The received
>> NCSI response packet is dumped on read. Below is an example to send
>> CIS command and dump its response.
>> 
>>    # echo CIS,0,0 > /sys/kernel/debug/ncsi/eth0/pkt
>>    # cat /sys/kernel/debug/ncsi/eth0/pkt
>>    NCSI response [CIS] packet received
>> 
>>    00 01 dd 80 00 0004 0000 0000
>
>Could this be done with a raw socket for Tx and
>libpcap/tcpdump/wireshart for Rx?
>

Andrew, it's really good question. Unfortunately, I don't think it can
be done solely by raw socket to transmit NCSI command packet because the
[packet sequence number] field in the packet can't be same to any used ones.
Otherwise the remote NIC will be confused and start to reponse abnormally.

We could reserve some sequence number to be used by raw socket. However, to
avoid  duplicated packet sequence number from raw socket should be done. I
think it's overall more complexed than current implementation (debugfs). Also,
it's going to introduce protocol specific rules to raw socket implementation.
I'm not sure it's worthy.

Cheers,
Gavin

^ permalink raw reply

* Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology
From: Gavin Shan @ 2017-05-04  6:17 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Gavin Shan, netdev, joe, kubakici, f.fainelli, davem
In-Reply-To: <20170503222111.749ce90b@xeon-e3>

On Wed, May 03, 2017 at 10:21:11PM -0700, Stephen Hemminger wrote:
>On Wed,  3 May 2017 14:44:35 +1000
>Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>
>> +void ncsi_ethtool_register_dev(struct net_device *dev)
>> +{
>> +	struct ethtool_ops *ops;
>> +
>> +	ops = (struct ethtool_ops *)(dev->ethtool_ops);
>> +	if (!ops)
>> +		return;
>> +
>> +	ops->get_ncsi_channels = ncsi_get_channels;
>> +}
>> +
>
>Instead of casting away const which opens up potential security
>issues. Have two ethtool_ops structures.
>

Thanks for the comments, Stephen. Yeah, It should be corrected
as Andrew already suggested.

Cheers,
Gavin

^ permalink raw reply

* Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology
From: Gavin Shan @ 2017-05-04  6:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Gavin Shan, netdev, joe, kubakici, f.fainelli, davem
In-Reply-To: <20170503221944.1dd0d576@xeon-e3>

On Wed, May 03, 2017 at 10:19:44PM -0700, Stephen Hemminger wrote:
>On Wed,  3 May 2017 14:44:35 +1000
>Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>
>> +static int ethtool_get_ncsi_channels(struct net_device *dev,
>> +				     void __user *useraddr)
>
>Please don't use an opaque type for this. See how other ethtool
>operations take a struct.
>

After checking output from below command, all other ethtool operations
uses "void __user *" or "char __user *".

   git grep static.*useraddr net/core/ethtool.c

>> +{
>> +	struct ethtool_ncsi_channels *enc;
>> +	short nr_channels;
>Should be __u16 or unsigned not short.
>

Nope, It's for signed number. User expects to get number of available
channels when negative number is passed in. When it's positive, it's
going to get the channels' information.

>> +	ssize_t size = 0;
>> +	int ret;
>> +
>> +	if (!dev->ethtool_ops->get_ncsi_channels)
>> +		return -EOPNOTSUPP;
>> +
>> +	if (copy_from_user(&nr_channels, useraddr + sizeof(enc->cmd),
>> +			   sizeof(nr_channels)))
>> +		return -EFAULT;
>> +
>> +	size = sizeof(*enc);
>> +	if (nr_channels > 0)
>> +		size += nr_channels * sizeof(enc->id[0]);
>
>You have no upper bound on number of channels, and therefore an incorrectly
>application could grab an excessive amount of kernel memory.
>

Yeah, I'll limit it to 256 in next respin. 256 is the maximal number
of channels for one particular net device.

Cheers,
Gavin

^ permalink raw reply

* Re: new warning at net/wireless/util.c:1236
From: Coelho, Luciano @ 2017-05-04  6:06 UTC (permalink / raw)
  To: torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
	kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org
In-Reply-To: <87o9v99r3g.fsf-5ukZ45wKbUHoml4zekdYB16hYfS7NtTn@public.gmane.org>

On Thu, 2017-05-04 at 07:35 +0300, Kalle Valo wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > So my Dell XPS 13 seems to have grown a new warning as of the
> > networking merge yesterday.
> > 
> > Things still work, but when it starts warning, it generates a *lot* of
> > noise (I got 36 of these within about ten minutes).
> > 
> > I have no idea what triggered it, because when I rebooted (not because
> > of this issue, but just to reboot into a newer kernel) I don't see it
> > again.
> > 
> > This is all pretty regular wireless - it's intel 8260 wireless in a
> > fairly normal laptop.
> > 
> > Things still seem to *work* ok, so the only problem here is the overly
> > verbose and useless WARN_ON. It doesn't even print out *which* rate it
> > is warning about, it just does that stupid unconditional WARN_ON()
> > without ever shutting up about it..
> > 
> > The WARN_ON() seems to be old, but my logs don't seem to have any
> > mention of this until today, so there's something that has changed
> > that now triggers it.
> > 
> > Ideas?
> > 
> >                   Linus
> > 
> > ---
> > 
> > WARNING: CPU: 3 PID: 1138 at net/wireless/util.c:1236
> > cfg80211_calculate_bitrate+0x139/0x170 [cfg80211]
> 
> As this is with iwlwifi adding also Luca. There were some rate handling
> changes in iwlwifi, like commit 77e409455f41, but don't know if that
> could cause this.

Thanks Kalle.  I don't see anything in the iwlwifi driver that could be
causing this.  Johannes suspects some RX rate changes he made in
mac80211...

--
Luca.

^ permalink raw reply

* Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology
From: Stephen Hemminger @ 2017-05-04  5:21 UTC (permalink / raw)
  To: Gavin Shan; +Cc: netdev, joe, kubakici, f.fainelli, davem
In-Reply-To: <1493786681-27468-5-git-send-email-gwshan@linux.vnet.ibm.com>

On Wed,  3 May 2017 14:44:35 +1000
Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:

> +void ncsi_ethtool_register_dev(struct net_device *dev)
> +{
> +	struct ethtool_ops *ops;
> +
> +	ops = (struct ethtool_ops *)(dev->ethtool_ops);
> +	if (!ops)
> +		return;
> +
> +	ops->get_ncsi_channels = ncsi_get_channels;
> +}
> +

Instead of casting away const which opens up potential security
issues. Have two ethtool_ops structures.

^ permalink raw reply

* Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology
From: Stephen Hemminger @ 2017-05-04  5:19 UTC (permalink / raw)
  To: Gavin Shan; +Cc: netdev, joe, kubakici, f.fainelli, davem
In-Reply-To: <1493786681-27468-5-git-send-email-gwshan@linux.vnet.ibm.com>

On Wed,  3 May 2017 14:44:35 +1000
Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:

> +static int ethtool_get_ncsi_channels(struct net_device *dev,
> +				     void __user *useraddr)

Please don't use an opaque type for this. See how other ethtool
operations take a struct.

> +{
> +	struct ethtool_ncsi_channels *enc;
> +	short nr_channels;
Should be __u16 or unsigned not short.

> +	ssize_t size = 0;
> +	int ret;
> +
> +	if (!dev->ethtool_ops->get_ncsi_channels)
> +		return -EOPNOTSUPP;
> +
> +	if (copy_from_user(&nr_channels, useraddr + sizeof(enc->cmd),
> +			   sizeof(nr_channels)))
> +		return -EFAULT;
> +
> +	size = sizeof(*enc);
> +	if (nr_channels > 0)
> +		size += nr_channels * sizeof(enc->id[0]);

You have no upper bound on number of channels, and therefore an incorrectly
application could grab an excessive amount of kernel memory.

^ permalink raw reply

* 58960 netdev
From: kelley @ 2017-05-04  5:11 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: 374278179.zip --]
[-- Type: application/zip, Size: 3233 bytes --]

^ permalink raw reply

* [net-next] net: remove duplicate add_device_randomness() call
From: Zhang Shengju @ 2017-05-04  3:40 UTC (permalink / raw)
  To: davem, netdev, edumazet

Since register_netdevice() already call add_device_randomness() and
dev_set_mac_address() will call it after mac address change.
It's not necessary to call at device UP.

Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
---
 net/core/dev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 35a06ce..cb48e40 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1347,7 +1347,6 @@ static int __dev_open(struct net_device *dev)
 		dev->flags |= IFF_UP;
 		dev_set_rx_mode(dev);
 		dev_activate(dev);
-		add_device_randomness(dev->dev_addr, dev->addr_len);
 	}
 
 	return ret;
-- 
1.8.3.1

^ permalink raw reply related

* [Patch net] ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf
From: Cong Wang @ 2017-05-04  5:07 UTC (permalink / raw)
  To: netdev; +Cc: andreyknvl, dsahern, Cong Wang

For each netns (except init_net), we initialize its null entry
in 3 places:

1) The template itself, as we use kmemdup()
2) Code around dst_init_metrics() in ip6_route_net_init()
3) ip6_route_dev_notify(), which is supposed to initialize it after
loopback registers

Unfortunately the last one still happens in a wrong order because
we expect to initialize net->ipv6.ip6_null_entry->rt6i_idev to
net->loopback_dev's idev, so we have to do that after we add
idev to it. However, this notifier has priority == 0 same as
ipv6_dev_notf, and ipv6_dev_notf is registered after
ip6_route_dev_notifier so it is called actually after
ip6_route_dev_notifier.

Fix it by specifying a smaller priority for ip6_route_dev_notifier.

Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 2f11366..4dbf7e2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4024,7 +4024,7 @@ static struct pernet_operations ip6_route_net_late_ops = {
 
 static struct notifier_block ip6_route_dev_notifier = {
 	.notifier_call = ip6_route_dev_notify,
-	.priority = 0,
+	.priority = -10, /* Must be called after addrconf_notify!! */
 };
 
 void __init ip6_route_init_special_entries(void)
-- 
2.5.5

^ permalink raw reply related

* [Patch net] ipv6: initialize route null entry in addrconf_init()
From: Cong Wang @ 2017-05-04  5:07 UTC (permalink / raw)
  To: netdev; +Cc: andreyknvl, dsahern, Cong Wang

Andrey reported a crash on init_net.ipv6.ip6_null_entry->rt6i_idev
since it is always NULL.

This is clearly wrong, we have code to initialize it to loopback_dev,
unfortunately the order is still not correct.

loopback_dev is registered very early during boot, we lose a chance
to re-initialize it in notifier. addrconf_init() is called after
ip6_route_init(), which means we have no chance to correct it.

Fix it by moving this initialization explicitly after
ipv6_add_dev(init_net.loopback_dev) in addrconf_init().

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/ip6_route.h |  1 +
 net/ipv6/addrconf.c     |  2 ++
 net/ipv6/route.c        | 26 +++++++++++++++-----------
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 9dc2c18..f5e625f 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -84,6 +84,7 @@ struct dst_entry *ip6_route_lookup(struct net *net, struct flowi6 *fl6,
 struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 			       int ifindex, struct flowi6 *fl6, int flags);
 
+void ip6_route_init_special_entries(void);
 int ip6_route_init(void);
 void ip6_route_cleanup(void);
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a2a370b..77a4bd5 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -6573,6 +6573,8 @@ int __init addrconf_init(void)
 		goto errlo;
 	}
 
+	ip6_route_init_special_entries();
+
 	for (i = 0; i < IN6_ADDR_HSIZE; i++)
 		INIT_HLIST_HEAD(&inet6_addr_lst[i]);
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index a1bf426..2f11366 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4027,6 +4027,21 @@ static struct notifier_block ip6_route_dev_notifier = {
 	.priority = 0,
 };
 
+void __init ip6_route_init_special_entries(void)
+{
+	/* Registering of the loopback is done before this portion of code,
+	 * the loopback reference in rt6_info will not be taken, do it
+	 * manually for init_net */
+	init_net.ipv6.ip6_null_entry->dst.dev = init_net.loopback_dev;
+	init_net.ipv6.ip6_null_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
+  #ifdef CONFIG_IPV6_MULTIPLE_TABLES
+	init_net.ipv6.ip6_prohibit_entry->dst.dev = init_net.loopback_dev;
+	init_net.ipv6.ip6_prohibit_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
+	init_net.ipv6.ip6_blk_hole_entry->dst.dev = init_net.loopback_dev;
+	init_net.ipv6.ip6_blk_hole_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
+  #endif
+}
+
 int __init ip6_route_init(void)
 {
 	int ret;
@@ -4053,17 +4068,6 @@ int __init ip6_route_init(void)
 
 	ip6_dst_blackhole_ops.kmem_cachep = ip6_dst_ops_template.kmem_cachep;
 
-	/* Registering of the loopback is done before this portion of code,
-	 * the loopback reference in rt6_info will not be taken, do it
-	 * manually for init_net */
-	init_net.ipv6.ip6_null_entry->dst.dev = init_net.loopback_dev;
-	init_net.ipv6.ip6_null_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
-  #ifdef CONFIG_IPV6_MULTIPLE_TABLES
-	init_net.ipv6.ip6_prohibit_entry->dst.dev = init_net.loopback_dev;
-	init_net.ipv6.ip6_prohibit_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
-	init_net.ipv6.ip6_blk_hole_entry->dst.dev = init_net.loopback_dev;
-	init_net.ipv6.ip6_blk_hole_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
-  #endif
 	ret = fib6_init();
 	if (ret)
 		goto out_register_subsys;
-- 
2.5.5

^ permalink raw reply related

* Re: new warning at net/wireless/util.c:1236
From: Kalle Valo @ 2017-05-04  4:35 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: David Miller, Johannes Berg, Linux Wireless List,
	Network Development, Linux Kernel Mailing List, Luca Coelho
In-Reply-To: <CA+55aFxyp5gpUZEYHXLwyhbK6DMWf4RoVykGxq9FPtO=j4yJsw@mail.gmail.com>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> So my Dell XPS 13 seems to have grown a new warning as of the
> networking merge yesterday.
>
> Things still work, but when it starts warning, it generates a *lot* of
> noise (I got 36 of these within about ten minutes).
>
> I have no idea what triggered it, because when I rebooted (not because
> of this issue, but just to reboot into a newer kernel) I don't see it
> again.
>
> This is all pretty regular wireless - it's intel 8260 wireless in a
> fairly normal laptop.
>
> Things still seem to *work* ok, so the only problem here is the overly
> verbose and useless WARN_ON. It doesn't even print out *which* rate it
> is warning about, it just does that stupid unconditional WARN_ON()
> without ever shutting up about it..
>
> The WARN_ON() seems to be old, but my logs don't seem to have any
> mention of this until today, so there's something that has changed
> that now triggers it.
>
> Ideas?
>
>                   Linus
>
> ---
>
> WARNING: CPU: 3 PID: 1138 at net/wireless/util.c:1236
> cfg80211_calculate_bitrate+0x139/0x170 [cfg80211]

As this is with iwlwifi adding also Luca. There were some rate handling
changes in iwlwifi, like commit 77e409455f41, but don't know if that
could cause this.

-- 
Kalle Valo

^ permalink raw reply

* Re: net/ipv6: GPF in rt6_device_match
From: David Ahern @ 2017-05-04  4:14 UTC (permalink / raw)
  To: Cong Wang
  Cc: Andrey Konovalov, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Dmitry Vyukov,
	Kostya Serebryany, Eric Dumazet, syzkaller
In-Reply-To: <CAM_iQpU8rBnJEk7NRdJ9bBan93BpjFPeVV7Yz9QSb0O-O+=BtQ@mail.gmail.com>

On 5/3/17 9:55 PM, Cong Wang wrote:
> Why not add a printk and play with my patch to see the difference?

I have other things to do. If you believe your patch fixes the problem,
send it and let Andrey verify.

^ permalink raw reply

* Re: net/ipv6: GPF in rt6_device_match
From: Cong Wang @ 2017-05-04  3:55 UTC (permalink / raw)
  To: David Ahern
  Cc: Andrey Konovalov, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Dmitry Vyukov,
	Kostya Serebryany, Eric Dumazet, syzkaller
In-Reply-To: <79157b41-2a13-6fda-ef31-c96f16850254@gmail.com>

On Wed, May 3, 2017 at 7:43 PM, David Ahern <dsahern@gmail.com> wrote:
> On 5/3/17 5:35 PM, Cong Wang wrote:
>> Ah, we need:
>>
>> @@ -4024,7 +4027,7 @@ static struct pernet_operations ip6_route_net_late_ops = {
>>
>>  static struct notifier_block ip6_route_dev_notifier = {
>>         .notifier_call = ip6_route_dev_notify,
>> -       .priority = 0,
>> +       .priority = -10, /* Must be called after addrconf_notify!! */
>>  };
>>
>
>
> It's not a notifier problem; the null_entry is created in ip6_route_init
> which is an init function.

Only init_net's null entry is created here.

>
> For network namespaces other than init_net, it is never initialized. See
> ip6_route_net_init.

I don't understand what you are talking about...

It is obviously initialized in 3 places:

1) The template itself, as we use memdup()
2) Code around dst_init_metrics() in ip6_route_net_init()
3) ip6_route_dev_notify(), which is supposed to initialize it after
loopback registers (the order needs to fix, as shown in my patch)

Why not add a printk and play with my patch to see the difference?

^ permalink raw reply

* Re: [PATCH net-next] selftests/bpf: get rid of -D__x86_64__
From: Alexei Starovoitov @ 2017-05-04  3:30 UTC (permalink / raw)
  To: David Miller; +Cc: daniel, netdev
In-Reply-To: <20170503.133512.310640909764585408.davem@davemloft.net>

On 5/3/17 10:35 AM, David Miller wrote:
> From: Alexei Starovoitov <ast@fb.com>
> Date: Wed, 3 May 2017 09:54:42 -0700
>
>> /usr/include/asm/types.h -> asm-generic/int-ll64.h
>> as far as I can see that should be the same on most archs.
>> Why doesn't it work for sparc?
>
> You can't assume anything about the kernel headers installed,
> on my debian Sparc box /usr/include/asm/types.h is below.
>
> They do things this way to facilitate multiarch building.  I think
> it's pretty reasonable.
>
> #ifndef _SPARC_TYPES_H
> #define _SPARC_TYPES_H
> /*
>  * This file is never included by application software unless
>  * explicitly requested (e.g., via linux/types.h) in which case the
>  * application is Linux specific so (user-) name space pollution is
>  * not a major issue.  However, for interoperability, libraries still
>  * need to be careful to avoid a name clashes.
>  */
>
> #if defined(__sparc__)
>
> #include <asm-generic/int-ll64.h>
>
> #ifndef __ASSEMBLY__
>
> typedef unsigned short umode_t;
>
> #endif /* __ASSEMBLY__ */
>
> #endif /* defined(__sparc__) */

if it was something like
#ifdef __sparc__
...
#else
#include_next <asm/types.h>

I would buy that debian folks indeed care about multi-arch, but
what above does is making #include <linux/types.h> to be a nop
for any cross-compiler on sparc that included it.
Which is probably quite painful to debug as we found out.
You're right that we cannot assume much about /usr/include craziness.
In that sense adding __native_arch__ macro is also wrong, since
it assumes sane /usr/include without inline asm or other things
that clang for bpf arch can consume.
In that sense the only way to be independent from arch dependent
things in /usr/include is to put all arch specific headers
into our own dir in tools/selftests/ (or may be tools/bpf/include)
and point clang to that. I think the list of .h in there will be
limited. Only things like linux/types.h and gnu/stubs.h,
so it will be manageable.
Thoughts?

^ permalink raw reply

* RE: FEC on i.MX 7 transmit queue timeout
From: Andy Duan @ 2017-05-04  3:08 UTC (permalink / raw)
  To: Stefan Agner
  Cc: fugang.duan@freescale.com, festevam@gmail.com,
	netdev@vger.kernel.org, netdev-owner@vger.kernel.org
In-Reply-To: <e366479239b66ba80415b5594def2369@agner.ch>

From: Stefan Agner <stefan@agner.ch> Sent: Thursday, May 04, 2017 9:22 AM
>To: Andy Duan <fugang.duan@nxp.com>
>Cc: fugang.duan@freescale.com; festevam@gmail.com;
>netdev@vger.kernel.org; netdev-owner@vger.kernel.org
>Subject: Re: FEC on i.MX 7 transmit queue timeout
>
>Hi Andy,
>
>On 2017-04-20 19:48, Andy Duan wrote:
>> On 2017年04月20日 07:15, Stefan Agner wrote:
>>> I tested again with imx6sx-fec compatible string. I could reproduce
>>> it on a Colibri with i.MX 7Dual. But not always: It really depends
>>> whether queue 2 is counting up or not. Just after boot, I check
>>> /proc/interrupts twice, if queue 2 is counting it will happen!
>>>
>>> But if only queue 0 is mostly in use, then it seems to work just fine.
>> If your case is only running best effort like tcp/udp, you can re-set
>> the "fsl,num-tx-queues" and "fsl,num-rx-queues" to 1 in board dts file.
>> Other two queues are for AVB audio/video queues, they have high
>> priority than queue 0. If running iperf tcp test on the three queues,
>> then the tcp segment may be out-of-order that cause net watchdog
>timeout.
>>>
>>> I also tried i.MX 7Dual SabreSD here, and the same thing. I had to
>>> reboot 3 times, then queue 2 was counting:
>>>   57:          8     GIC-0 150 Level     30be0000.ethernet
>>>   58:      20137     GIC-0 151 Level     30be0000.ethernet
>>>   59:       9269     GIC-0 152 Level     30be0000.ethernet
>>>
>>> It took me about 40 minutes on Sabre until it happened, and I had to
>>> force it using iperf, but then I got the ring dumps:
>> My board had ran more than 47 hours with nfs rootfs in 4.11.0-rc6, but
>> not running iperf.
>> I am testing with iperf.
>
>Any update on this issue?
>
>When using iperf (server) on the board with Linux 4.11 the issue appears
>within a few iperf iterations on a Sabre (TO 1.2, Board Rev C, if that matters)...
>
I don’t know whether you received my last mail. (maybe failed due to I received some rejection mails)

If your case is only running best effort like tcp/udp, you can re-set the "fsl,num-tx-queues" and "fsl,num-rx-queues" to 1 in board dts file.
Other two queues are for AVB audio/video queues, they have high priority than queue 0. If running iperf tcp test on the three queues, then the tcp segment may be out-of-order that cause net watchdog timeout.
In fsl kernel tree, there have one patch that only select the queue0 for best effort like tcp/udp. Pls test again in your board, if no problem I will upstream the patch.

^ permalink raw reply

* Re: net/ipv6: GPF in rt6_device_match
From: David Ahern @ 2017-05-04  2:43 UTC (permalink / raw)
  To: Cong Wang
  Cc: Andrey Konovalov, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Dmitry Vyukov,
	Kostya Serebryany, Eric Dumazet, syzkaller
In-Reply-To: <CAM_iQpVbcOL3BD6SkVAOFLVCX5q_OpO0vfYnwG6XE4vdJkiw5Q@mail.gmail.com>

On 5/3/17 5:35 PM, Cong Wang wrote:
> Ah, we need:
> 
> @@ -4024,7 +4027,7 @@ static struct pernet_operations ip6_route_net_late_ops = {
> 
>  static struct notifier_block ip6_route_dev_notifier = {
>         .notifier_call = ip6_route_dev_notify,
> -       .priority = 0,
> +       .priority = -10, /* Must be called after addrconf_notify!! */
>  };
> 


It's not a notifier problem; the null_entry is created in ip6_route_init
which is an init function.

For network namespaces other than init_net, it is never initialized. See
ip6_route_net_init.

^ permalink raw reply

* new warning at net/wireless/util.c:1236
From: Linus Torvalds @ 2017-05-04  2:28 UTC (permalink / raw)
  To: David Miller, Johannes Berg
  Cc: Linux Wireless List, Network Development,
	Linux Kernel Mailing List

So my Dell XPS 13 seems to have grown a new warning as of the
networking merge yesterday.

Things still work, but when it starts warning, it generates a *lot* of
noise (I got 36 of these within about ten minutes).

I have no idea what triggered it, because when I rebooted (not because
of this issue, but just to reboot into a newer kernel) I don't see it
again.

This is all pretty regular wireless - it's intel 8260 wireless in a
fairly normal laptop.

Things still seem to *work* ok, so the only problem here is the overly
verbose and useless WARN_ON. It doesn't even print out *which* rate it
is warning about, it just does that stupid unconditional WARN_ON()
without ever shutting up about it..

The WARN_ON() seems to be old, but my logs don't seem to have any
mention of this until today, so there's something that has changed
that now triggers it.

Ideas?

                  Linus

---

WARNING: CPU: 3 PID: 1138 at net/wireless/util.c:1236
cfg80211_calculate_bitrate+0x139/0x170 [cfg80211]
Modules linked in: rfcomm fuse ccm ip6t_rpfilter ip6t_REJECT
nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_broute bridge stp
llc ebtable_nat ip6table_security ip6table_mangle ip6table_nat nf_con
 snd_hda_codec iwlmvm irqbypass snd_hwdep snd_hda_core intel_cstate
mac80211 snd_seq intel_rapl_perf snd_seq_device snd_pcm iwlwifi
rtsx_pci_ms snd_timer cfg80211 memstick snd soundcore i2c_i801 shpchp
 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt
fb_sys_fops drm i2c_hid video
CPU: 3 PID: 1138 Comm: NetworkManager Tainted: G        W
4.11.0-06543-g2f34c1231bfc #60
Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.4.13 12/28/2016
task: ffff9c1d1bfcbb80 task.stack: ffffbb95c337c000
RIP: 0010:cfg80211_calculate_bitrate+0x139/0x170 [cfg80211]
RSP: 0018:ffffbb95c337f5b8 EFLAGS: 00010293
RAX: 0000000000000000 RBX: ffff9c1cb080cc00 RCX: 0000000000000000
RDX: 0000000000000005 RSI: 0000000000000002 RDI: ffffbb95c337f76e
RBP: ffffbb95c337f5b8 R08: 0000000000000004 R09: ffff9c1cc36fe0c4
R10: 00000000f7c00472 R11: ffffffffc0682000 R12: ffff9c1cc36fe0c0
R13: ffffbb95c337f76e R14: 0000000000000000 R15: ffff9c1cc36fe030
FS:  00007f000015f980(0000) GS:ffff9c1d3ed80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffd1ae62748 CR3: 0000000469f4f000 CR4: 00000000003406e0
Call Trace:
 nl80211_put_sta_rate+0x56/0x210 [cfg80211]
 nl80211_send_station.isra.63+0x639/0xd60 [cfg80211]
 nl80211_get_station+0x1e4/0x250 [cfg80211]
 genl_family_rcv_msg+0x1fa/0x3e0
 genl_rcv_msg+0x4c/0x90
 netlink_rcv_skb+0xde/0x110
 genl_rcv+0x28/0x40
 netlink_unicast+0x189/0x220
 netlink_sendmsg+0x2ba/0x3b0
 sock_sendmsg+0x38/0x50
 ___sys_sendmsg+0x2b6/0x2d0
 __sys_sendmsg+0x54/0x90
 SyS_sendmsg+0x12/0x20
 entry_SYSCALL_64_fastpath+0x13/0x94
RIP: 0033:0x7efffebf82fd
RSP: 002b:00007fff97d4af30 EFLAGS: 00000293 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007efffebf82fd
RDX: 0000000000000000 RSI: 00007fff97d4afc0 RDI: 0000000000000010
RBP: 00007fff97d4b070 R08: 0000000000000000 R09: 00007efffcc7b168
R10: 000055eb69e6d110 R11: 0000000000000293 R12: 00007fff97d4b0e0
R13: 0000000000000001 R14: 0000000000000000 R15: 000055eb68a32760
Code: 89 d0 f7 e1 d1 ea 8d 14 92 01 d2 81 c2 50 c3 00 00 b9 c5 5a 7c
0a c1 ea 05 89 d0 f7 e1 5d 89 d0 c1 e8 07 c3 31 c0 80 f9 02 74 b7 <0f>
ff 31 c0 eb b1 0f ff 31 c0 5d c3 0f ff 31 c0 5d c3 8d 04 40

^ permalink raw reply

* Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data
From: Luis R. Rodriguez @ 2017-05-04  2:28 UTC (permalink / raw)
  To: Arend Van Spriel
  Cc: Luis R. Rodriguez, Pavel Machek, Daniel Wagner, Tom Gundersen,
	Pali Rohár, Ming Lei, Greg Kroah-Hartman, Kalle Valo,
	David Gnedt, Michal Kazior, Tony Lindgren, Sebastian Reichel,
	Ivaylo Dimitrov, Aaro Koskinen, Takashi Iwai, David Woodhouse,
	Bjorn Andersson, Grazvydas Ignotas, linux-kernel, linux-wireless
In-Reply-To: <936bf348-58ac-882c-a433-83f209862deb@broadcom.com>

On Wed, May 03, 2017 at 09:02:20PM +0200, Arend Van Spriel wrote:
> On 3-1-2017 18:59, Luis R. Rodriguez wrote:
> > On Mon, Dec 26, 2016 at 05:35:59PM +0100, Pavel Machek wrote:
> >>
> >> Right question is "should we solve it without user-space help"?
> >>
> >> Answer is no, too. Way too complex. Yes, it would be nice if hardware
> >> was designed in such a way that getting calibration data from kernel
> >> is easy, and if you design hardware, please design it like that. But
> >> N900 is not designed like that and getting the calibration through
> >> userspace looks like only reasonable solution.
> > 
> > Arend seems to have a better alternative in mind possible for other
> > devices which *can* probably pull of doing this easily and nicely,
> > given the nasty history of the usermode helper crap we should not
> > in any way discourage such efforts.
> > 
> > Arend -- please look at the firmware cache, it not a hash but a hash
> > table for an O(1) lookups would be a welcomed change, then it could
> > be repurposed for what you describe, I think the only difference is
> > you'd perhaps want a custom driver hook to fetch the calibration data
> > so the driver does whatever it needs.
> 
> Hi Luis,
> 
> I let my idea catch dust on the shelf for a while. 

:) BTW did you get to check out Daniel Wagner and Tom Gundersen's firmware work
[0] ? This can provide a proper clear fallback mechanism which *also* helps
address the race between "critical mount points ready" problem we had discussed
long ago. IIRC it did this by having two modes of operation a best effort-mode
and a final-mode. The final-mode would only be used once all the real rootfs is
ready. Userspace decides when to kick / signal firmwared to switch to final-mode
as only userspace will know for sure when that is ready. The best-effort mode
would be used in initramfs. There is also no need for a "custom fallback", the
uevent fallback mechanism can be relied upon alone. Custom tools can just fork
off and do something similar to firmward then in terms of architecture. This is
a form of fallback mechanism I think I'd be happy to see enabled on the new
driver data API. But of course, I recall also liking what you had in mind as well
so would be happy to see more alternatives! The cleaner the better !

[0] https://github.com/teg/firmwared

> Actually had a couple
> of patches ready, but did get around testing them. So I wanted to rebase
> them on your linux-next tree. I bumped into the umh lock thing and was
> wondering why direct filesystem access was under that lock as well.

Indeed, it was an odd thing.

> In your tree I noticed a fix for that. 

Yup!

It took a lot of git archeology to reach a sound approach forward which makes
me feel comfortable without regressing the kernel, this should not regress
the kernel.

> The more reason to base my work on
> top of your firmware_class changes. Now my question is what is the best
> branch to choose, because you have a "few" in that repo to choose from ;-)

I have a series of topical changes, and I rebase onto the latest linux-next
regularly before posting patches, if 0-day finds issues, I dish out a next
try2 or tryX iteration until all issues are fixed. So in this case you'd
just go for the latest driver-data-$(next_date) and if there is a try
postfix use the latest tryX.

In this case 20170501-driver-data-try2, this is based on linux-next tag
next-20170501. If you have issues booting on that next tag though you
could instead try the v4.11-rc8-driver-data-try3 branch based on v4.11-rc8.
But naturally patches ultimately should be based on the latest linux-next
tag for actual submission.

PS. after my changes the fallback mechanism can easily be shoved into its
own file, not sure if that helps with how clean of a solution your work
will be.

  Luis

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox