* [PATCH 0/5] net: hwmon fixes
From: Guenter Roeck @ 2013-11-23 6:07 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Nithin Nayak Sujir,
Michael Chan, e1000-devel, lm-sensors, Guenter Roeck
The hwmon subsystem is used by various network drivers to report temperature
sensor and other information. Unfortunately, its use is often not correct.
Typical errors are that the mandatory name sysfs attribute is not created,
that the temperature sensor index starts with 0 instead of 1, and/or that
sysfs attributes are created after the hwmon device was created.
The following sequence of patches fixes most of the problems.
The igb patches have been tested with real hardware; the others are compile
tested only.
^ permalink raw reply
* [PATCH 1/5] tg3: Convert to use hwmon_device_register_with_groups
From: Guenter Roeck @ 2013-11-23 6:07 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Nithin Nayak Sujir,
Michael Chan, e1000-devel, lm-sensors, Guenter Roeck
In-Reply-To: <1385186881-7931-1-git-send-email-linux@roeck-us.net>
Use new hwmon API to simplify code, provide missing mandatory 'name'
sysfs attribute, and attach hwmon attributes to hwmon device instead
of pci device.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/net/ethernet/broadcom/tg3.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index a9e0684..369b736 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -10629,10 +10629,8 @@ static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
static ssize_t tg3_show_temp(struct device *dev,
struct device_attribute *devattr, char *buf)
{
- struct pci_dev *pdev = to_pci_dev(dev);
- struct net_device *netdev = pci_get_drvdata(pdev);
- struct tg3 *tp = netdev_priv(netdev);
struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+ struct tg3 *tp = dev_get_drvdata(dev);
u32 temperature;
spin_lock_bh(&tp->lock);
@@ -10650,29 +10648,25 @@ static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, tg3_show_temp, NULL,
static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, tg3_show_temp, NULL,
TG3_TEMP_MAX_OFFSET);
-static struct attribute *tg3_attributes[] = {
+static struct attribute *tg3_attrs[] = {
&sensor_dev_attr_temp1_input.dev_attr.attr,
&sensor_dev_attr_temp1_crit.dev_attr.attr,
&sensor_dev_attr_temp1_max.dev_attr.attr,
NULL
};
-
-static const struct attribute_group tg3_group = {
- .attrs = tg3_attributes,
-};
+ATTRIBUTE_GROUPS(tg3);
static void tg3_hwmon_close(struct tg3 *tp)
{
if (tp->hwmon_dev) {
hwmon_device_unregister(tp->hwmon_dev);
tp->hwmon_dev = NULL;
- sysfs_remove_group(&tp->pdev->dev.kobj, &tg3_group);
}
}
static void tg3_hwmon_open(struct tg3 *tp)
{
- int i, err;
+ int i;
u32 size = 0;
struct pci_dev *pdev = tp->pdev;
struct tg3_ocir ocirs[TG3_SD_NUM_RECS];
@@ -10690,18 +10684,11 @@ static void tg3_hwmon_open(struct tg3 *tp)
if (!size)
return;
- /* Register hwmon sysfs hooks */
- err = sysfs_create_group(&pdev->dev.kobj, &tg3_group);
- if (err) {
- dev_err(&pdev->dev, "Cannot create sysfs group, aborting\n");
- return;
- }
-
- tp->hwmon_dev = hwmon_device_register(&pdev->dev);
+ tp->hwmon_dev = hwmon_device_register_with_groups(&pdev->dev, "tg3",
+ tp, tg3_groups);
if (IS_ERR(tp->hwmon_dev)) {
tp->hwmon_dev = NULL;
dev_err(&pdev->dev, "Cannot register hwmon device, aborting\n");
- sysfs_remove_group(&pdev->dev.kobj, &tg3_group);
}
}
--
1.7.9.7
^ permalink raw reply related
* [PATCH 2/5] igb: Convert to use devm_hwmon_device_register_with_groups
From: Guenter Roeck @ 2013-11-23 6:07 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Nithin Nayak Sujir,
Michael Chan, e1000-devel, lm-sensors, Guenter Roeck
In-Reply-To: <1385186881-7931-1-git-send-email-linux@roeck-us.net>
Simplify the code. Attach hwmon sysfs attributes to hwmon device
instead of pci device. Avoid race conditions caused by attributes
being created after registration and provide mandatory 'name'
attribute by using new hwmon API.
Other cleanup:
Instead of allocating memory for hwmon attributes, move attributes
and all other hwmon related data into struct hwmon_buff and allocate
the entire structure using devm_kzalloc.
Check return value from calls to igb_add_hwmon_attr() one by one instead
of logically combining them all together.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/net/ethernet/intel/igb/igb.h | 8 ++-
drivers/net/ethernet/intel/igb/igb_hwmon.c | 100 +++++++++++++---------------
2 files changed, 53 insertions(+), 55 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 5e9ed89..99c3d33 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -337,8 +337,10 @@ struct hwmon_attr {
};
struct hwmon_buff {
- struct device *device;
- struct hwmon_attr *hwmon_list;
+ struct attribute_group group;
+ const struct attribute_group *groups[2];
+ struct attribute *attrs[E1000_MAX_SENSORS * 4 + 1];
+ struct hwmon_attr hwmon_list[E1000_MAX_SENSORS * 4];
unsigned int n_hwmon;
};
#endif
@@ -440,7 +442,7 @@ struct igb_adapter {
char fw_version[32];
#ifdef CONFIG_IGB_HWMON
- struct hwmon_buff igb_hwmon_buff;
+ struct hwmon_buff *igb_hwmon_buff;
bool ets;
#endif
struct i2c_algo_bit_data i2c_algo;
diff --git a/drivers/net/ethernet/intel/igb/igb_hwmon.c b/drivers/net/ethernet/intel/igb/igb_hwmon.c
index 58f1ce9..2e7ef2d 100644
--- a/drivers/net/ethernet/intel/igb/igb_hwmon.c
+++ b/drivers/net/ethernet/intel/igb/igb_hwmon.c
@@ -117,8 +117,8 @@ static int igb_add_hwmon_attr(struct igb_adapter *adapter,
unsigned int n_attr;
struct hwmon_attr *igb_attr;
- n_attr = adapter->igb_hwmon_buff.n_hwmon;
- igb_attr = &adapter->igb_hwmon_buff.hwmon_list[n_attr];
+ n_attr = adapter->igb_hwmon_buff->n_hwmon;
+ igb_attr = &adapter->igb_hwmon_buff->hwmon_list[n_attr];
switch (type) {
case IGB_HWMON_TYPE_LOC:
@@ -154,30 +154,16 @@ static int igb_add_hwmon_attr(struct igb_adapter *adapter,
igb_attr->dev_attr.attr.mode = S_IRUGO;
igb_attr->dev_attr.attr.name = igb_attr->name;
sysfs_attr_init(&igb_attr->dev_attr.attr);
- rc = device_create_file(&adapter->pdev->dev,
- &igb_attr->dev_attr);
- if (rc == 0)
- ++adapter->igb_hwmon_buff.n_hwmon;
- return rc;
+ adapter->igb_hwmon_buff->attrs[n_attr] = &igb_attr->dev_attr.attr;
+
+ ++adapter->igb_hwmon_buff->n_hwmon;
+
+ return 0;
}
static void igb_sysfs_del_adapter(struct igb_adapter *adapter)
{
- int i;
-
- if (adapter == NULL)
- return;
-
- for (i = 0; i < adapter->igb_hwmon_buff.n_hwmon; i++) {
- device_remove_file(&adapter->pdev->dev,
- &adapter->igb_hwmon_buff.hwmon_list[i].dev_attr);
- }
-
- kfree(adapter->igb_hwmon_buff.hwmon_list);
-
- if (adapter->igb_hwmon_buff.device)
- hwmon_device_unregister(adapter->igb_hwmon_buff.device);
}
/* called from igb_main.c */
@@ -189,11 +175,11 @@ void igb_sysfs_exit(struct igb_adapter *adapter)
/* called from igb_main.c */
int igb_sysfs_init(struct igb_adapter *adapter)
{
- struct hwmon_buff *igb_hwmon = &adapter->igb_hwmon_buff;
+ struct hwmon_buff *igb_hwmon;
+ struct i2c_client *client;
+ struct device *hwmon_dev;
unsigned int i;
- int n_attrs;
int rc = 0;
- struct i2c_client *client = NULL;
/* If this method isn't defined we don't support thermals */
if (adapter->hw.mac.ops.init_thermal_sensor_thresh == NULL)
@@ -201,34 +187,16 @@ int igb_sysfs_init(struct igb_adapter *adapter)
/* Don't create thermal hwmon interface if no sensors present */
rc = (adapter->hw.mac.ops.init_thermal_sensor_thresh(&adapter->hw));
- if (rc)
- goto exit;
-
- /* init i2c_client */
- client = i2c_new_device(&adapter->i2c_adap, &i350_sensor_info);
- if (client == NULL) {
- dev_info(&adapter->pdev->dev,
- "Failed to create new i2c device..\n");
+ if (rc)
goto exit;
- }
- adapter->i2c_client = client;
- /* Allocation space for max attributes
- * max num sensors * values (loc, temp, max, caution)
- */
- n_attrs = E1000_MAX_SENSORS * 4;
- igb_hwmon->hwmon_list = kcalloc(n_attrs, sizeof(struct hwmon_attr),
- GFP_KERNEL);
- if (!igb_hwmon->hwmon_list) {
+ igb_hwmon = devm_kzalloc(&adapter->pdev->dev, sizeof(*igb_hwmon),
+ GFP_KERNEL);
+ if (!igb_hwmon) {
rc = -ENOMEM;
- goto err;
- }
-
- igb_hwmon->device = hwmon_device_register(&adapter->pdev->dev);
- if (IS_ERR(igb_hwmon->device)) {
- rc = PTR_ERR(igb_hwmon->device);
- goto err;
+ goto exit;
}
+ adapter->igb_hwmon_buff = igb_hwmon;
for (i = 0; i < E1000_MAX_SENSORS; i++) {
@@ -240,11 +208,39 @@ int igb_sysfs_init(struct igb_adapter *adapter)
/* Bail if any hwmon attr struct fails to initialize */
rc = igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_CAUTION);
- rc |= igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_LOC);
- rc |= igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_TEMP);
- rc |= igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_MAX);
if (rc)
- goto err;
+ goto exit;
+ rc = igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_LOC);
+ if (rc)
+ goto exit;
+ rc = igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_TEMP);
+ if (rc)
+ goto exit;
+ rc = igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_MAX);
+ if (rc)
+ goto exit;
+ }
+
+ /* init i2c_client */
+ client = i2c_new_device(&adapter->i2c_adap, &i350_sensor_info);
+ if (client == NULL) {
+ dev_info(&adapter->pdev->dev,
+ "Failed to create new i2c device.\n");
+ rc = -ENODEV;
+ goto exit;
+ }
+ adapter->i2c_client = client;
+
+ igb_hwmon->groups[0] = &igb_hwmon->group;
+ igb_hwmon->group.attrs = igb_hwmon->attrs;
+
+ hwmon_dev = devm_hwmon_device_register_with_groups(&adapter->pdev->dev,
+ client->name,
+ igb_hwmon,
+ igb_hwmon->groups);
+ if (IS_ERR(hwmon_dev)) {
+ rc = PTR_ERR(hwmon_dev);
+ goto err;
}
goto exit;
--
1.7.9.7
^ permalink raw reply related
* [PATCH 3/5] ixgbe: Convert to use devm_hwmon_device_register_with_groups
From: Guenter Roeck @ 2013-11-23 6:07 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Nithin Nayak Sujir,
Michael Chan, e1000-devel, lm-sensors, Guenter Roeck
In-Reply-To: <1385186881-7931-1-git-send-email-linux@roeck-us.net>
Simplify the code. Attach hwmon sysfs attributes to hwmon device
instead of pci device. Avoid race conditions caused by attributes
being created after hwmon device registration. Implicitly
(through hwmon API) add mandatory 'name' sysfs attribute.
Other cleanup:
Instead of allocating memory for hwmon attributes, move attributes
and all other hwmon related data into struct hwmon_buff and allocate
the entire structure using devm_kzalloc.
Check return value from calls to igb_add_hwmon_attr() one by one instead
of logically combining them all together.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 8 ++-
drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 76 ++++++++++--------------
2 files changed, 36 insertions(+), 48 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index f38fc0a..49531cd 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -552,8 +552,10 @@ struct hwmon_attr {
};
struct hwmon_buff {
- struct device *device;
- struct hwmon_attr *hwmon_list;
+ struct attribute_group group;
+ const struct attribute_group *groups[2];
+ struct attribute *attrs[IXGBE_MAX_SENSORS * 4 + 1];
+ struct hwmon_attr hwmon_list[IXGBE_MAX_SENSORS * 4];
unsigned int n_hwmon;
};
#endif /* CONFIG_IXGBE_HWMON */
@@ -775,7 +777,7 @@ struct ixgbe_adapter {
u32 vferr_refcount;
struct kobject *info_kobj;
#ifdef CONFIG_IXGBE_HWMON
- struct hwmon_buff ixgbe_hwmon_buff;
+ struct hwmon_buff *ixgbe_hwmon_buff;
#endif /* CONFIG_IXGBE_HWMON */
#ifdef CONFIG_DEBUG_FS
struct dentry *ixgbe_dbg_adapter;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c
index d118def..3081974 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c
@@ -111,8 +111,8 @@ static int ixgbe_add_hwmon_attr(struct ixgbe_adapter *adapter,
unsigned int n_attr;
struct hwmon_attr *ixgbe_attr;
- n_attr = adapter->ixgbe_hwmon_buff.n_hwmon;
- ixgbe_attr = &adapter->ixgbe_hwmon_buff.hwmon_list[n_attr];
+ n_attr = adapter->ixgbe_hwmon_buff->n_hwmon;
+ ixgbe_attr = &adapter->ixgbe_hwmon_buff->hwmon_list[n_attr];
switch (type) {
case IXGBE_HWMON_TYPE_LOC:
@@ -147,32 +147,17 @@ static int ixgbe_add_hwmon_attr(struct ixgbe_adapter *adapter,
ixgbe_attr->dev_attr.store = NULL;
ixgbe_attr->dev_attr.attr.mode = S_IRUGO;
ixgbe_attr->dev_attr.attr.name = ixgbe_attr->name;
+ sysfs_attr_init(&ixgbe_attr->dev_attr.attr);
- rc = device_create_file(&adapter->pdev->dev,
- &ixgbe_attr->dev_attr);
+ adapter->ixgbe_hwmon_buff->attrs[n_attr] = &ixgbe_attr->dev_attr.attr;
- if (rc == 0)
- ++adapter->ixgbe_hwmon_buff.n_hwmon;
+ ++adapter->ixgbe_hwmon_buff->n_hwmon;
- return rc;
+ return 0;
}
static void ixgbe_sysfs_del_adapter(struct ixgbe_adapter *adapter)
{
- int i;
-
- if (adapter == NULL)
- return;
-
- for (i = 0; i < adapter->ixgbe_hwmon_buff.n_hwmon; i++) {
- device_remove_file(&adapter->pdev->dev,
- &adapter->ixgbe_hwmon_buff.hwmon_list[i].dev_attr);
- }
-
- kfree(adapter->ixgbe_hwmon_buff.hwmon_list);
-
- if (adapter->ixgbe_hwmon_buff.device)
- hwmon_device_unregister(adapter->ixgbe_hwmon_buff.device);
}
/* called from ixgbe_main.c */
@@ -184,9 +169,9 @@ void ixgbe_sysfs_exit(struct ixgbe_adapter *adapter)
/* called from ixgbe_main.c */
int ixgbe_sysfs_init(struct ixgbe_adapter *adapter)
{
- struct hwmon_buff *ixgbe_hwmon = &adapter->ixgbe_hwmon_buff;
+ struct hwmon_buff *ixgbe_hwmon;
+ struct device *hwmon_dev;
unsigned int i;
- int n_attrs;
int rc = 0;
/* If this method isn't defined we don't support thermals */
@@ -198,23 +183,13 @@ int ixgbe_sysfs_init(struct ixgbe_adapter *adapter)
if (adapter->hw.mac.ops.init_thermal_sensor_thresh(&adapter->hw))
goto exit;
- /*
- * Allocation space for max attributs
- * max num sensors * values (loc, temp, max, caution)
- */
- n_attrs = IXGBE_MAX_SENSORS * 4;
- ixgbe_hwmon->hwmon_list = kcalloc(n_attrs, sizeof(struct hwmon_attr),
- GFP_KERNEL);
- if (!ixgbe_hwmon->hwmon_list) {
+ ixgbe_hwmon = devm_kzalloc(&adapter->pdev->dev, sizeof(*ixgbe_hwmon),
+ GFP_KERNEL);
+ if (ixgbe_hwmon == NULL) {
rc = -ENOMEM;
- goto err;
- }
-
- ixgbe_hwmon->device = hwmon_device_register(&adapter->pdev->dev);
- if (IS_ERR(ixgbe_hwmon->device)) {
- rc = PTR_ERR(ixgbe_hwmon->device);
- goto err;
+ goto exit;
}
+ adapter->ixgbe_hwmon_buff = ixgbe_hwmon;
for (i = 0; i < IXGBE_MAX_SENSORS; i++) {
/*
@@ -226,17 +201,28 @@ int ixgbe_sysfs_init(struct ixgbe_adapter *adapter)
/* Bail if any hwmon attr struct fails to initialize */
rc = ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_CAUTION);
- rc |= ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_LOC);
- rc |= ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_TEMP);
- rc |= ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_MAX);
if (rc)
- goto err;
+ goto exit;
+ rc = ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_LOC);
+ if (rc)
+ goto exit;
+ rc = ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_TEMP);
+ if (rc)
+ goto exit;
+ rc = ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_MAX);
+ if (rc)
+ goto exit;
}
- goto exit;
+ ixgbe_hwmon->groups[0] = &ixgbe_hwmon->group;
+ ixgbe_hwmon->group.attrs = ixgbe_hwmon->attrs;
-err:
- ixgbe_sysfs_del_adapter(adapter);
+ hwmon_dev = devm_hwmon_device_register_with_groups(&adapter->pdev->dev,
+ "ixgbe",
+ ixgbe_hwmon,
+ ixgbe_hwmon->groups);
+ if (IS_ERR(hwmon_dev))
+ rc = PTR_ERR(hwmon_dev);
exit:
return rc;
}
--
1.7.9.7
^ permalink raw reply related
* [PATCH 5/5] ixgbe: Start temperature sensor attribute index with 1
From: Guenter Roeck @ 2013-11-23 6:08 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Nithin Nayak Sujir,
Michael Chan, e1000-devel, lm-sensors, Guenter Roeck
In-Reply-To: <1385186881-7931-1-git-send-email-linux@roeck-us.net>
Per hwmon ABI, temperature sensor attribute index starts with 1, not 0.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c
index 3081974..e74ae36 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c
@@ -118,22 +118,22 @@ static int ixgbe_add_hwmon_attr(struct ixgbe_adapter *adapter,
case IXGBE_HWMON_TYPE_LOC:
ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_location;
snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name),
- "temp%u_label", offset);
+ "temp%u_label", offset + 1);
break;
case IXGBE_HWMON_TYPE_TEMP:
ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_temp;
snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name),
- "temp%u_input", offset);
+ "temp%u_input", offset + 1);
break;
case IXGBE_HWMON_TYPE_CAUTION:
ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_cautionthresh;
snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name),
- "temp%u_max", offset);
+ "temp%u_max", offset + 1);
break;
case IXGBE_HWMON_TYPE_MAX:
ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_maxopthresh;
snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name),
- "temp%u_crit", offset);
+ "temp%u_crit", offset + 1);
break;
default:
rc = -EPERM;
--
1.7.9.7
^ permalink raw reply related
* [PATCH] ipv6: fix leaking uninitialized port number of offender sockaddr
From: Hannes Frederic Sowa @ 2013-11-23 6:22 UTC (permalink / raw)
To: netdev
Offenders don't have port numbers, so set it to 0.
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
net/ipv6/datagram.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index a454b0f..6b1f73b 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -377,6 +377,7 @@ int ipv6_recv_error(struct sock *sk, struct msghdr *msg, int len)
if (serr->ee.ee_origin != SO_EE_ORIGIN_LOCAL) {
sin->sin6_family = AF_INET6;
sin->sin6_flowinfo = 0;
+ sin->sin6_port = 0;
if (skb->protocol == htons(ETH_P_IPV6)) {
sin->sin6_addr = ipv6_hdr(skb)->saddr;
if (np->rxopt.all)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: sched: tbf: fix an oops when a GSO packet cannot be enqueued
From: Yang Yingliang @ 2013-11-23 8:08 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev, ben, jpirko, jhs
In-Reply-To: <1385131801.10637.84.camel@edumazet-glaptop2.roam.corp.google.com>
On 2013/11/22 22:50, Eric Dumazet wrote:
> On Fri, 2013-11-22 at 17:27 +0800, Yang Yingliang wrote:
>> It seems commit e43ac79a4b("sch_tbf: segment too big GSO packets")
>> introduce this oops.
>>
>> When GSO mode is on, use the following command:
>> tc qdisc add dev eth0 root handle 1: tbf latency 50ms burst 1KB rate 50mbit mtu 1KB
>> iperf -c host -t 30
>
> I think your patch is not needed, the stack trace point to a different
> spot.
>
> The issue should be fixed by :
>
> http://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=9d8506cc2d7ea1f911c72c100193a3677f6668c3
>
> Could you check again ?
>
> Thanks
I've applied this patch, but it's oops too.
I think it's not the same problem. In this
problem, when the segs is not enqueued, we do
nothing to the segs.I think it causes the
problem. If I am wrong, please point out.
Thanks!
Here is the call trace after applying the above patch:
BTW, the call trace is not always same no matter the
above patch is applied.
[ 355.298416] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
[ 355.391292] IP: [<ffffffff81377f99>] __skb_recv_datagram+0x259/0x500
[ 355.466628] PGD 20312fc067 PUD 2030ed9067 PMD 0
[ 355.521336] Oops: 0002 [#1] SMP
[ 355.559528] Modules linked in: sch_tbf(O) tun ip6table_filter ip6_tables iptable_filter ip_tables x_tables edd af_packet bridge stp llc microcode fuse loop dm_mod igb dca i2c_algo_bit i2c_i801 ptp serio_raw sg ehci_pci ipv6 kvm_intel i2c_core kvm iTCO_wdt iTCO_vendor_support lpc_ich mfd_core hid_generic mptctl pps_core pcspkr button ext3 jbd mbcache usbhid hid uhci_hcd ehci_hcd usbcore sd_mod usb_common crc_t10dif crct10dif_common processor thermal_sys hwmon scsi_dh_hp_sw scsi_dh_emc scsi_dh_alua scsi_dh_rdac scsi_dh ata_generic ata_piix libata mptsas mptscsih mptbase scsi_transport_sas scsi_mod
[ 356.186224] CPU: 9 PID: 10073 Comm: acc-snf Tainted: G O 3.12.0-rc6-0.7-default+ #8
[ 356.287345] Hardware name: Huawei Technologies Co., Ltd. Tecal XH620 /BC21THSA , BIOS TTSAV020 12/02/2011
[ 356.424571] task: ffff881fbd4e0380 ti: ffff881fbf028000 task.ti: ffff881fbf028000
[ 356.513306] RIP: 0010:[<ffffffff81377f99>] [<ffffffff81377f99>] __skb_recv_datagram+0x259/0x500
[ 356.617524] RSP: 0018:ffff881fbf029b68 EFLAGS: 00210046
[ 356.680466] RAX: 0000000000000000 RBX: ffff881fbd0e1b70 RCX: 0000000000200296
[ 356.765074] RDX: 0000000000000000 RSI: 0000000000200296 RDI: ffff881fbbc6f0a4
[ 356.849683] RBP: ffff881fbf029c48 R08: ffff881fbf029cb4 R09: ffff88203eb24840
[ 356.934291] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 357.018900] R13: ffff881fbbc6f000 R14: ffff881fbbc6f090 R15: ffff881fbf029c64
[ 357.103508] FS: 0000000000000000(0000) GS:ffff88203f320000(0063) knlGS:00000000f7320b70
[ 357.199465] CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
[ 357.267566] CR2: 0000000000000008 CR3: 0000001fbb973000 CR4: 00000000000027e0
[ 357.352175] Stack:
[ 357.375911] 0000000000200286 ffff881fbf029c00 ffff881fbf028010 ffff881fbd4e0380
[ 357.463640] ffff881fbf028010 ffff881fbbc6f1b8 ffff881fbf029cb4 ffff881fbf029c60
[ 357.551369] ffff881f80000020 7fffffffffffffff ffff881fbbc6f0a4 ffff881fbbc6f0cc
[ 357.639099] Call Trace:
[ 357.667998] [<ffffffff81371afa>] ? skb_release_data+0xaa/0xe0
[ 357.737135] [<ffffffff81378272>] skb_recv_datagram+0x32/0x40
[ 357.805237] [<ffffffffa08d7096>] packet_recvmsg+0x66/0x360 [af_packet]
[ 357.883662] [<ffffffff81369cdc>] sock_recvmsg+0xac/0xe0
[ 357.946613] [<ffffffff8117f80d>] ? compat_core_sys_select+0x24d/0x260
[ 358.024001] [<ffffffff81369dce>] SyS_recvfrom+0xbe/0x120
[ 358.087976] [<ffffffff8136b898>] ? compat_sock_ioctl+0x88/0xb0
[ 358.158145] [<ffffffff811810ec>] ? compat_sys_ioctl+0x7c/0x1380
[ 358.229344] [<ffffffff81096d93>] ? ktime_get_ts+0x53/0x100
[ 358.295387] [<ffffffff8139e620>] compat_sys_socketcall+0x150/0x240
[ 358.369685] [<ffffffff8143f595>] sysenter_dispatch+0x7/0x1a
[ 358.436756] Code: 00 48 89 8d 68 ff ff ff e9 38 fe ff ff 41 83 ad a0 00 00 00 01 48 8b 13 48 8b 43 08 48 c7 03 00 00 00 00 48 c7 43 08 00 00 00 00 <48> 89 42 08 48 89 10 48 8b bd 70 ff ff ff e8 04 f9 0b 00 48 8b
[ 358.661068] RIP [<ffffffff81377f99>] __skb_recv_datagram+0x259/0x500
------------------------------cut here-------------------------------------
Regards,
Yang
>
>
>
>
^ permalink raw reply
* Re: [PATCH] net: sctp: set chunk->tsn_gap_acked at the end of cycle
From: Chang @ 2013-11-23 8:22 UTC (permalink / raw)
To: Vlad Yasevich, nhorman, davem; +Cc: linux-sctp, netdev, linux-kernel
In-Reply-To: <528FDF52.4030500@gmail.com>
On 11/22/2013 11:48 PM, Vlad Yasevich wrote:
> On 11/22/2013 02:24 PM, Chang wrote:
>> On 11/22/2013 03:27 PM, Vlad Yasevich wrote:
>>> On 11/22/2013 02:49 AM, Chang Xiangzhong wrote:
>>>> tsn_gap_acked is an important state flag in chunk, which indicates if
>>>> the
>>>> chunk has been acked in gap reports before.
>>> Actually, this bit indicates simply that the chunk has been acked. It
>>> doesn't state whether it's been acked in a gap report or via
>>> cumulative tsn.
>> Thanks for pointing this out. Sorry for not having made that clear.
>>>> SFR-CACC algorithm depends on this
>>>> variable. So set this at the end of each iteration, otherwise the
>>>> SFR-CACC
>>>> algorithm would never be toggled.
>>>>
>>>> Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com>
>>>> ---
>>>> net/sctp/outqueue.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>>>> index 1b494fa..bff828c 100644
>>>> --- a/net/sctp/outqueue.c
>>>> +++ b/net/sctp/outqueue.c
>>>> @@ -1396,7 +1396,6 @@ static void sctp_check_transmitted(struct
>>>> sctp_outq *q,
>>>> * while DATA was outstanding).
>>>> */
>>>> if (!tchunk->tsn_gap_acked) {
>>>> - tchunk->tsn_gap_acked = 1;
>>>> if (TSN_lt(*highest_new_tsn_in_sack, tsn))
>>>> *highest_new_tsn_in_sack = tsn;
>>>> bytes_acked += sctp_data_size(tchunk);
>>>> @@ -1460,6 +1459,8 @@ static void sctp_check_transmitted(struct
>>>> sctp_outq *q,
>>>> */
>>>> list_add_tail(lchunk, &tlist);
>>>> }
>>>> +
>>>> + tchunk->tsn_gap_acked = 1;
>>> The current code will set the state if it hasn't been set yet. Why is
>>> this needed?
>> Because in line 1420 ~ 1440 The SFR-CACC algorithms use tsn_gap_acked.
>> That "if block" would never be triggered because the varaiable's been
>> set. That's why I move the "state changing sentence" to the end of the
>> iteration.
>>> Now there is an issue with tracking highest_new_tsn_in_sack. The spec
>>> is a little vague on this, but the highest_new_tsn_in_sack only supposed
>>> to track tsns that have not been resent. If a tsn has been reneged, and
>>> then sent again, it is not considered 'new' thus should not count
>>> toward highest_new_tsn_in_sack. We currently do not track this right.
>> I'll try to figure this out.
> So the solution that's been proposed before is to move the CACC block up
> and merge it with the !tsn_gap_acked block. This requires additional
> change though to only mark highest_new_tsn if the chunk has not been
> retransmitted (which you've added in a prior patch).
>
> -vlad
Yeah, I've mixed that up with a prior patch. How was the previous patch?
Was it approved?
-chang
>>> -vlad
>>>
>>> -vlad
>>>
>>>> } else {
>>>> if (tchunk->tsn_gap_acked) {
>>>> pr_debug("%s: receiver reneged on data TSN:0x%x\n",
>>>>
^ permalink raw reply
* Darlehen bieten jetzt bewerben.
From: SUN EAST FEDERAL CREDIT UNION @ 2013-11-23 8:35 UTC (permalink / raw)
SUN EAST FEDERAL CREDIT UNION® geben wir Kredite zu einem niedrigen Zinssatz von 3%. Eine seriöse, legitim und akkreditierte Geld Lending Company. Wir leihen Geld aus, um Menschen in Not der finanziellen Unterstützung. Haben Sie eine schlechte Kredit haben oder Sie brauchen Geld, um Rechnungen zu bezahlen? Wir wollen dieses Medium nutzen, um Ihnen mitzuteilen, dass wir zuverlässige Empfänger Hilfe machen, wie wir Ihnen gerne ein Darlehen anbieten zu können.
Nach der Reaktion, werden Sie ein Darlehen Antragsformular per Post zu füllen.
Keine soziale Sicherheit und keine Bonitätsprüfung, 100% garantiert.
Wir freuen uns erlaubt mir der Service für Sie sein.
Benötigte Informationen
*****************************
Ihre Namen: ...........
Adresse: .........
Telefonnummer: .........
Betrag benötigt: ......
Dauer: ...............
Monatliches Einkommen Niveau .....
Land: .........
Postleitzahl: ....
Susane Robert.
Telefon: +447035993973
Oder mailen Sie uns unter: sun.east@webadicta.org
^ permalink raw reply
* PRIVATE MESSAGE
From: OSBORNES SOLICITORS LLP @ 2013-11-23 10:52 UTC (permalink / raw)
--
This is to inform you that an inheritance was bequeathed in your
favour. Letters were posted to you to this regards, but returned
undelivered. Kindly contact me once you recieve this email for more
information.
Sincerely
Barr Mark Freedman
^ permalink raw reply
* Re: [PATCH] net: sctp: set chunk->tsn_gap_acked at the end of cycle
From: Chang @ 2013-11-23 11:14 UTC (permalink / raw)
To: Vlad Yasevich, nhorman, davem; +Cc: linux-sctp, netdev, linux-kernel
In-Reply-To: <528FDF52.4030500@gmail.com>
Hi,
Could you please why a **reneged** newly acked TSN doesn't qualify the
highest_new_tsn? What's the wrongs of doing that?
I've been thinking a few scenarios, but I couldn't figure out what's
wrong with that.
^ permalink raw reply
* [PATCH] genetlink: Fix uninitialized variable in genl_validate_assign_mc_groups()
From: Geert Uytterhoeven @ 2013-11-23 12:01 UTC (permalink / raw)
To: David S. Miller, Johannes Berg; +Cc: netdev, linux-kernel, Geert Uytterhoeven
net/netlink/genetlink.c: In function ‘genl_validate_assign_mc_groups’:
net/netlink/genetlink.c:217: warning: ‘err’ may be used uninitialized in this
function
Commit 2a94fe48f32ccf7321450a2cc07f2b724a444e5b ("genetlink: make multicast
groups const, prevent abuse") split genl_register_mc_group() in multiple
functions, but dropped the initialization of err.
Initialize err to zero to fix this.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Question: Is the for_each_net_rcu() loop in genl_validate_assign_mc_groups()
guaranteed to loop at least once?
If yes, this is a false positive.
net/netlink/genetlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 4518a57aa5fe..803206e82450 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -214,7 +214,7 @@ static int genl_validate_assign_mc_groups(struct genl_family *family)
{
int first_id;
int n_groups = family->n_mcgrps;
- int err, i;
+ int err = 0, i;
bool groups_allocated = false;
if (!n_groups)
--
1.7.9.5
^ permalink raw reply related
* Re: Fw: [Bug 65261] New: Packet loss or excessive packet delay repeatedly for some seconds
From: Arno Wagner @ 2013-11-23 12:11 UTC (permalink / raw)
To: netdev, stephen
In-Reply-To: <20131123043359.GC15822@order.stressinduktion.org>
On Sat, Nov 23, 2013 at 05:33:59 CET, Hannes Frederic Sowa wrote:
> On Wed, Nov 20, 2013 at 12:23:16PM -0800, Stephen Hemminger wrote:
> >
> >
> > Begin forwarded message:
> >
> > Date: Wed, 20 Nov 2013 05:57:59 -0800
> > From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
> > To: "stephen@networkplumber.org" <stephen@networkplumber.org>
> > Subject: [Bug 65261] New: Packet loss or excessive packet delay repeatedly for some seconds
> >
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=65261
> >
> > Bug ID: 65261
> > Summary: Packet loss or excessive packet delay repeatedly for
> > some seconds
> > Product: Networking
> > Version: 2.5
> > Kernel Version: 3.10.19
> > Hardware: x86-64
> > OS: Linux
> > Tree: Mainline
> > Status: NEW
> > Severity: high
> > Priority: P1
> > Component: IPV4
> > Assignee: shemminger@linux-foundation.org
> > Reporter: arno@wagner.name
> > Regression: No
> >
> > Created attachment 115261
> > --> https://bugzilla.kernel.org/attachment.cgi?id=115261&action=edit
> > Kernel config 3.10.19
> >
> > I recently upgraded my development-server/firewall/NAT-box from 3.10.17 to
> > 3.10.19. Since then I noticed increased DNS lookup failures on a connected
> > Windows box and occasional slow updates on putty-SSH logins when scrolling in
> > an editor (joe, takes something like an estimated 100-300ms for screen
> > updates). These update delays are repeatable for something like 10-20 seconds
> > or more, e.g. inserting a line and then deleting again, then vanish. The delays
> > make remote editing hard to do when they happen. The DNS lookup failures are
> > really annoying.
> >
> > I have not found a way to reliably trigger the problem.
> >
> > Going back to 3.10.17 fixed the issue as far as I can tell. (Several hours
> > editing source code without it showing up.) The only change between the two
> > configurations was that I added the scsi CDROM driver (which should not be able
> > to cause this?).
> >
> > The network connection has an iptables "all pass" on the server side for the
> > affected connection. The client side is a Win7 machine. The network link is GbE
> > with 2 Switches in there. No packet loss on ping/ping -f. Network hardware is
> > Intel 82574L Gigabit card on both sides.
>
> Could you try dropwatch while the packet drops are happening?
> Often it is helpful to check ip monitor all if events happen during bursty
> losses.
Sorry, but dropwatch is not in Debian and compiling it from sources
seem to require something called "rpmbuild". Hence I cannot use
it or at least would have to write my own Makefile for it.
Arno
--
Arno Wagner, Dr. sc. techn., Dipl. Inform., Email: arno@wagner.name
GnuPG: ID: CB5D9718 FP: 12D6 C03B 1B30 33BB 13CF B774 E35C 5FA1 CB5D 9718
----
There are two ways of constructing a software design: One way is to make it
so simple that there are obviously no deficiencies, and the other way is to
make it so complicated that there are no obvious deficiencies. The first
method is far more difficult. --Tony Hoare
^ permalink raw reply
* Re: [lm-sensors] [PATCH 4/5] igb: Start temperature sensor attribute index with 1
From: Jean Delvare @ 2013-11-23 12:58 UTC (permalink / raw)
To: Guenter Roeck
Cc: netdev, Nithin Nayak Sujir, e1000-devel, Don Skidmore,
Bruce Allan, Jesse Brandeburg, lm-sensors, Greg Rose,
Jeff Kirsher, Michael Chan, Carolyn Wyborny, David S. Miller
In-Reply-To: <1385186881-7931-5-git-send-email-linux@roeck-us.net>
On Fri, 22 Nov 2013 22:08:00 -0800, Guenter Roeck wrote:
> Per hwmon ABI, temperature sensor attribute index starts with 1, not 0.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/net/ethernet/intel/igb/igb_hwmon.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_hwmon.c b/drivers/net/ethernet/intel/igb/igb_hwmon.c
> index 2e7ef2d..e0af5bc 100644
> --- a/drivers/net/ethernet/intel/igb/igb_hwmon.c
> +++ b/drivers/net/ethernet/intel/igb/igb_hwmon.c
> @@ -124,22 +124,22 @@ static int igb_add_hwmon_attr(struct igb_adapter *adapter,
> case IGB_HWMON_TYPE_LOC:
> igb_attr->dev_attr.show = igb_hwmon_show_location;
> snprintf(igb_attr->name, sizeof(igb_attr->name),
> - "temp%u_label", offset);
> + "temp%u_label", offset + 1);
> break;
> case IGB_HWMON_TYPE_TEMP:
> igb_attr->dev_attr.show = igb_hwmon_show_temp;
> snprintf(igb_attr->name, sizeof(igb_attr->name),
> - "temp%u_input", offset);
> + "temp%u_input", offset + 1);
> break;
> case IGB_HWMON_TYPE_CAUTION:
> igb_attr->dev_attr.show = igb_hwmon_show_cautionthresh;
> snprintf(igb_attr->name, sizeof(igb_attr->name),
> - "temp%u_max", offset);
> + "temp%u_max", offset + 1);
> break;
> case IGB_HWMON_TYPE_MAX:
> igb_attr->dev_attr.show = igb_hwmon_show_maxopthresh;
> snprintf(igb_attr->name, sizeof(igb_attr->name),
> - "temp%u_crit", offset);
> + "temp%u_crit", offset + 1);
> break;
> default:
> rc = -EPERM;
Reviewed-by: Jean Delvare <khali@linux-fr.org>
--
Jean Delvare
^ permalink raw reply
* Re: [lm-sensors] [PATCH 5/5] ixgbe: Start temperature sensor attribute index with 1
From: Jean Delvare @ 2013-11-23 13:01 UTC (permalink / raw)
To: Guenter Roeck
Cc: netdev, Nithin Nayak Sujir, e1000-devel, Don Skidmore,
Bruce Allan, Jesse Brandeburg, lm-sensors, Greg Rose,
Jeff Kirsher, Michael Chan, Carolyn Wyborny, David S. Miller
In-Reply-To: <1385186881-7931-6-git-send-email-linux@roeck-us.net>
On Fri, 22 Nov 2013 22:08:01 -0800, Guenter Roeck wrote:
> Per hwmon ABI, temperature sensor attribute index starts with 1, not 0.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c
> index 3081974..e74ae36 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c
> @@ -118,22 +118,22 @@ static int ixgbe_add_hwmon_attr(struct ixgbe_adapter *adapter,
> case IXGBE_HWMON_TYPE_LOC:
> ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_location;
> snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name),
> - "temp%u_label", offset);
> + "temp%u_label", offset + 1);
> break;
> case IXGBE_HWMON_TYPE_TEMP:
> ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_temp;
> snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name),
> - "temp%u_input", offset);
> + "temp%u_input", offset + 1);
> break;
> case IXGBE_HWMON_TYPE_CAUTION:
> ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_cautionthresh;
> snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name),
> - "temp%u_max", offset);
> + "temp%u_max", offset + 1);
> break;
> case IXGBE_HWMON_TYPE_MAX:
> ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_maxopthresh;
> snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name),
> - "temp%u_crit", offset);
> + "temp%u_crit", offset + 1);
> break;
> default:
> rc = -EPERM;
Reviewed-by: Jean Delvare <khali@linux-fr.org>
--
Jean Delvare
^ permalink raw reply
* Re: [lm-sensors] [PATCH 1/5] tg3: Convert to use hwmon_device_register_with_groups
From: Jean Delvare @ 2013-11-23 14:32 UTC (permalink / raw)
To: Guenter Roeck
Cc: netdev, Nithin Nayak Sujir, e1000-devel, Don Skidmore,
Bruce Allan, Jesse Brandeburg, lm-sensors, Greg Rose,
Jeff Kirsher, Michael Chan, Carolyn Wyborny, David S. Miller
In-Reply-To: <1385186881-7931-2-git-send-email-linux@roeck-us.net>
Hi Guenter,
On Fri, 22 Nov 2013 22:07:57 -0800, Guenter Roeck wrote:
> Use new hwmon API to simplify code, provide missing mandatory 'name'
> sysfs attribute, and attach hwmon attributes to hwmon device instead
> of pci device.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/net/ethernet/broadcom/tg3.c | 25 ++++++-------------------
> 1 file changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index a9e0684..369b736 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -10629,10 +10629,8 @@ static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir)
> static ssize_t tg3_show_temp(struct device *dev,
> struct device_attribute *devattr, char *buf)
> {
> - struct pci_dev *pdev = to_pci_dev(dev);
> - struct net_device *netdev = pci_get_drvdata(pdev);
> - struct tg3 *tp = netdev_priv(netdev);
> struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
> + struct tg3 *tp = dev_get_drvdata(dev);
> u32 temperature;
>
> spin_lock_bh(&tp->lock);
> @@ -10650,29 +10648,25 @@ static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, tg3_show_temp, NULL,
> static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, tg3_show_temp, NULL,
> TG3_TEMP_MAX_OFFSET);
>
> -static struct attribute *tg3_attributes[] = {
> +static struct attribute *tg3_attrs[] = {
> &sensor_dev_attr_temp1_input.dev_attr.attr,
> &sensor_dev_attr_temp1_crit.dev_attr.attr,
> &sensor_dev_attr_temp1_max.dev_attr.attr,
> NULL
> };
> -
> -static const struct attribute_group tg3_group = {
> - .attrs = tg3_attributes,
> -};
> +ATTRIBUTE_GROUPS(tg3);
>
> static void tg3_hwmon_close(struct tg3 *tp)
> {
> if (tp->hwmon_dev) {
> hwmon_device_unregister(tp->hwmon_dev);
> tp->hwmon_dev = NULL;
> - sysfs_remove_group(&tp->pdev->dev.kobj, &tg3_group);
> }
> }
>
> static void tg3_hwmon_open(struct tg3 *tp)
> {
> - int i, err;
> + int i;
> u32 size = 0;
> struct pci_dev *pdev = tp->pdev;
> struct tg3_ocir ocirs[TG3_SD_NUM_RECS];
> @@ -10690,18 +10684,11 @@ static void tg3_hwmon_open(struct tg3 *tp)
> if (!size)
> return;
>
> - /* Register hwmon sysfs hooks */
> - err = sysfs_create_group(&pdev->dev.kobj, &tg3_group);
> - if (err) {
> - dev_err(&pdev->dev, "Cannot create sysfs group, aborting\n");
> - return;
> - }
> -
> - tp->hwmon_dev = hwmon_device_register(&pdev->dev);
> + tp->hwmon_dev = hwmon_device_register_with_groups(&pdev->dev, "tg3",
> + tp, tg3_groups);
> if (IS_ERR(tp->hwmon_dev)) {
> tp->hwmon_dev = NULL;
> dev_err(&pdev->dev, "Cannot register hwmon device, aborting\n");
> - sysfs_remove_group(&pdev->dev.kobj, &tg3_group);
> }
> }
>
I can't test this, but the changes look good to me.
Reviewed-by: Jean Delvare <khali@linux-fr.org>
--
Jean Delvare
^ permalink raw reply
* Re: Fw: [Bug 65261] New: Packet loss or excessive packet delay repeatedly for some seconds
From: Ben Hutchings @ 2013-11-23 16:45 UTC (permalink / raw)
To: Arno Wagner; +Cc: netdev, stephen
In-Reply-To: <20131123121140.GB28775@tansi.org>
[-- Attachment #1: Type: text/plain, Size: 733 bytes --]
On Sat, 2013-11-23 at 13:11 +0100, Arno Wagner wrote:
> On Sat, Nov 23, 2013 at 05:33:59 CET, Hannes Frederic Sowa wrote:
[...]
> > Could you try dropwatch while the packet drops are happening?
> > Often it is helpful to check ip monitor all if events happen during bursty
> > losses.
>
> Sorry, but dropwatch is not in Debian and compiling it from sources
> seem to require something called "rpmbuild". Hence I cannot use
> it or at least would have to write my own Makefile for it.
You could use 'perf script net_dropmonitor' instead.
Ben.
--
Ben Hutchings
The generation of random numbers is too important to be left to chance.
- Robert Coveyou
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH 0/5] net: hwmon fixes
From: Ben Hutchings @ 2013-11-23 16:48 UTC (permalink / raw)
To: Guenter Roeck
Cc: netdev, David S. Miller, Jeff Kirsher, Jesse Brandeburg,
Bruce Allan, Carolyn Wyborny, Don Skidmore, Greg Rose,
Nithin Nayak Sujir, Michael Chan, e1000-devel, lm-sensors
In-Reply-To: <1385186881-7931-1-git-send-email-linux@roeck-us.net>
On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote:
> The hwmon subsystem is used by various network drivers to report temperature
> sensor and other information. Unfortunately, its use is often not correct.
> Typical errors are that the mandatory name sysfs attribute is not created,
> that the temperature sensor index starts with 0 instead of 1, and/or that
> sysfs attributes are created after the hwmon device was created.
As it happens, I was just looking at what we do in sfc
(drivers/net/ethernet/sfc/mcdi_mon.c) and wondering why I made it create
the hwmon device before the attributes. I think I avoided the other
bugs though.
Ben.
> The following sequence of patches fixes most of the problems.
>
> The igb patches have been tested with real hardware; the others are compile
> tested only.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [Doubt on Implict BUGs caused LRO] doubt about lro status in v3.12 of torvalds/linux.git
From: Yanfei Wang @ 2013-11-23 16:59 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <CAJULongu0OA0wsvnzoJQwSDR1OsE5MYz3Skn7_H0ti-azSTPTg@mail.gmail.com>
Hi，Eric&all,
I figure out I must have encountered some issues from git db usage,
even though I have not completely understood what happened on git, and
I will try to figure out what happened in reality.
so ignore all doubts in this email, please.
I am so sorry !
--
Yanfei Wang
On Sat, Nov 23, 2013 at 11:16 AM, Yanfei Wang <backyes@mail.ustc.edu.cn> wrote:
> Hi Eric,
>
> Thanks for quick reply. :-)
>
> Also, I am confused that in v3.12 why inet_lro.c file is not found in
> /net/ipv4/ directory, while Makefile has "obj-$(CONFIG_INET_LRO) +=
> inet_lro.o" and LRO heads file exsits.
>
> If I am wrong, pls correct me, :-)
>
> See the steps 3 for details,(I have repeated it for several times)
> ----
> 3. cd net/ipv4,
> [backyes@f14 ipv4]$ cat Makefile | grep lro
> obj-$(CONFIG_INET_LRO) += inet_lro.o
> [backyes@f14 ipv4]$ ls inet_*
> inet_connection_sock.c inet_diag.c inet_fragment.c
> inet_hashtables.c inet_timewait_sock.c
> ---
>
> Best regards
>
> Yanfei Wang
>
>
>
>
> On Sat, Nov 23, 2013 at 12:32 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Fri, 2013-11-22 at 23:00 +0800, Yanfei Wang wrote:
>>> Hi,
>>>
>>> Compared to the earlier kernel version, I found that Large receive
>>> offload(LRO) is obsolete in latest kernel, and that is verified from
>>> other materials about Generic receive offload(GRO).
>>>
>>> However, I also found some LRO header files and lro-related
>>> functions referred by some drivers, and the lro-related function
>>> definition is already removed at the same time!
>>>
>>> TEST steps:
>>> 1. my git local db config as follow, url =
>>> http://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>> 2. git checkout v3.12 -b 3.12
>>> 3. cd net/ipv4,
>>> [backyes@f14 ipv4]$ cat Makefile | grep lro
>>> obj-$(CONFIG_INET_LRO) += inet_lro.o
>>> [backyes@f14 ipv4]$ ls inet_*
>>> inet_connection_sock.c inet_diag.c inet_fragment.c
>>> inet_hashtables.c inet_timewait_sock.c
>>> [backyes@f14 ipv4]$
>>> Here, Makefile has inet_lro.o config, while the inet_lro.c is removed.
>>> 4. lro functions referred by some drivers, as follow,
>>> [backyes@f14 linux]$ grep "inet_lro" * -r
>>> drivers/net/ethernet/pasemi/pasemi_mac.c:#include <linux/inet_lro.h>
>>> drivers/net/ethernet/pasemi/pasemi_mac_ethtool.c:#include <linux/inet_lro.h>
>>> drivers/infiniband/hw/nes/nes_hw.c:#include <linux/inet_lro.h>
>>> drivers/infiniband/hw/nes/nes_hw.h:#include <linux/inet_lro.h>
>>> include/linux/inet_lro.h: * linux/include/linux/inet_lro.h
>>> net/ipv4/Makefile:obj-$(CONFIG_INET_LRO) += inet_lro.o
>>>
>>> so, I understand that the removal of LRO is not clean, which will
>>> cause compiler error for some kernel config.
>>>
>>
>> I think you are slightly confused.
>>
>> LRO is not yet removed.
>>
>> Feel free to contribute ;)
>>
>> As long as at least one driver still uses LRO, we can not remove LRO.
>>
>> Converting a driver means testing the new driver, and owning the
>> hardware.
>>
>>
>>
^ permalink raw reply
* Re: [PATCH 0/5] net: hwmon fixes
From: Guenter Roeck @ 2013-11-23 17:07 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev, David S. Miller, Jeff Kirsher, Jesse Brandeburg,
Bruce Allan, Carolyn Wyborny, Don Skidmore, Greg Rose,
Nithin Nayak Sujir, Michael Chan, e1000-devel, lm-sensors
In-Reply-To: <1385225290.20467.73.camel@deadeye.wl.decadent.org.uk>
On 11/23/2013 08:48 AM, Ben Hutchings wrote:
> On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote:
>> The hwmon subsystem is used by various network drivers to report temperature
>> sensor and other information. Unfortunately, its use is often not correct.
>> Typical errors are that the mandatory name sysfs attribute is not created,
>> that the temperature sensor index starts with 0 instead of 1, and/or that
>> sysfs attributes are created after the hwmon device was created.
>
> As it happens, I was just looking at what we do in sfc
> (drivers/net/ethernet/sfc/mcdi_mon.c) and wondering why I made it create
> the hwmon device before the attributes. I think I avoided the other
> bugs though.
>
Hi Ben,
Yes, I know about that one. It concluded that it would be too invasive
and risky to try to fix it without access to hardware to test the results.
That is why I said "fixes _most_ of the problems".
As for why the attributes are created after registration, it was most likely
because there was no API available to attach the sysfs attributes to
the hwmon device in a clean way. The new APIs fix that.
Thanks,
Guenter
> Ben.
>
>> The following sequence of patches fixes most of the problems.
>>
>> The igb patches have been tested with real hardware; the others are compile
>> tested only.
>
^ permalink raw reply
* Re: [PATCH] net: sched: tbf: fix an oops when a GSO packet cannot be enqueued
From: Eric Dumazet @ 2013-11-23 17:56 UTC (permalink / raw)
To: Yang Yingliang; +Cc: davem, netdev, ben, jpirko, jhs
In-Reply-To: <5290628F.6080604@huawei.com>
On Sat, 2013-11-23 at 16:08 +0800, Yang Yingliang wrote:
>
> I've applied this patch, but it's oops too.
> I think it's not the same problem. In this
> problem, when the segs is not enqueued, we do
> nothing to the segs.I think it causes the
> problem. If I am wrong, please point out.
> Thanks!
>
> Here is the call trace after applying the above patch:
> BTW, the call trace is not always same no matter the
> above patch is applied.
>
Oh right. Your patch on the right way, but changelog very confusing.
BTW, setting a mtu of 1KB on TBF never worked if your ethernet device
has MTU of 1500.
I think that we should emit a warning if TBF mtu is too low.
Ideally we should not even call skb_gso_segment() at all.
I'll post an updated version of the patch with a more detailed changelog
and explanations.
Thanks !
^ permalink raw reply
* Re: Bug - regression - Via velocity interface coming up freezes kernel
From: Michele Baldessari @ 2013-11-23 18:28 UTC (permalink / raw)
To: Francois Romieu; +Cc: Dirk Kraft, netdev, Julia Lawall
In-Reply-To: <20131115000113.GA22051@electric-eye.fr.zoreil.com>
Hi Francois,
On Fri, 15 Nov 2013 01:01:13 +0100
Francois Romieu <romieu@fr.zoreil.com> wrote:
> Francois Romieu <romieu@fr.zoreil.com> :
> > Michele Baldessari <michele@acksyn.org> :
> > [...]
> > > any chance you can submit this officially ?
> >
> > Yes.
> >
> > > Or does this patch need some more work ?
> >
> > I'll write some narrative for the commit message.
>
> I would actually rather send the patch below.
>
> The previous patch isn't safe against mtu changes. Please give the
> patch below some testing. Test case: receive lots of packets while
> changing MTU. Loop. The former patch may crash. The later shouldn't.
testing was performed by Alex A. Schmidt on this second patch variant
with MTU changes and no crashes were observed. Is the patch good
to go now?
thanks,
Michele
^ permalink raw reply
* Re: [PATCH net v3 1/2] net: sched: tbf: fix calculation of max_size
From: Eric Dumazet @ 2013-11-23 19:06 UTC (permalink / raw)
To: Yang Yingliang; +Cc: davem, netdev, brouer, jpirko, jbrouer
In-Reply-To: <1384845939-8424-2-git-send-email-yangyingliang@huawei.com>
On Tue, 2013-11-19 at 15:25 +0800, Yang Yingliang wrote:
> commit b757c9336d63f94c6b57532(tbf: improved accuracy at high rates)
> introduce a regression.
>
> With the follow command:
> tc qdisc add dev eth1 root handle 1: tbf latency 50ms burst 10KB rate 30gbit mtu 64k
>
> Without this patch, the max_size value is 10751(bytes).
> But, in fact, the real max_size value should be smaller than 7440(bytes).
> Or a packet whose length is bigger than 7440 will cause network congestion.
> Because the packet is so big that can't get enough tokens. Even all the tokens
> in the buffer is given to the packet.
>
> With this patch, the max_size value is 7440(bytes).
> The packets whose length is bigger than 7440(bytes) will be dropped or reshape
> in tbf_enqueue().
This changelog makes no sense.
If userland asks 'burst 10KB', then burst is 10KB, not 7440.
^ permalink raw reply
* Re: [PATCH] net: sctp: set chunk->tsn_gap_acked at the end of cycle
From: Vlad Yasevich @ 2013-11-23 19:37 UTC (permalink / raw)
To: Chang, nhorman, davem; +Cc: linux-sctp, netdev, linux-kernel
In-Reply-To: <52908E04.40200@gmail.com>
On 11/23/2013 06:14 AM, Chang wrote:
> Hi,
> Could you please why a **reneged** newly acked TSN doesn't qualify the
> highest_new_tsn? What's the wrongs of doing that?
>
> I've been thinking a few scenarios, but I couldn't figure out what's
> wrong with that.
>
The spec is a bit conflicting on this topic. Here is what it says
Section 7.2.4
Miss indications SHOULD follow the HTNA (Highest TSN Newly
Acknowledged) algorithm. For each incoming SACK, miss indications
are incremented only for missing TSNs prior to the highest TSN newly
acknowledged in the SACK. A newly acknowledged DATA chunk is one not
previously acknowledged in a SACK.
But section 6.2.1 says:
iii) If the SACK is missing a TSN that was previously acknowledged
via a Gap Ack Block (e.g., the data receiver reneged on the
data), then consider the corresponding DATA that might be
possibly missing: Count one miss indication towards Fast
Retransmit as described in Section 7.2.4, and if no
retransmit timer is running for the destination address to
which the DATA chunk was originally transmitted, then T3-rtx
is started for that destination address.
So, the question becomes does the reneged tsn update HTNA counter? It
has been acked by a previous SACK, but 6.2.1 says to treat as missing.
The more I look at this the more I think we should continue doing what
we are doing which is following section 6.2.1.
-vlad
^ permalink raw reply
* r8169 - it's dead jim
From: James Feeney @ 2013-11-23 19:46 UTC (permalink / raw)
To: Realtek and the Linux r8169 crew
Hey
Going from 3.12.0-1-ARCH to 3.12.1-1-ARCH, the r8169 module is having the "link
detect" problem again.
Initially, "ethtool -s <blah> autoneg off" gives "invalid argument".
"sudo mii-tool -R enp22s0" - Reset the MII to its default configuration - will
allow a 10Mb/s half duplex connection, with "autoneg off".
"ethtool -s <blah> autoneg off" then will work, after the "mii-tool -R <blah>".
After some time, Auto-negotiation will spontaneously go on, and a 100Mb/s full
duplex connection will come up, on a 1Gb/s interface. Subsequent unplugging and
plugging the cable gives a 1Gb/s full duplex connection.
This problem has been recurring for so many years, I hope the solution is
something obvious, perhaps a recent change to the kernel?
Thanks
James
^ 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