* Re: [PATCH 6/15] drivers/net/wireless/iwlegacy/4965-mac.c: adjust duplicate test
From: Stanislaw Gruszka @ 2013-01-21 12:07 UTC (permalink / raw)
To: Julia Lawall
Cc: kernel-janitors, John W. Linville, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1358773378-4700-7-git-send-email-Julia.Lawall@lip6.fr>
On Mon, Jan 21, 2013 at 02:02:50PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Delete successive tests to the same location. This looks like simple code
> duplication.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
>
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
> return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
> when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
> return ...; }
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
ACK
^ permalink raw reply
* Re: [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: YOSHIFUJI Hideaki @ 2013-01-21 12:16 UTC (permalink / raw)
To: stephan.gatzka; +Cc: linux1394-devel, netdev, David Miller, YOSHIFUJI Hideaki
In-Reply-To: <50FCE228.3090801@gmail.com>
Stephan Gatzka wrote:
>> [ARP and NDISC]
>> - both can be handled in more natural way.
>> -- You will not need to mangle those packets when
>> sending/receiving.
>
> This is also true if we introduce a function pointer. My last implementation does not mangle packets in the driver.
>
>> -- You do not need to inspect ARP/NDISC packet.
>> By using netevent notification mechanism, you can
>> learn peer parameters.
>
> That is worth to investigate. Will I get notifications when a ARP/NDISC will be send or only if the stack received ARP/NDISC packets? However, I can't see how this will help to get the firewire specific information into the ARP/NDISC packet. This has to be done either via the suggested function pointer or your extension of the mac address.
Netevent example: drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c:netevent_cb().
It will be notified when NCE has been updated (by ARP/NDP, in most cases).
Yes, it is for learning Uniq-ID and FIFO from peer.
To send our Uniq-ID and FIFO to peer via ARP/NDP, I am proposing to use
extended MAC. This will also help MCAP implementation, without big change
in IPv4/IPv6 or even whole net stack.
--yoshfuji
^ permalink raw reply
* [patch 0/6] s390: network patches for net-next
From: frank.blaschka @ 2013-01-21 12:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390
Hi Dave,
here are some s390 related patches for net-next
shortlog:
Stefan Raspl (6)
qeth: Fix retry logic in hardsetup
qeth: Remove unused exports
qeth: Support VEPA mode
qeth: Update Kconfig wording
qeth: Make s390dbf card entries persistent
qeth: Fix HiperSockets performance regression
Thanks,
Frank
^ permalink raw reply
* [patch 5/6] [PATCH] qeth: Make s390dbf card entries persistent
From: frank.blaschka @ 2013-01-21 12:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Stefan Raspl
In-Reply-To: <20130121123017.240686588@de.ibm.com>
[-- Attachment #1: 604-qeth-persistent-s390dbf.diff --]
[-- Type: text/plain, Size: 4872 bytes --]
From: Stefan Raspl <raspl@linux.vnet.ibm.com>
As of now, s390dbf entries for the cards are discarded as soon as the
device is removed. However, this will also bar us of all chances of
getting valuable debug information after a device has been removed.
This patch will keep the s390dbf entries around until the qeth module
is removed.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
Reviewed-by: Ursula Braun <ursula.braun@de.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 90 ++++++++++++++++++++++++++++++++------
1 file changed, 78 insertions(+), 12 deletions(-)
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -5122,13 +5122,81 @@ static const struct device_type qeth_osn
.groups = qeth_osn_attr_groups,
};
+#define DBF_NAME_LEN 20
+
+struct qeth_dbf_entry {
+ char dbf_name[DBF_NAME_LEN];
+ debug_info_t *dbf_info;
+ struct list_head dbf_list;
+};
+
+static LIST_HEAD(qeth_dbf_list);
+static DEFINE_MUTEX(qeth_dbf_list_mutex);
+
+static debug_info_t *qeth_get_dbf_entry(char *name)
+{
+ struct qeth_dbf_entry *entry;
+ debug_info_t *rc = NULL;
+
+ mutex_lock(&qeth_dbf_list_mutex);
+ list_for_each_entry(entry, &qeth_dbf_list, dbf_list) {
+ if (strcmp(entry->dbf_name, name) == 0) {
+ rc = entry->dbf_info;
+ break;
+ }
+ }
+ mutex_unlock(&qeth_dbf_list_mutex);
+ return rc;
+}
+
+static int qeth_add_dbf_entry(struct qeth_card *card, char *name)
+{
+ struct qeth_dbf_entry *new_entry;
+
+ card->debug = debug_register(name, 2, 1, 8);
+ if (!card->debug) {
+ QETH_DBF_TEXT_(SETUP, 2, "%s", "qcdbf");
+ goto err;
+ }
+ if (debug_register_view(card->debug, &debug_hex_ascii_view))
+ goto err_dbg;
+ new_entry = kzalloc(sizeof(struct qeth_dbf_entry), GFP_KERNEL);
+ if (!new_entry)
+ goto err_dbg;
+ strncpy(new_entry->dbf_name, name, DBF_NAME_LEN);
+ new_entry->dbf_info = card->debug;
+ mutex_lock(&qeth_dbf_list_mutex);
+ list_add(&new_entry->dbf_list, &qeth_dbf_list);
+ mutex_unlock(&qeth_dbf_list_mutex);
+
+ return 0;
+
+err_dbg:
+ debug_unregister(card->debug);
+err:
+ return -ENOMEM;
+}
+
+static void qeth_clear_dbf_list(void)
+{
+ struct qeth_dbf_entry *entry, *tmp;
+
+ mutex_lock(&qeth_dbf_list_mutex);
+ list_for_each_entry_safe(entry, tmp, &qeth_dbf_list, dbf_list) {
+ list_del(&entry->dbf_list);
+ debug_unregister(entry->dbf_info);
+ kfree(entry);
+ }
+ mutex_unlock(&qeth_dbf_list_mutex);
+}
+
static int qeth_core_probe_device(struct ccwgroup_device *gdev)
{
struct qeth_card *card;
struct device *dev;
int rc;
unsigned long flags;
- char dbf_name[20];
+ char dbf_name[DBF_NAME_LEN];
QETH_DBF_TEXT(SETUP, 2, "probedev");
@@ -5147,13 +5215,12 @@ static int qeth_core_probe_device(struct
snprintf(dbf_name, sizeof(dbf_name), "qeth_card_%s",
dev_name(&gdev->dev));
- card->debug = debug_register(dbf_name, 2, 1, 8);
+ card->debug = qeth_get_dbf_entry(dbf_name);
if (!card->debug) {
- QETH_DBF_TEXT_(SETUP, 2, "%s", "qcdbf");
- rc = -ENOMEM;
- goto err_card;
+ rc = qeth_add_dbf_entry(card, dbf_name);
+ if (rc)
+ goto err_card;
}
- debug_register_view(card->debug, &debug_hex_ascii_view);
card->read.ccwdev = gdev->cdev[0];
card->write.ccwdev = gdev->cdev[1];
@@ -5167,12 +5234,12 @@ static int qeth_core_probe_device(struct
rc = qeth_determine_card_type(card);
if (rc) {
QETH_DBF_TEXT_(SETUP, 2, "3err%d", rc);
- goto err_dbf;
+ goto err_card;
}
rc = qeth_setup_card(card);
if (rc) {
QETH_DBF_TEXT_(SETUP, 2, "2err%d", rc);
- goto err_dbf;
+ goto err_card;
}
if (card->info.type == QETH_CARD_TYPE_OSN)
@@ -5185,7 +5252,7 @@ static int qeth_core_probe_device(struct
case QETH_CARD_TYPE_OSM:
rc = qeth_core_load_discipline(card, QETH_DISCIPLINE_LAYER2);
if (rc)
- goto err_dbf;
+ goto err_card;
rc = card->discipline->setup(card->gdev);
if (rc)
goto err_disc;
@@ -5204,8 +5271,6 @@ static int qeth_core_probe_device(struct
err_disc:
qeth_core_free_discipline(card);
-err_dbf:
- debug_unregister(card->debug);
err_card:
qeth_core_free_card(card);
err_dev:
@@ -5225,7 +5290,6 @@ static void qeth_core_remove_device(stru
qeth_core_free_discipline(card);
}
- debug_unregister(card->debug);
write_lock_irqsave(&qeth_core_card_list.rwlock, flags);
list_del(&card->list);
write_unlock_irqrestore(&qeth_core_card_list.rwlock, flags);
@@ -5579,6 +5643,7 @@ static int __init qeth_core_init(void)
pr_info("loading core functions\n");
INIT_LIST_HEAD(&qeth_core_card_list.list);
+ INIT_LIST_HEAD(&qeth_dbf_list);
rwlock_init(&qeth_core_card_list.rwlock);
mutex_init(&qeth_mod_mutex);
@@ -5630,6 +5695,7 @@ out_err:
static void __exit qeth_core_exit(void)
{
+ qeth_clear_dbf_list();
destroy_workqueue(qeth_wq);
ccwgroup_driver_unregister(&qeth_core_ccwgroup_driver);
ccw_driver_unregister(&qeth_ccw_driver);
^ permalink raw reply
* [patch 3/6] [PATCH] qeth: Support VEPA mode
From: frank.blaschka @ 2013-01-21 12:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Stefan Raspl
In-Reply-To: <20130121123017.240686588@de.ibm.com>
[-- Attachment #1: 602-qeth-vepa-mode.diff --]
[-- Type: text/plain, Size: 15435 bytes --]
From: Stefan Raspl <raspl@linux.vnet.ibm.com>
The existing port isolation mode 'forward' will now verify that the adjacent
switch port supports the required reflective relay (RR) mode. This patch adds
the required error handling for the cases where enabling port isolation mode
'forward' can now fail.
Furthermore, once established, we never fall back from one of the port
isolation modes to a non-isolated mode without further user-interaction.
This includes cases where the isolation mode was enabled successfully, but
ceases to work e.g. due to configuration changes at the switch port.
Finally, configuring an isolation mode with the device being offline
will make onlining the device fail permanently upon errors encountered until
either errors are resolved or the isolation mode is changed by the user to a
different mode.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
Reviewed-by: Ursula Braun <ursula.braun@de.ibm.com>
---
drivers/s390/net/qeth_core.h | 5 +
drivers/s390/net/qeth_core_main.c | 135 +++++++++++++++++++++++---------------
drivers/s390/net/qeth_core_mpc.c | 1
drivers/s390/net/qeth_core_mpc.h | 5 +
drivers/s390/net/qeth_core_sys.c | 3
drivers/s390/net/qeth_l2_main.c | 16 ++--
drivers/s390/net/qeth_l3_main.c | 14 +--
7 files changed, 112 insertions(+), 67 deletions(-)
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -678,6 +678,7 @@ struct qeth_card_options {
int performance_stats;
int rx_sg_cb;
enum qeth_ipa_isolation_modes isolation;
+ enum qeth_ipa_isolation_modes prev_isolation;
int sniffer;
enum qeth_cq cq;
char hsuid[9];
@@ -789,6 +790,7 @@ struct qeth_card {
struct qeth_rx rx;
struct delayed_work buffer_reclaim_work;
int reclaim_index;
+ struct work_struct close_dev_work;
};
struct qeth_card_list_struct {
@@ -925,12 +927,13 @@ void qeth_core_get_strings(struct net_de
void qeth_core_get_drvinfo(struct net_device *, struct ethtool_drvinfo *);
void qeth_dbf_longtext(debug_info_t *id, int level, char *text, ...);
int qeth_core_ethtool_get_settings(struct net_device *, struct ethtool_cmd *);
-int qeth_set_access_ctrl_online(struct qeth_card *card);
+int qeth_set_access_ctrl_online(struct qeth_card *card, int fallback);
int qeth_hdr_chk_and_bounce(struct sk_buff *, int);
int qeth_configure_cq(struct qeth_card *, enum qeth_cq);
int qeth_hw_trap(struct qeth_card *, enum qeth_diags_trap_action);
int qeth_query_ipassists(struct qeth_card *, enum qeth_prot_versions prot);
void qeth_trace_features(struct qeth_card *);
+void qeth_close_dev(struct qeth_card *);
/* exports for OSN */
int qeth_osn_assist(struct net_device *, void *, int);
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -68,6 +68,27 @@ static void qeth_clear_output_buffer(str
enum qeth_qdio_buffer_states newbufstate);
static int qeth_init_qdio_out_buf(struct qeth_qdio_out_q *, int);
+static struct workqueue_struct *qeth_wq;
+
+static void qeth_close_dev_handler(struct work_struct *work)
+{
+ struct qeth_card *card;
+
+ card = container_of(work, struct qeth_card, close_dev_work);
+ QETH_CARD_TEXT(card, 2, "cldevhdl");
+ rtnl_lock();
+ dev_close(card->dev);
+ rtnl_unlock();
+ ccwgroup_set_offline(card->gdev);
+}
+
+void qeth_close_dev(struct qeth_card *card)
+{
+ QETH_CARD_TEXT(card, 2, "cldevsubm");
+ queue_work(qeth_wq, &card->close_dev_work);
+}
+EXPORT_SYMBOL_GPL(qeth_close_dev);
+
static inline const char *qeth_get_cardname(struct qeth_card *card)
{
if (card->info.guestlan) {
@@ -542,11 +563,23 @@ static struct qeth_ipa_cmd *qeth_check_i
} else {
switch (cmd->hdr.command) {
case IPA_CMD_STOPLAN:
- dev_warn(&card->gdev->dev,
+ if (cmd->hdr.return_code ==
+ IPA_RC_VEPA_TO_VEB_TRANSITION) {
+ dev_err(&card->gdev->dev,
+ "Interface %s is down because the "
+ "adjacent port is no longer in "
+ "reflective relay mode\n",
+ QETH_CARD_IFNAME(card));
+ qeth_close_dev(card);
+ } else {
+ dev_warn(&card->gdev->dev,
"The link for interface %s on CHPID"
" 0x%X failed\n",
QETH_CARD_IFNAME(card),
card->info.chpid);
+ qeth_issue_ipa_msg(cmd,
+ cmd->hdr.return_code, card);
+ }
card->lan_online = 0;
if (card->dev && netif_carrier_ok(card->dev))
netif_carrier_off(card->dev);
@@ -1416,6 +1449,7 @@ static int qeth_setup_card(struct qeth_c
/* init QDIO stuff */
qeth_init_qdio_info(card);
INIT_DELAYED_WORK(&card->buffer_reclaim_work, qeth_buffer_reclaim_work);
+ INIT_WORK(&card->close_dev_work, qeth_close_dev_handler);
return 0;
}
@@ -4057,6 +4091,7 @@ static int qeth_setadpparms_set_access_c
{
struct qeth_ipa_cmd *cmd;
struct qeth_set_access_ctrl *access_ctrl_req;
+ int fallback = *(int *)reply->param;
QETH_CARD_TEXT(card, 4, "setaccb");
@@ -4066,12 +4101,14 @@ static int qeth_setadpparms_set_access_c
QETH_DBF_TEXT_(SETUP, 2, "%s", card->gdev->dev.kobj.name);
QETH_DBF_TEXT_(SETUP, 2, "rc=%d",
cmd->data.setadapterparms.hdr.return_code);
+ if (cmd->data.setadapterparms.hdr.return_code !=
+ SET_ACCESS_CTRL_RC_SUCCESS)
+ QETH_DBF_MESSAGE(3, "ERR:SET_ACCESS_CTRL(%s,%d)==%d\n",
+ card->gdev->dev.kobj.name,
+ access_ctrl_req->subcmd_code,
+ cmd->data.setadapterparms.hdr.return_code);
switch (cmd->data.setadapterparms.hdr.return_code) {
case SET_ACCESS_CTRL_RC_SUCCESS:
- case SET_ACCESS_CTRL_RC_ALREADY_NOT_ISOLATED:
- case SET_ACCESS_CTRL_RC_ALREADY_ISOLATED:
- {
- card->options.isolation = access_ctrl_req->subcmd_code;
if (card->options.isolation == ISOLATION_MODE_NONE) {
dev_info(&card->gdev->dev,
"QDIO data connection isolation is deactivated\n");
@@ -4079,72 +4116,64 @@ static int qeth_setadpparms_set_access_c
dev_info(&card->gdev->dev,
"QDIO data connection isolation is activated\n");
}
- QETH_DBF_MESSAGE(3, "OK:SET_ACCESS_CTRL(%s, %d)==%d\n",
- card->gdev->dev.kobj.name,
- access_ctrl_req->subcmd_code,
- cmd->data.setadapterparms.hdr.return_code);
break;
- }
+ case SET_ACCESS_CTRL_RC_ALREADY_NOT_ISOLATED:
+ QETH_DBF_MESSAGE(2, "%s QDIO data connection isolation already "
+ "deactivated\n", dev_name(&card->gdev->dev));
+ if (fallback)
+ card->options.isolation = card->options.prev_isolation;
+ break;
+ case SET_ACCESS_CTRL_RC_ALREADY_ISOLATED:
+ QETH_DBF_MESSAGE(2, "%s QDIO data connection isolation already"
+ " activated\n", dev_name(&card->gdev->dev));
+ if (fallback)
+ card->options.isolation = card->options.prev_isolation;
+ break;
case SET_ACCESS_CTRL_RC_NOT_SUPPORTED:
- {
- QETH_DBF_MESSAGE(3, "ERR:SET_ACCESS_CTRL(%s,%d)==%d\n",
- card->gdev->dev.kobj.name,
- access_ctrl_req->subcmd_code,
- cmd->data.setadapterparms.hdr.return_code);
dev_err(&card->gdev->dev, "Adapter does not "
"support QDIO data connection isolation\n");
-
- /* ensure isolation mode is "none" */
- card->options.isolation = ISOLATION_MODE_NONE;
break;
- }
case SET_ACCESS_CTRL_RC_NONE_SHARED_ADAPTER:
- {
- QETH_DBF_MESSAGE(3, "ERR:SET_ACCESS_MODE(%s,%d)==%d\n",
- card->gdev->dev.kobj.name,
- access_ctrl_req->subcmd_code,
- cmd->data.setadapterparms.hdr.return_code);
dev_err(&card->gdev->dev,
"Adapter is dedicated. "
"QDIO data connection isolation not supported\n");
-
- /* ensure isolation mode is "none" */
- card->options.isolation = ISOLATION_MODE_NONE;
+ if (fallback)
+ card->options.isolation = card->options.prev_isolation;
break;
- }
case SET_ACCESS_CTRL_RC_ACTIVE_CHECKSUM_OFF:
- {
- QETH_DBF_MESSAGE(3, "ERR:SET_ACCESS_MODE(%s,%d)==%d\n",
- card->gdev->dev.kobj.name,
- access_ctrl_req->subcmd_code,
- cmd->data.setadapterparms.hdr.return_code);
dev_err(&card->gdev->dev,
"TSO does not permit QDIO data connection isolation\n");
-
- /* ensure isolation mode is "none" */
- card->options.isolation = ISOLATION_MODE_NONE;
+ if (fallback)
+ card->options.isolation = card->options.prev_isolation;
+ break;
+ case SET_ACCESS_CTRL_RC_REFLREL_UNSUPPORTED:
+ dev_err(&card->gdev->dev, "The adjacent switch port does not "
+ "support reflective relay mode\n");
+ if (fallback)
+ card->options.isolation = card->options.prev_isolation;
+ break;
+ case SET_ACCESS_CTRL_RC_REFLREL_FAILED:
+ dev_err(&card->gdev->dev, "The reflective relay mode cannot be "
+ "enabled at the adjacent switch port");
+ if (fallback)
+ card->options.isolation = card->options.prev_isolation;
+ break;
+ case SET_ACCESS_CTRL_RC_REFLREL_DEACT_FAILED:
+ dev_warn(&card->gdev->dev, "Turning off reflective relay mode "
+ "at the adjacent switch failed\n");
break;
- }
default:
- {
/* this should never happen */
- QETH_DBF_MESSAGE(3, "ERR:SET_ACCESS_MODE(%s,%d)==%d"
- "==UNKNOWN\n",
- card->gdev->dev.kobj.name,
- access_ctrl_req->subcmd_code,
- cmd->data.setadapterparms.hdr.return_code);
-
- /* ensure isolation mode is "none" */
- card->options.isolation = ISOLATION_MODE_NONE;
+ if (fallback)
+ card->options.isolation = card->options.prev_isolation;
break;
}
- }
qeth_default_setadapterparms_cb(card, reply, (unsigned long) cmd);
return 0;
}
static int qeth_setadpparms_set_access_ctrl(struct qeth_card *card,
- enum qeth_ipa_isolation_modes isolation)
+ enum qeth_ipa_isolation_modes isolation, int fallback)
{
int rc;
struct qeth_cmd_buffer *iob;
@@ -4164,12 +4193,12 @@ static int qeth_setadpparms_set_access_c
access_ctrl_req->subcmd_code = isolation;
rc = qeth_send_ipa_cmd(card, iob, qeth_setadpparms_set_access_ctrl_cb,
- NULL);
+ &fallback);
QETH_DBF_TEXT_(SETUP, 2, "rc=%d", rc);
return rc;
}
-int qeth_set_access_ctrl_online(struct qeth_card *card)
+int qeth_set_access_ctrl_online(struct qeth_card *card, int fallback)
{
int rc = 0;
@@ -4179,12 +4208,13 @@ int qeth_set_access_ctrl_online(struct q
card->info.type == QETH_CARD_TYPE_OSX) &&
qeth_adp_supported(card, IPA_SETADP_SET_ACCESS_CONTROL)) {
rc = qeth_setadpparms_set_access_ctrl(card,
- card->options.isolation);
+ card->options.isolation, fallback);
if (rc) {
QETH_DBF_MESSAGE(3,
"IPA(SET_ACCESS_CTRL,%s,%d) sent failed\n",
card->gdev->dev.kobj.name,
rc);
+ rc = -EOPNOTSUPP;
}
} else if (card->options.isolation != ISOLATION_MODE_NONE) {
card->options.isolation = ISOLATION_MODE_NONE;
@@ -5552,6 +5582,8 @@ static int __init qeth_core_init(void)
rwlock_init(&qeth_core_card_list.rwlock);
mutex_init(&qeth_mod_mutex);
+ qeth_wq = create_singlethread_workqueue("qeth_wq");
+
rc = qeth_register_dbf_views();
if (rc)
goto out_err;
@@ -5598,6 +5630,7 @@ out_err:
static void __exit qeth_core_exit(void)
{
+ destroy_workqueue(qeth_wq);
ccwgroup_driver_unregister(&qeth_core_ccwgroup_driver);
ccw_driver_unregister(&qeth_ccw_driver);
kmem_cache_destroy(qeth_qdio_outbuf_cache);
--- a/drivers/s390/net/qeth_core_mpc.c
+++ b/drivers/s390/net/qeth_core_mpc.c
@@ -204,6 +204,7 @@ static struct ipa_rc_msg qeth_ipa_rc_msg
{IPA_RC_INVALID_SETRTG_INDICATOR, "Invalid SETRTG indicator"},
{IPA_RC_MC_ADDR_ALREADY_DEFINED, "Multicast address already defined"},
{IPA_RC_LAN_OFFLINE, "STRTLAN_LAN_DISABLED - LAN offline"},
+ {IPA_RC_VEPA_TO_VEB_TRANSITION, "Adj. switch disabled port mode RR"},
{IPA_RC_INVALID_IP_VERSION2, "Invalid IP version"},
{IPA_RC_ENOMEM, "Memory problem"},
{IPA_RC_FFFF, "Unknown Error"}
--- a/drivers/s390/net/qeth_core_mpc.h
+++ b/drivers/s390/net/qeth_core_mpc.h
@@ -177,6 +177,7 @@ enum qeth_ipa_return_codes {
IPA_RC_INVALID_SETRTG_INDICATOR = 0xe012,
IPA_RC_MC_ADDR_ALREADY_DEFINED = 0xe013,
IPA_RC_LAN_OFFLINE = 0xe080,
+ IPA_RC_VEPA_TO_VEB_TRANSITION = 0xe090,
IPA_RC_INVALID_IP_VERSION2 = 0xf001,
IPA_RC_ENOMEM = 0xfffe,
IPA_RC_FFFF = 0xffff
@@ -269,6 +270,9 @@ enum qeth_ipa_set_access_mode_rc {
SET_ACCESS_CTRL_RC_ALREADY_ISOLATED = 0x0010,
SET_ACCESS_CTRL_RC_NONE_SHARED_ADAPTER = 0x0014,
SET_ACCESS_CTRL_RC_ACTIVE_CHECKSUM_OFF = 0x0018,
+ SET_ACCESS_CTRL_RC_REFLREL_UNSUPPORTED = 0x0022,
+ SET_ACCESS_CTRL_RC_REFLREL_FAILED = 0x0024,
+ SET_ACCESS_CTRL_RC_REFLREL_DEACT_FAILED = 0x0028,
};
@@ -386,6 +390,7 @@ struct qeth_snmp_ureq {
/* SET_ACCESS_CONTROL: same format for request and reply */
struct qeth_set_access_ctrl {
__u32 subcmd_code;
+ __u8 reserved[8];
} __attribute__((packed));
struct qeth_query_oat {
--- a/drivers/s390/net/qeth_core_sys.c
+++ b/drivers/s390/net/qeth_core_sys.c
@@ -513,10 +513,11 @@ static ssize_t qeth_dev_isolation_store(
rc = count;
/* defer IP assist if device is offline (until discipline->set_online)*/
+ card->options.prev_isolation = card->options.isolation;
card->options.isolation = isolation;
if (card->state == CARD_STATE_SOFTSETUP ||
card->state == CARD_STATE_UP) {
- int ipa_rc = qeth_set_access_ctrl_online(card);
+ int ipa_rc = qeth_set_access_ctrl_online(card, 1);
if (ipa_rc != 0)
rc = ipa_rc;
}
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -1025,9 +1025,14 @@ static int __qeth_l2_set_online(struct c
contin:
if ((card->info.type == QETH_CARD_TYPE_OSD) ||
- (card->info.type == QETH_CARD_TYPE_OSX))
+ (card->info.type == QETH_CARD_TYPE_OSX)) {
/* configure isolation level */
- qeth_set_access_ctrl_online(card);
+ rc = qeth_set_access_ctrl_online(card, 0);
+ if (rc) {
+ rc = -ENODEV;
+ goto out_remove;
+ }
+ }
if (card->info.type != QETH_CARD_TYPE_OSN &&
card->info.type != QETH_CARD_TYPE_OSM)
@@ -1144,12 +1149,9 @@ static int qeth_l2_recover(void *ptr)
dev_info(&card->gdev->dev,
"Device successfully recovered!\n");
else {
- if (rtnl_trylock()) {
- dev_close(card->dev);
- rtnl_unlock();
- dev_warn(&card->gdev->dev, "The qeth device driver "
+ qeth_close_dev(card);
+ dev_warn(&card->gdev->dev, "The qeth device driver "
"failed to recover an error on the device\n");
- }
}
qeth_clear_thread_start_bit(card, QETH_RECOVER_THREAD);
qeth_clear_thread_running_bit(card, QETH_RECOVER_THREAD);
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1449,7 +1449,8 @@ static int qeth_l3_start_ipassists(struc
{
QETH_CARD_TEXT(card, 3, "strtipas");
- qeth_set_access_ctrl_online(card); /* go on*/
+ if (qeth_set_access_ctrl_online(card, 0))
+ return -EIO;
qeth_l3_start_ipa_arp_processing(card); /* go on*/
qeth_l3_start_ipa_ip_fragmentation(card); /* go on*/
qeth_l3_start_ipa_source_mac(card); /* go on*/
@@ -3388,8 +3389,10 @@ contin:
QETH_DBF_TEXT_(SETUP, 2, "2err%d", rc);
if (!card->options.sniffer) {
rc = qeth_l3_start_ipassists(card);
- if (rc)
+ if (rc) {
QETH_DBF_TEXT_(SETUP, 2, "3err%d", rc);
+ goto out_remove;
+ }
rc = qeth_l3_setrouting_v4(card);
if (rc)
QETH_DBF_TEXT_(SETUP, 2, "4err%d", rc);
@@ -3511,12 +3514,9 @@ static int qeth_l3_recover(void *ptr)
dev_info(&card->gdev->dev,
"Device successfully recovered!\n");
else {
- if (rtnl_trylock()) {
- dev_close(card->dev);
- rtnl_unlock();
- dev_warn(&card->gdev->dev, "The qeth device driver "
+ qeth_close_dev(card);
+ dev_warn(&card->gdev->dev, "The qeth device driver "
"failed to recover an error on the device\n");
- }
}
qeth_clear_thread_start_bit(card, QETH_RECOVER_THREAD);
qeth_clear_thread_running_bit(card, QETH_RECOVER_THREAD);
^ permalink raw reply
* [patch 4/6] [PATCH] qeth: Update Kconfig wording
From: frank.blaschka @ 2013-01-21 12:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Stefan Raspl
In-Reply-To: <20130121123017.240686588@de.ibm.com>
[-- Attachment #1: 603-qeth-kconfig-help.diff --]
[-- Type: text/plain, Size: 869 bytes --]
From: Stefan Raspl <raspl@linux.vnet.ibm.com>
Refer to virtual NICs instead of GuestLANs.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
---
drivers/s390/net/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/s390/net/Kconfig
+++ b/drivers/s390/net/Kconfig
@@ -74,8 +74,8 @@ config QETH
depends on CCW && NETDEVICES && IP_MULTICAST && QDIO
help
This driver supports the IBM System z OSA Express adapters
- in QDIO mode (all media types), HiperSockets interfaces and VM GuestLAN
- interfaces in QDIO and HIPER mode.
+ in QDIO mode (all media types), HiperSockets interfaces and z/VM
+ virtual NICs for Guest LAN and VSWITCH.
For details please refer to the documentation provided by IBM at
<http://www.ibm.com/developerworks/linux/linux390>
^ permalink raw reply
* [patch 6/6] [PATCH] qeth: Fix HiperSockets performance regression
From: frank.blaschka @ 2013-01-21 12:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Stefan Raspl
In-Reply-To: <20130121123017.240686588@de.ibm.com>
[-- Attachment #1: 605-qeth-hipersocket-performance.diff --]
[-- Type: text/plain, Size: 1120 bytes --]
From: Stefan Raspl <raspl@linux.vnet.ibm.com>
Commit 46d3ceab "tcp: TCP Small Queues" has severly degraded
performance for single connection RR workloads on HiperSockets with
MTU >=16K due to a conflict of the TCP Small Queues approach with our
buffer scan threshold which releases buffers not frequently enough yet.
This fix restores performance to the same level as before cited commit.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4700,7 +4700,7 @@ static int qeth_qdio_establish(struct qe
init_data.output_sbal_addr_array = (void **) out_sbal_ptrs;
init_data.output_sbal_state_array = card->qdio.out_bufstates;
init_data.scan_threshold =
- (card->info.type == QETH_CARD_TYPE_IQD) ? 8 : 32;
+ (card->info.type == QETH_CARD_TYPE_IQD) ? 1 : 32;
if (atomic_cmpxchg(&card->qdio.state, QETH_QDIO_ALLOCATED,
QETH_QDIO_ESTABLISHED) == QETH_QDIO_ALLOCATED) {
^ permalink raw reply
* [patch 2/6] [PATCH] qeth: Remove unused exports
From: frank.blaschka @ 2013-01-21 12:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Stefan Raspl
In-Reply-To: <20130121123017.240686588@de.ibm.com>
[-- Attachment #1: 601-qeth-exports.diff --]
[-- Type: text/plain, Size: 2253 bytes --]
From: Stefan Raspl <raspl@linux.vnet.ibm.com>
Remove exports that are not used anywhere else.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
---
drivers/s390/net/qeth_core.h | 3 ---
drivers/s390/net/qeth_core_main.c | 6 ++----
2 files changed, 2 insertions(+), 7 deletions(-)
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -909,9 +909,6 @@ struct qeth_cmd_buffer *qeth_wait_for_bu
int qeth_mdio_read(struct net_device *, int, int);
int qeth_snmp_command(struct qeth_card *, char __user *);
int qeth_query_oat_command(struct qeth_card *, char __user *);
-struct qeth_cmd_buffer *qeth_get_adapter_cmd(struct qeth_card *, __u32, __u32);
-int qeth_default_setadapterparms_cb(struct qeth_card *, struct qeth_reply *,
- unsigned long);
int qeth_send_control_data(struct qeth_card *, int, struct qeth_cmd_buffer *,
int (*reply_cb)(struct qeth_card *, struct qeth_reply*, unsigned long),
void *reply_param);
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -2868,7 +2868,7 @@ int qeth_send_startlan(struct qeth_card
}
EXPORT_SYMBOL_GPL(qeth_send_startlan);
-int qeth_default_setadapterparms_cb(struct qeth_card *card,
+static int qeth_default_setadapterparms_cb(struct qeth_card *card,
struct qeth_reply *reply, unsigned long data)
{
struct qeth_ipa_cmd *cmd;
@@ -2881,7 +2881,6 @@ int qeth_default_setadapterparms_cb(stru
cmd->data.setadapterparms.hdr.return_code;
return 0;
}
-EXPORT_SYMBOL_GPL(qeth_default_setadapterparms_cb);
static int qeth_query_setadapterparms_cb(struct qeth_card *card,
struct qeth_reply *reply, unsigned long data)
@@ -2901,7 +2900,7 @@ static int qeth_query_setadapterparms_cb
return qeth_default_setadapterparms_cb(card, reply, (unsigned long)cmd);
}
-struct qeth_cmd_buffer *qeth_get_adapter_cmd(struct qeth_card *card,
+static struct qeth_cmd_buffer *qeth_get_adapter_cmd(struct qeth_card *card,
__u32 command, __u32 cmdlen)
{
struct qeth_cmd_buffer *iob;
@@ -2917,7 +2916,6 @@ struct qeth_cmd_buffer *qeth_get_adapter
return iob;
}
-EXPORT_SYMBOL_GPL(qeth_get_adapter_cmd);
int qeth_query_setadapterparms(struct qeth_card *card)
{
^ permalink raw reply
* [patch 1/6] [PATCH] qeth: Fix retry logic in hardsetup
From: frank.blaschka @ 2013-01-21 12:30 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Stefan Raspl
In-Reply-To: <20130121123017.240686588@de.ibm.com>
[-- Attachment #1: 600-qeth-retry-logic.diff --]
[-- Type: text/plain, Size: 1188 bytes --]
From: Stefan Raspl <raspl@linux.vnet.ibm.com>
The previous code did never retry any idx setup unless retries were done
for device offline/online at the beginning of the function.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
Reviewed-by: Ursula Braun <ursula.braun@de.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4765,14 +4765,14 @@ static struct ccw_driver qeth_ccw_driver
int qeth_core_hardsetup_card(struct qeth_card *card)
{
- int retries = 0;
+ int retries = 3;
int rc;
QETH_DBF_TEXT(SETUP, 2, "hrdsetup");
atomic_set(&card->force_alloc_skb, 0);
qeth_update_from_chp_desc(card);
retry:
- if (retries)
+ if (retries < 3)
QETH_DBF_MESSAGE(2, "%s Retrying to do IDX activates.\n",
dev_name(&card->gdev->dev));
ccw_device_set_offline(CARD_DDEV(card));
@@ -4794,7 +4794,7 @@ retriable:
return rc;
} else if (rc) {
QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
- if (++retries > 3)
+ if (--retries < 0)
goto out;
else
goto retry;
^ permalink raw reply
* Re: [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: YOSHIFUJI Hideaki @ 2013-01-21 12:32 UTC (permalink / raw)
To: Stefan Richter
Cc: stephan.gatzka, Waskiewicz Jr, Peter P, linux1394-devel, netdev,
davem, YOSHIFUJI Hideaki
In-Reply-To: <20130121092806.3c741bd2@stein>
Stefan Richter wrote:
> b) RFC 2734 MCAP (Multicast Channel Allocation Protocol): currently not
> implemented; not sure if needed (I see a few big downsides to using
> per-multicast-group channels); not sure if networking core support
> would be needed for this
It is definitely needed because we need to listen on multicast group
for NDP, which might have already had its own channel.
MCAP sends "multicast group address", which is whole IP address.
In IP stack we only manages listening groups by special
"link-layer address" mapped by device specific transform function.
See net/ipv6/ndiscc:ndisc_mc_map() for IPv6.
I am proposing to extend MAC address to 16 bytes long so that we can
"map" whole IPv6 address into the "MAC".
On tx on fwnet, it can determine if the destination is multicast or not
by either by
- Checking protocol, and looking into protocol dependent multicast
range
or
- Checking "EUI-64" group bit (here, we assume all node do not set it
in its unique Id, and we map IPv6 multicast and IPv4 multicast in
special way; e.g.
ff00::/8 (which has "multicast bit" set)
0100::/96 (which is also "multicast bit" set)
--yoshfuji
^ permalink raw reply
* Re: [V2] xen-netback notify DomU to send ARP.
From: Jason Luan @ 2013-01-21 12:38 UTC (permalink / raw)
To: Ian Campbell; +Cc: netdev, xen-devel, linux-kernel
In-Reply-To: <1358770987.3279.196.camel@zakaz.uk.xensource.com>
[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]
于 2013年01月21日 20:23, Ian Campbell 写道:
> Which Linux did you test? pvops or something based on the classic-Xen
> patches?
>
> On Mon, 2013-01-21 at 11:55 +0000, Jan Beulich wrote:
>>>>> On 21.01.13 at 08:26, jianhai luan <jianhai.luan@oracle.com> wrote:
>>> +static void notify_front_arping(struct xenbus_device *dev)
>>> +{
>>> + int err;
>>> +
>>> + if (dev->state != XenbusStateConnected)
>>> + return;
>>> +
>>> + err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
>>> + if (err) {
>>> + pr_fmt("Error writing the state");
>> What's this? pr_fmt() alone makes no sense at all, and I'd be
>> pretty surprised if the compiler didn't warn about this construct.
>>
>> Further, you probably want to say "re-writing" and include the
>> error code in the message. And of course you want a \n at the
>> end.
>>
>> Finally - no need for the braces ...
>>
>>> + }
>>> +
>>> + return;
>> ... nor this "return".
> Agreed on all counts.
>
> Jason, when you resend with these fixes please CC the netdev list and
> folks listed by ./scripts/get-maintainers.pl for the patch.
>
> Ian.
Thank you for your notifying.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
Thanks,
Jason
[-- Attachment #2: 0001-xen-netback-notify-frontend-to-send-gratuitous-ARP.patch --]
[-- Type: text/x-patch, Size: 3130 bytes --]
>From a1acd31b9672d1000549e71a9467b71b1c229d7d Mon Sep 17 00:00:00 2001
From: Jason Luan <jianhai.luan@oracle.com>
Date: Fri, 28 Dec 2012 15:43:06 +0800
Subject: [PATCH] xen-netback notify frontend to send gratuitous ARP.
In the real network environment, some cause will lead
to Active-Backup mode bonding chose new actived port.
After that, the trffic, destinated to DomU by inactived
port (former actived port), will be unreachable at now.
DomU should send gratutious ARP initialtivly to find the
new corrected path.
By netback's Connected->Connected transition, frontend
will watch the change, and send gratuitous ARP.
Signed-off-by: Jason Luan <jianhai.luan@oracle.com>
---
drivers/net/xen-netback/xenbus.c | 48 ++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 410018c..844fd86 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -26,6 +26,7 @@ struct backend_info {
struct xenvif *vif;
enum xenbus_state frontend_state;
struct xenbus_watch hotplug_status_watch;
+ struct notifier_block vif_notifier;
u8 have_hotplug_status_watch:1;
};
@@ -34,11 +35,54 @@ static void connect(struct backend_info *);
static void backend_create_xenvif(struct backend_info *be);
static void unregister_hotplug_status_watch(struct backend_info *be);
+/*
+ * By Connected->Connected transition, netfront will watch the change and
+ * send gratuitous ARP.
+ */
+static void notify_front_arping(struct xenbus_device *dev)
+{
+ int err;
+
+ if (dev->state != XenbusStateConnected)
+ return;
+
+ err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+ if (err)
+ pr_warn("NetBack: Failure to notify DomU (err=%d)\n", err);
+}
+
+#define nb_to_backend(nb) container_of(nb, struct backend_info, vif_notifier)
+/**
+ * When network condition of vif change, notify the frontend.
+ */
+static int netback_netdev_event(struct notifier_block *this,
+ unsigned long event, void *ptr)
+{
+ struct net_device *event_dev = ptr;
+ struct backend_info *be = nb_to_backend(this);
+
+ pr_debug("event_dev: %s, event: %lx\n",
+ event_dev ? event_dev->name : "None", event);
+
+ if (!be->vif)
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case NETDEV_NOTIFY_PEERS:
+ /* Notify frontend to Send gratuitous ARP */
+ notify_front_arping(be->dev);
+ break;
+ }
+
+ return NOTIFY_DONE;
+}
+
static int netback_remove(struct xenbus_device *dev)
{
struct backend_info *be = dev_get_drvdata(&dev->dev);
unregister_hotplug_status_watch(be);
+ unregister_netdevice_notifier(&be->vif_notifier);
if (be->vif) {
kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
@@ -129,6 +173,10 @@ static int netback_probe(struct xenbus_device *dev,
/* This kicks hotplug scripts, so do it immediately. */
backend_create_xenvif(be);
+ /* Register Frontend Event Notify */
+ be->vif_notifier.notifier_call = netback_netdev_event;
+ register_netdevice_notifier(&be->vif_notifier);
+
return 0;
abort_transaction:
--
1.7.9.5
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related
* [PATCH 4/15] drivers/net/ethernet/sfc/ptp.c: adjust duplicate test
From: Julia Lawall @ 2013-01-21 13:02 UTC (permalink / raw)
To: Solarflare linux maintainers
Cc: kernel-janitors, Ben Hutchings, netdev, linux-kernel
In-Reply-To: <1358773378-4700-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Delete successive tests to the same location. rc was previously tested and
not subsequently updated. efx_phc_adjtime can return an error code, so the
call is updated so that is tested instead.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
{ ... when forall
return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
{ ... when forall
return ...; }
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
Not tested. I'm not familiar with this function, so I don't know whether it
is desirable to test its result.
drivers/net/ethernet/sfc/ptp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 0767043..3f93624 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -1439,7 +1439,7 @@ static int efx_phc_settime(struct ptp_clock_info *ptp,
delta = timespec_sub(*e_ts, time_now);
- efx_phc_adjtime(ptp, timespec_to_ns(&delta));
+ rc = efx_phc_adjtime(ptp, timespec_to_ns(&delta));
if (rc != 0)
return rc;
^ permalink raw reply related
* [PATCH 6/15] drivers/net/wireless/iwlegacy/4965-mac.c: adjust duplicate test
From: Julia Lawall @ 2013-01-21 13:02 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: kernel-janitors, John W. Linville, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1358773378-4700-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Delete successive tests to the same location. This looks like simple code
duplication.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@s exists@
local idexpression y;
expression x,e;
@@
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
{ ... when forall
return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
{ ... when forall
return ...; }
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/net/wireless/iwlegacy/4965-mac.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index 10fc249..0195190 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -6572,9 +6572,6 @@ il4965_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (err)
goto out_free_eeprom;
- if (err)
- goto out_free_eeprom;
-
/* extract MAC Address */
il4965_eeprom_get_mac(il, il->addresses[0].addr);
D_INFO("MAC address: %pM\n", il->addresses[0].addr);
^ permalink raw reply related
* Re: [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: Stefan Richter @ 2013-01-21 13:15 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: netdev, linux1394-devel, David Miller
In-Reply-To: <50FD2EBE.9050608@linux-ipv6.org>
On Jan 21 YOSHIFUJI Hideaki wrote:
> Stefan Richter wrote:
> > On Jan 20 Stephan Gatzka wrote:
> >> On 01/20/2013 07:47 PM, YOSHIFUJI Hideaki wrote:
> >>
> >>> My current position is to change "mac address" to
> >>>
> >>> struct fwnet_hwaddr {
> >>> u8 guid[8];
> >>> u8 max_rec;
> >>> u8 sspd;
> >>> u8 fifo[6];
> >>> };
> >>>
> >>
> >> That is something I'm not really convinced of. As Stefan Richter pointed
> >> out clearly, the fifo address might be different between IPv4 and IPv6
> >> communication.
> >
> > If it is of any help, the initial implementation could assume that IPv4
> > unicast_FIFO and IPv6 unicast_FIFO are the same. RFC 3146 is silent on
> > this topic (which means it can be one way or the other), but from an
> > implementation point of view, using one FIFO offset for both seems quite
> > natural. Currently the only existing RFC 3146 implementation which is
> > known to us is Mac OS X, and since your tests with OS X 10.6 went well,
> > they obviously use one offset for both protocols.
> >
> > But if we actually put this assumption into the implementation now, we
> > should make sure that we can easily expand the implementation later in the
> > event that a third implementation comes across which uses separate
> > unicast_FIFOs.
>
> Well, FIFO for which side?
Our Linux implementation should expose a single unicast_FIFO for
reception of both protocols, just in case that another implementation
expects just this.
For transmission, we should be ready to keep an IP-peer-to-1394-node
mapping with per-protocol unicast_FIFOs, but in my mind it is doubtful
that any such implementation exists (hence we could just stand prepared
to implement it later when proven to be needed -- if this simplifies the
initial implementation notably).
> I do believe sender will not (or say, must not) care if they use
> different FIFO for both protocol or not.
>
> Assume that peer has FIFO per protocol, one for IPv4 and another for
> IPv6. ARP advertise FIFO for IPv4 and NDP advertise FIFO for IPv6.
> neighbour subsystem has protocol dependent tables, and two different
> NCEs (neighbour cache entries) will be created. So, sender will
> correctly get FIFO from NCE for each protocol.
OK.
--
Stefan Richter
-=====-===-= ---= =-=-=
http://arcgraph.de/sr/
^ permalink raw reply
* Re: IPsec AH use of ahash
From: Steven Rostedt @ 2013-01-21 13:38 UTC (permalink / raw)
To: Tom St Denis; +Cc: David Dillow, Borislav Petkov, linux-kernel, netdev
In-Reply-To: <207592188.94267.1358763627877.JavaMail.root@elliptictech.com>
On Mon, 2013-01-21 at 05:20 -0500, Tom St Denis wrote:
>
> ----- Original Message -----
> > From: "David Dillow" <dave@thedillows.org>
> > To: "Tom St Denis" <tstdenis@elliptictech.com>
> > Cc: "Borislav Petkov" <bp@alien8.de>, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, "Steven Rostedt"
> > <rostedt@goodmis.org>
> > Sent: Monday, 21 January, 2013 4:18:01 AM
> > Subject: Re: IPsec AH use of ahash
> >
> > On Sun, 2013-01-20 at 19:40 -0500, Tom St Denis wrote:
> > > The problem is for me to add the ()'s is a no brainer.
> >
> > It was more than just the ()'s. You completely botched the
> > indentation
> > in the parts of code you didn't copy from xcbc.c, among other issues.
>
> Leaving coding indentation aside [I assume you mean the use of braces because my tab stops should match given the patchset for the other files was minimal] that's not really a good use of time here.
But it's a good use of David Miller's time?
$ git log v3.6..v3.7 |grep 'Signed-off-by: David.*Miller' | wc -l
679
$ git log v3.6..v3.7 |grep 'Author: David.*Miller' | wc -l
136
David handled 679 patches, where 136 of them were his. This is between
3.6 and 3.7 which is a 3 month period. He handled 543 patches that were
not his. Imagine if he had to fix up coding styles for every one of
those patches. That would make any sane man quit their job.
I'm curious, how many patches do you handle for your OSS projects over a
3 month period?
>
> > > For me to re-write complete statements for coding style reasons
> > > means
> > > I have to actually go out and test it again.
> >
> > Well, for some of the code you submitted, that would be the first
> > testing it got, even compile testing AFAICT. I don't doubt that you
> > tested against your hardware, but it is obvious that you didn't run
> > the
> > algorithm tests you added, or even tried to compile them. You were
> > missing a semicolon after your test data, and you were also missing a
> > "\x" in there, so your test data was wrong.
>
> The missing semi-colon is in fact missing and for that I apologize. This btw is the first legitimate gripe with the code thus far.
I've also found that those that have poor coding styles also have more
of these "legitimate" problems too. Which is another reason not to
accept patches with coding style issues.
>
> Though I must say this code has "compiled" in 3.6, 3.6.7, 3.7, and 3.8 without error so it's not surprising it wasn't caught. After having consulted the "documentation" I never really did figure out how to get testmgr to run so I took a bit of leap of faith there that everything was fine. Not really an excuse but it is at least an observable hole in the build documentation.
Did you try different configs? Before submitting my code, I run under 8
different configs. SMP, non-SMP, PREEMPT, non PREEMPT, etc. also I run a
few random configs before submitting.
>
> I couldn't spot the missing \x but I don't doubt you. That format for vector data is so backwards that it's easy to make that mistake. After having worked on crypto code for 10+ years I've never seen anyone use that format and I've seen stupid formats before (NIST CAVP anyone?).
>
> Again all valid critiques of the code and valid reasons to do a re-spin. I apologize for the oversight there. Having "used the source" I found the build symbol required to activate the test manager so I should be good to go now.
Until you can play by the rules, don't bother playing. Feel free to dump
code on LKML. Maybe someone that knows how to play the game will take
it. But leave your gripes to your managers. No one here, but you, thinks
there's a problem with the frame work.
>
> > I know this, because I just spent the five minutes required to fix up
> > the checkpatch warnings to prove to myself that you could have done
> > it
> > faster than sending one considered message to the list about how much
> > we
> > suck. Of course, that assumes you are trying to have a discussion
> > instead of trolling; it's much easier to spew bile than to think
> > before
> > you post.
>
> This wasn't about how you all suck. Quite the contrary it was about how good people are missing perspective. You all started this by harping on the coding style "problems" but as a new developer to the kernel (well at least a new submitter...) I used the source as a guide so that I wouldn't deviate from what I perceived as the norm. I was legitimately frustrated because performing a re-spin solely to work past coding style problems is not something a business case could be made for. I'd like to contribute properly but at the same time you can't throw red flags on the play each time someone plays the game like you do.
> At least the testmgr errors are something I can work on whenever without setting up a lab so likely that'll be something I can tackle today actually.
While you're at it. You could spend an extra 5 minutes cleaning up the
coding style ;-)
-- Steve
^ permalink raw reply
* Re: IPsec AH use of ahash
From: Tom St Denis @ 2013-01-21 13:45 UTC (permalink / raw)
To: Steven Rostedt; +Cc: David Dillow, Borislav Petkov, linux-kernel, netdev
In-Reply-To: <1358775534.21576.8.camel@gandalf.local.home>
----- Original Message -----
> From: "Steven Rostedt" <rostedt@goodmis.org>
> To: "Tom St Denis" <tstdenis@elliptictech.com>
> Cc: "David Dillow" <dave@thedillows.org>, "Borislav Petkov" <bp@alien8.de>, linux-kernel@vger.kernel.org,
> netdev@vger.kernel.org
> Sent: Monday, 21 January, 2013 8:38:54 AM
> Subject: Re: IPsec AH use of ahash
> > The missing semi-colon is in fact missing and for that I apologize.
> > This btw is the first legitimate gripe with the code thus far.
>
> I've also found that those that have poor coding styles also have
> more
> of these "legitimate" problems too. Which is another reason not to
> accept patches with coding style issues.
I find that 73% of all stats are made up.
What actually happened was testmgr.h was being #ifdef'ed out so despite the fact I saw "testmgr.o" on the build process it wasn't actually doing anything. Not blaming anyone here .. just missed a build option.
I wouldn't have submitted it if the code failed to compile.
> > Though I must say this code has "compiled" in 3.6, 3.6.7, 3.7, and
> > 3.8 without error so it's not surprising it wasn't caught. After
> > having consulted the "documentation" I never really did figure out
> > how to get testmgr to run so I took a bit of leap of faith there
> > that everything was fine. Not really an excuse but it is at least
> > an observable hole in the build documentation.
>
> Did you try different configs? Before submitting my code, I run under
> 8
> different configs. SMP, non-SMP, PREEMPT, non PREEMPT, etc. also I
> run a
> few random configs before submitting.
None of course apply to this code [or shouldn't at least].
> Until you can play by the rules, don't bother playing. Feel free to
> dump
> code on LKML. Maybe someone that knows how to play the game will take
> it. But leave your gripes to your managers. No one here, but you,
> thinks
> there's a problem with the frame work.
I actually did resubmit this morning with most of the checkpatch issues fixed.
I'll avoid beating the dead horse though I'd like to move forward.
> > At least the testmgr errors are something I can work on whenever
> > without setting up a lab so likely that'll be something I can
> > tackle today actually.
>
> While you're at it. You could spend an extra 5 minutes cleaning up
> the
> coding style ;-)
It actually took me about 45 minutes and about 5 revisions of the patches to clean up all the random coding style gripes from checkpatch. The only reason I worked on it though is that there were build errors. That way I can justify it to my boss.
Seriously, no spaces on the trailing edge of multi line comments? :-/
Anyways, I did re-submit. I still have no idea how testmgr works but hopefully someone can pick it up from there.
Tom
^ permalink raw reply
* Re: [PATCH net] net: cdc_ncm: workaround for missing CDC Union
From: Bjørn Mork @ 2013-01-21 14:08 UTC (permalink / raw)
To: Alexey Orishko
Cc: Oliver Neukum, netdev, linux-usb, Greg Suarez, Alexey Orishko
In-Reply-To: <CAL_Kpj1B=_+eN3Y0n3d9_r4JoeHD2LQFSU+b+taCZL0G4icu6A@mail.gmail.com>
Hello Alexey,
I have another issue with the Sierra firmware which I hope you can help
me with: The MC7710 device requires at ZLP even if we send
dwNtbOutMaxSize sized NTBs. This is a problem because the current code
explicitly prevents this.
The following code in the v3.8 cdc-ncm was written to keep existing
behaviour from the pre-v3.8 driver:
if (((skb_out->len % le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0) &&
(skb_out->len < le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize)) && skb_tailroom(skb_out))
*skb_put(skb_out, 1) = 0; /* force short packet */
The previous implementaion looked like this:
6c60408e (Alexey Orishko 2011-05-06 03:01:30 +0000 832) if (((last_offset < ctx->tx_max) && ((last_offset %
6c60408e (Alexey Orishko 2011-05-06 03:01:30 +0000 833) le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0)) ||
6c60408e (Alexey Orishko 2011-05-06 03:01:30 +0000 834) (((last_offset == ctx->tx_max) && ((ctx->tx_max %
6c60408e (Alexey Orishko 2011-05-06 03:01:30 +0000 835) le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0)) &&
6c60408e (Alexey Orishko 2011-05-06 03:01:30 +0000 836) (ctx->tx_max < le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize)))) {
900d495a (Alexey Orishko 2010-11-29 23:23:28 +0000 837) /* force short packet */
900d495a (Alexey Orishko 2010-11-29 23:23:28 +0000 838) *(((u8 *)skb_out->data) + last_offset) = 0;
900d495a (Alexey Orishko 2010-11-29 23:23:28 +0000 839) last_offset++;
900d495a (Alexey Orishko 2010-11-29 23:23:28 +0000 840) }
The effect is the same: We add a 0 byte if the NTB length is a multiplum
of wMaxPacketSize *except* if the length is equal to dwNtbOutMaxSize.
This exception will happen very often because of the way we pad NTBs.
Now, I have tried to find what the above code was based on, and my guess
is that it is this note in table 3-1 in the CDC NCM spec:
If wBlockLength = 0x0000, the block is terminated by a
short packet. In this case, the USB transfer must still
be shorter than dwNtbInMaxSize or dwNtbOutMax-
Size. If exactly dwNtbInMaxSize or dwNtbOutMaxSize
bytes are sent, and the size is a multiple of wMax-
PacketSize for the given pipe, then no ZLP shall be
sent.
Is that correct? I cannot find any special ZLP handling mentioned
anywhere else in the standard.
If so, then I believe it is a misinterpretation. The above text deals
only with the exceptional case of wBlockLength = 0x0000, which we do not
do. As long as wBlockLength > 0 then I believe the device is in its
full right to expect a ZLP if wBlockLength % wMaxPacketSize is 0.
Would you feel comfortable dropping the additional condition and going
with
if (((skb_out->len % le16_to_cpu(ctx->out_ep->desc.wMaxPacketSize)) == 0) &&
skb_tailroom(skb_out))
*skb_put(skb_out, 1) = 0; /* force short packet */
? I have verified that this is sufficient to make the Sierra device
work. I will of course test it with the other NCM and MBIM devices I've
got, but that is a very limited set...
The other option I see is making a device specific quirk for this. But
I suspect that Sierra is using the current Qualcomm MBIM implemetation
here, and if so then we are likely to see a large number of similar
devices in the near future. I'd really like to avoid having device
specific quirks for all of them if at all possible.
Bjørn
^ permalink raw reply
* Re: [PATCH net-next 2/2] mcast: add multicast proxy support (IPv4 and IPv6)
From: David Stevens @ 2013-01-21 14:12 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: davem, netdev, netdev-owner, Nicolas Dichtel
In-Reply-To: <1358528378-2110-3-git-send-email-nicolas.dichtel@6wind.com>
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index a9454cb..e8ec94a 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -828,6 +828,48 @@ static struct mfc_cache *ipmr_cache_find(struct
> mr_table *mrt,
> return NULL;
> }
>
> +/* Look for a (*,*,oif) entry */
> +static struct mfc_cache *ipmr_cache_find_any_parent(struct mr_table
*mrt,
> + int vifi)
> +{
> + int line = MFC_HASH(0, 0);
> + struct mfc_cache *c;
> +
> + list_for_each_entry_rcu(c, &mrt->mfc_cache_array[line], list)
> + if (c->mfc_origin == 0 &&
> + c->mfc_mcastgrp == 0 &&
These are addresses-- should compare against INADDR_ANY to make
that
clear.
...
> +static struct mfc_cache *ipmr_cache_find_any(struct mr_table *mrt,
> + __be32 mcastgrp, int vifi)
> +{
...
> +
> + list_for_each_entry_rcu(c, &mrt->mfc_cache_array[line], list)
> + if (c->mfc_origin == 0 && c->mfc_mcastgrp == mcastgrp) {
Use INADDR_ANY instead of 0.
>
> - if (!ipv4_is_multicast(mfc->mfcc_mcastgrp.s_addr))
> + if (mfc->mfcc_mcastgrp.s_addr &&
> + !ipv4_is_multicast(mfc->mfcc_mcastgrp.s_addr))
> return -EINVAL;
mfc->mfcc_mcastgrp.s_addr != INADDR_ANY &&
Otherwise, it looks ok to me. With those changes,
Acked-by: David L Stevens <dlstevens@us.ibm.com>
+-DLS
^ permalink raw reply
* Re: [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: YOSHIFUJI Hideaki @ 2013-01-21 14:15 UTC (permalink / raw)
Cc: YOSHIFUJI Hideaki, Stefan Richter, stephan.gatzka,
Waskiewicz Jr, Peter P, linux1394-devel, netdev, davem
In-Reply-To: <50FD3575.1060303@linux-ipv6.org>
YOSHIFUJI Hideaki wrote:
> Stefan Richter wrote:
>
>> b) RFC 2734 MCAP (Multicast Channel Allocation Protocol): currently not
>> implemented; not sure if needed (I see a few big downsides to using
>> per-multicast-group channels); not sure if networking core support
>> would be needed for this
>
> It is definitely needed because we need to listen on multicast group
> for NDP, which might have already had its own channel.
Sorry, this was unclear and incorrect. RFC3146 Section 9
(IPv6 Multicast) says that packets for all-nodes/all-routers/
solicited-node multicast addresses must be sent via broadcast
channel. So, NDP should work without MCAP.
But, even so, I do not want to have broken implementation without
MCAP, or at least, we need prepare to support full multicast.
I do NOT want to have patchy implementation just for NDISC.
With extended MAC, networking core (including IPv4/IPv6) does not
need to be changed very much for supporting MCAP; no new hooks,
no new callbacks.
--yoshfuji
^ permalink raw reply
* Re: [PATCH net-next 1/2] mcast: define and use MRT[6]_MAX in ip[6]_mroute_opt()
From: David Stevens @ 2013-01-21 14:16 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: davem, netdev, netdev-owner, Nicolas Dichtel
In-Reply-To: <1358528378-2110-2-git-send-email-nicolas.dichtel@6wind.com>
Acked-by: David L Stevens <dlstevens@us.ibm.com>
netdev-owner@vger.kernel.org wrote on 01/18/2013 11:59:37 AM:
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net, Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: 01/18/2013 12:10 PM
> Subject: [PATCH net-next 1/2] mcast: define and use MRT[6]_MAX in ip
> [6]_mroute_opt()
> Sent by: netdev-owner@vger.kernel.org
>
> This will ease further addition of new MRT[6]_* values and avoid to
update
> in6.h each time.
> Note that we reduce the maximum value from 210 to 209, but 210 does not
match
> any known value in ip[6]_mroute_setsockopt().
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> include/linux/mroute.h | 2 +-
> include/linux/mroute6.h | 2 +-
> include/uapi/linux/in6.h | 15 ++++-----------
> include/uapi/linux/mroute.h | 1 +
> include/uapi/linux/mroute6.h | 1 +
> 5 files changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/mroute.h b/include/linux/mroute.h
> index ea00d91..79aaa9f 100644
> --- a/include/linux/mroute.h
> +++ b/include/linux/mroute.h
> @@ -9,7 +9,7 @@
> #ifdef CONFIG_IP_MROUTE
> static inline int ip_mroute_opt(int opt)
> {
> - return (opt >= MRT_BASE) && (opt <= MRT_BASE + 10);
> + return (opt >= MRT_BASE) && (opt <= MRT_MAX);
> }
> #else
> static inline int ip_mroute_opt(int opt)
> diff --git a/include/linux/mroute6.h b/include/linux/mroute6.h
> index a223561..66982e7 100644
> --- a/include/linux/mroute6.h
> +++ b/include/linux/mroute6.h
> @@ -10,7 +10,7 @@
> #ifdef CONFIG_IPV6_MROUTE
> static inline int ip6_mroute_opt(int opt)
> {
> - return (opt >= MRT6_BASE) && (opt <= MRT6_BASE + 10);
> + return (opt >= MRT6_BASE) && (opt <= MRT6_MAX);
> }
> #else
> static inline int ip6_mroute_opt(int opt)
> diff --git a/include/uapi/linux/in6.h b/include/uapi/linux/in6.h
> index 5673b97..53b1d56 100644
> --- a/include/uapi/linux/in6.h
> +++ b/include/uapi/linux/in6.h
> @@ -259,17 +259,10 @@ struct in6_flowlabel_req {
>
> /*
> * Multicast Routing:
> - * see include/linux/mroute6.h.
> + * see include/uapi/linux/mroute6.h.
> *
> - * MRT6_INIT 200
> - * MRT6_DONE 201
> - * MRT6_ADD_MIF 202
> - * MRT6_DEL_MIF 203
> - * MRT6_ADD_MFC 204
> - * MRT6_DEL_MFC 205
> - * MRT6_VERSION 206
> - * MRT6_ASSERT 207
> - * MRT6_PIM 208
> - * (reserved) 209
> + * MRT6_BASE 200
> + * ...
> + * MRT6_MAX
> */
> #endif /* _UAPI_LINUX_IN6_H */
> diff --git a/include/uapi/linux/mroute.h b/include/uapi/linux/mroute.h
> index 1692999..1c11004 100644
> --- a/include/uapi/linux/mroute.h
> +++ b/include/uapi/linux/mroute.h
> @@ -26,6 +26,7 @@
> #define MRT_ASSERT (MRT_BASE+7) /* Activate PIM assert mode */
> #define MRT_PIM (MRT_BASE+8) /* enable PIM code */
> #define MRT_TABLE (MRT_BASE+9) /* Specify mroute table ID */
> +#define MRT_MAX (MRT_BASE+9)
>
> #define SIOCGETVIFCNT SIOCPROTOPRIVATE /* IP protocol privates */
> #define SIOCGETSGCNT (SIOCPROTOPRIVATE+1)
> diff --git a/include/uapi/linux/mroute6.h b/include/uapi/linux/mroute6.h
> index 3e89b5e..c206ae3 100644
> --- a/include/uapi/linux/mroute6.h
> +++ b/include/uapi/linux/mroute6.h
> @@ -26,6 +26,7 @@
> #define MRT6_ASSERT (MRT6_BASE+7) /* Activate PIM assert mode */
> #define MRT6_PIM (MRT6_BASE+8) /* enable PIM code */
> #define MRT6_TABLE (MRT6_BASE+9) /* Specify mroute table ID */
> +#define MRT6_MAX (MRT6_BASE+9)
>
> #define SIOCGETMIFCNT_IN6 SIOCPROTOPRIVATE /* IP protocol privates
*/
> #define SIOCGETSGCNT_IN6 (SIOCPROTOPRIVATE+1)
> --
> 1.8.0.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: IPsec AH use of ahash
From: Steven Rostedt @ 2013-01-21 14:37 UTC (permalink / raw)
To: Tom St Denis; +Cc: David Dillow, Borislav Petkov, linux-kernel, netdev
In-Reply-To: <1924594099.94729.1358775949651.JavaMail.root@elliptictech.com>
On Mon, 2013-01-21 at 08:45 -0500, Tom St Denis wrote:
>
> ----- Original Message -----
> > From: "Steven Rostedt" <rostedt@goodmis.org>
> > To: "Tom St Denis" <tstdenis@elliptictech.com>
> > Cc: "David Dillow" <dave@thedillows.org>, "Borislav Petkov" <bp@alien8.de>, linux-kernel@vger.kernel.org,
> > netdev@vger.kernel.org
> > Sent: Monday, 21 January, 2013 8:38:54 AM
> > Subject: Re: IPsec AH use of ahash
>
> > > The missing semi-colon is in fact missing and for that I apologize.
> > > This btw is the first legitimate gripe with the code thus far.
> >
> > I've also found that those that have poor coding styles also have
> > more
> > of these "legitimate" problems too. Which is another reason not to
> > accept patches with coding style issues.
>
> I find that 73% of all stats are made up.
I was only talking about my own experience. I gave no numbers.
> > Did you try different configs? Before submitting my code, I run under
> > 8
> > different configs. SMP, non-SMP, PREEMPT, non PREEMPT, etc. also I
> > run a
> > few random configs before submitting.
>
> None of course apply to this code [or shouldn't at least].
I was just giving examples of what I use. As those usually apply to what
I do. If your code is affected by any configs, you should compile with
them on and off to make sure you didn't break them. This is a bit more
extensive testing, and not always required. But it does help to do so,
as it becomes embarrassing if your code breaks on a config you didn't
test.
That's coming from my own experience too ;-)
>
>
> > Until you can play by the rules, don't bother playing. Feel free to
> > dump
> > code on LKML. Maybe someone that knows how to play the game will take
> > it. But leave your gripes to your managers. No one here, but you,
> > thinks
> > there's a problem with the frame work.
>
> I actually did resubmit this morning with most of the checkpatch issues fixed.
Thank you.
>
> I'll avoid beating the dead horse though I'd like to move forward.
>
> > > At least the testmgr errors are something I can work on whenever
> > > without setting up a lab so likely that'll be something I can
> > > tackle today actually.
> >
> > While you're at it. You could spend an extra 5 minutes cleaning up
> > the
> > coding style ;-)
>
> It actually took me about 45 minutes and about 5 revisions of the patches to clean up all the random coding style gripes from checkpatch. The only reason I worked on it though is that there were build errors. That way I can justify it to my boss.
Your boss micro manages your time that much? And 45 mintes to do that?
>
> Seriously, no spaces on the trailing edge of multi line comments? :-/
Some of checkpatch'es complaints are annoying. I'll grant you that. And
checkpatch is more of a guideline than a strict rule. It's up to the
maintainer of the code to determine how much checkpatch should be
enforced.
For example, checkpatch complains on code like:
+ asm volatile (
+#ifdef CONFIG_X86_64
+ " xchg %%rbx,%%rsp \n"
+#else
+ " xchgl %%ebx,%%esp \n"
+#endif
+ " int3 \n"
+ " .globl jprobe_return_end\n"
+ " jprobe_return_end: \n"
+ " nop \n"::"b"
+ (kcb->jprobe_saved_sp):"memory");
Because the white space before the '\n' is not needed. But adding that
whitespace makes it easier to read the assembly.
When enforcing checkpatch makes the code less readable, that's when it
should be ignored. But again, that's really up to the maintainer of the
code to decide.
>
> Anyways, I did re-submit. I still have no idea how testmgr works but hopefully someone can pick it up from there.
Well thank you again. This is the way the kernel community works. Just
state you're not familiar with testmgr, and someone who is should check
it out.
-- Steve
^ permalink raw reply
* Re: [PATCH net] net: cdc_ncm: workaround for missing CDC Union
From: Bjørn Mork @ 2013-01-21 14:47 UTC (permalink / raw)
To: Alexey Orishko
Cc: Oliver Neukum, netdev, linux-usb, Greg Suarez, Alexey Orishko
In-Reply-To: <87r4ley6g9.fsf@nemi.mork.no>
Bjørn Mork <bjorn@mork.no> writes:
> I have another issue with the Sierra firmware which I hope you can help
> me with: The MC7710 device requires at ZLP even if we send
> dwNtbOutMaxSize sized NTBs. This is a problem because the current code
> explicitly prevents this.
If anyone found this more than normally confused, then that is
correct...
What the NCM code does is that it sends a *short* packet whenever the
usbnet would normally have done so, except if the NTP length >=
dwNtbOutMaxSize.
The problem is that we do not set
urb->transfer_flags |= URB_ZERO_PACKET;
either in this special case, resulting in neither a short nor a zero
length packet being sent. I believe this is what the Sierra firmware
chokes at. This seems to fix the issue, and is IMHO correct:
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 42f51c7..3a5673a 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -366,7 +366,7 @@ err:
static const struct driver_info cdc_mbim_info = {
.description = "CDC MBIM",
- .flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN,
+ .flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN | FLAG_SEND_ZLP,
.bind = cdc_mbim_bind,
.unbind = cdc_mbim_unbind,
.manage_power = cdc_mbim_manage_power,
But I wonder if this isn't really a generic problem in usbnet. The
FLAG_MULTI_PACKET test here seems completely bogus:
if (length % dev->maxpacket == 0) {
if (!(info->flags & FLAG_SEND_ZLP)) {
if (!(info->flags & FLAG_MULTI_PACKET)) {
urb->transfer_buffer_length++;
if (skb_tailroom(skb)) {
skb->data[skb->len] = 0;
__skb_put(skb, 1);
}
}
} else
urb->transfer_flags |= URB_ZERO_PACKET;
}
Either the FLAG_MULTI_PACKET minidriver will have already padded the
buffer so that we do not hit (length % dev->maxpacket == 0), or we
should choose one of the alternatives: ZLP or padding.
Minidrivers not wanting the short packets (like cdc_ncm) must set
FLAG_SEND_ZLP.
Bjørn
^ permalink raw reply related
* Re: IPsec AH use of ahash
From: Tom St Denis @ 2013-01-21 14:51 UTC (permalink / raw)
To: Steven Rostedt; +Cc: David Dillow, Borislav Petkov, linux-kernel, netdev
In-Reply-To: <1358779061.21576.19.camel@gandalf.local.home>
----- Original Message -----
> From: "Steven Rostedt" <rostedt@goodmis.org>
> To: "Tom St Denis" <tstdenis@elliptictech.com>
> Cc: "David Dillow" <dave@thedillows.org>, "Borislav Petkov" <bp@alien8.de>, linux-kernel@vger.kernel.org,
> netdev@vger.kernel.org
> Sent: Monday, 21 January, 2013 9:37:41 AM
> Subject: Re: IPsec AH use of ahash
> >
> > I find that 73% of all stats are made up.
>
> I was only talking about my own experience. I gave no numbers.
That was a joke. You assumed that because I don't trim whitespace from multi-line comments [among other asinine code style issues] that I'm a bad developer. Yet what actually happened was a build configuration error in which the file wasn't being compiled fully.
Let it go.
> I was just giving examples of what I use. As those usually apply to
> what
> I do. If your code is affected by any configs, you should compile
> with
> them on and off to make sure you didn't break them. This is a bit
> more
> extensive testing, and not always required. But it does help to do
> so,
> as it becomes embarrassing if your code breaks on a config you didn't
> test.
>
> That's coming from my own experience too ;-)
Yup, I missed the self-test flag in the menu. That's full on my bad.
That said, it should be the opposite [default on] since self-testing should be relatively cheap and easy. Generally unless it's prohibitive you want as much self-test code-path testing in the default build as possible. Users who are tight on memory can turn it off if it suits their platform.
> > I actually did resubmit this morning with most of the checkpatch
> > issues fixed.
>
> Thank you.
Like I said I'm not trying to force everyone else to adopt to how we do things. I was just airing out a complaint from the point of view of a new submitter.
I still think checkpatch rules are full of sh!t but I know now to run code I submit through it regardless of where the code came from. :-)
> Your boss micro manages your time that much? And 45 mintes to do
> that?
Strictly speaking I haven't actually tried the code out in the lab yet. I was hoping that testmgr would run but it hasn't.
Realistically speaking none of the changes I made this morning should have any bearing on the correctness of the code. I'd be surprised if it failed in the lab.
> > Seriously, no spaces on the trailing edge of multi line comments?
> > :-/
>
> Some of checkpatch'es complaints are annoying. I'll grant you that.
> And
> checkpatch is more of a guideline than a strict rule. It's up to the
> maintainer of the code to determine how much checkpatch should be
> enforced.
That's not the impression I got from this weekends exchange.
> For example, checkpatch complains on code like:
>
> + asm volatile (
> +#ifdef CONFIG_X86_64
> + " xchg %%rbx,%%rsp \n"
> +#else
> + " xchgl %%ebx,%%esp \n"
> +#endif
> + " int3 \n"
> + " .globl jprobe_return_end\n"
> + " jprobe_return_end: \n"
> + " nop \n"::"b"
> + (kcb->jprobe_saved_sp):"memory");
>
> Because the white space before the '\n' is not needed. But adding
> that
> whitespace makes it easier to read the assembly.
So who gets to decide when/where to deviate from the rules?
> When enforcing checkpatch makes the code less readable, that's when
> it
> should be ignored. But again, that's really up to the maintainer of
> the
> code to decide.
So we divine what the maintainer wants and doesn't want when we submit patches? I think for me I'm going to follow them literally for now to avoid issues.
> > Anyways, I did re-submit. I still have no idea how testmgr works
> > but hopefully someone can pick it up from there.
>
> Well thank you again. This is the way the kernel community works.
> Just
> state you're not familiar with testmgr, and someone who is should
> check
> it out.
Hopefully :-)
Tom
^ permalink raw reply
* Re: [PATCH net] net: cdc_ncm: workaround for missing CDC Union
From: Oliver Neukum @ 2013-01-21 14:55 UTC (permalink / raw)
To: Bjørn Mork
Cc: Alexey Orishko, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Greg Suarez, Alexey Orishko
In-Reply-To: <87mww2y4ni.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
On Monday 21 January 2013 15:47:13 Bjørn Mork wrote:
> But I wonder if this isn't really a generic problem in usbnet. The
> FLAG_MULTI_PACKET test here seems completely bogus:
>
> if (length % dev->maxpacket == 0) {
> if (!(info->flags & FLAG_SEND_ZLP)) {
> if (!(info->flags & FLAG_MULTI_PACKET)) {
> urb->transfer_buffer_length++;
> if (skb_tailroom(skb)) {
> skb->data[skb->len] = 0;
> __skb_put(skb, 1);
> }
> }
> } else
> urb->transfer_flags |= URB_ZERO_PACKET;
> }
>
> Either the FLAG_MULTI_PACKET minidriver will have already padded the
> buffer so that we do not hit (length % dev->maxpacket == 0), or we
> should choose one of the alternatives: ZLP or padding.
But we cannot simply call __skb_put for a complicated data frame.
Besides you may want the current behavior.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* r8169 doing more work than napi weight
From: Timo Teras @ 2013-01-21 15:12 UTC (permalink / raw)
To: Francois Romieu, netdev
Hi,
I'm getting:
WARNING: at linux-grsec/src/linux-3.4/net/core/dev.c:3875 net_rx_action+0xab/0x153()
on one of my r8169 boxes.
This would be the:
WARN_ON_ONCE(work > weight);
Now the only way this seems to be possible to happen is that the AMD
workaround triggers:
if ((desc->opts2 & cpu_to_le32(0xfffe000)) &&
(tp->mac_version == RTL_GIGA_MAC_VER_05)) {
desc->opts2 = 0;
cur_rx++;
}
And yes, the hardware where the WARN_ON_ONCE triggers is indeed
RTL_GIGA_MAC_VER_05.
This would cause cur_rx to be incremented twice in the loop, but
rx_left not decremented accordingly.
As the work done is counted finally based on cur_rx, we might end up
returning more work done than what was our quota.
This has also the unwanted consequence of messing NAPI state as if more
work than quota was done then polling is stopped as the work == weight
does not trigger and the polling is not rescheduled.
Git log says that this workaround was copied from Realtek's r8168
driver, but I don't see anything like this there anymore.
I'm wondering if we should just delete the
cur_rx++;
Or add:
rx_left--;
Or just delete the whole block as obsolete. 'git log' says the problem
should have gone away by always using hardware Rx VLAN. See commit
05af214 "r8169: fix Ethernet Hangup for RTL8110SC rev d".
Thanks,
Timo
^ 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