* Re: [patch v1 1/2] Allow Mellanox network vendor to be configured if only I2C bus is configured
From: David Miller @ 2017-08-14 0:25 UTC (permalink / raw)
To: ohado; +Cc: leon, netdev, jiri, saeedm, vadimp, system-sw-low-level
In-Reply-To: <VI1PR05MB1262B06790B2738CE1C0F889C88F0@VI1PR05MB1262.eurprd05.prod.outlook.com>
From: Ohad Oz <ohado@mellanox.com>
Date: Sun, 13 Aug 2017 15:26:56 +0000
>
>
>> -----Original Message-----
>> From: Leon Romanovsky [mailto:leon@kernel.org]
>> Sent: Saturday, August 12, 2017 5:37 PM
>> To: Ohad Oz <ohado@mellanox.com>
>> Cc: davem@davemloft.net; netdev@vger.kernel.org; jiri@resnulli.us; Saeed
>> Mahameed <saeedm@mellanox.com>; Vadim Pasternak
>> <vadimp@mellanox.com>; system-sw-low-level <system-sw-low-
>> level@mellanox.com>
>> Subject: Re: [patch v1 1/2] Allow Mellanox network vendor to be configured
>> if only I2C bus is configured
>>
>> On Thu, Aug 10, 2017 at 05:11:51PM +0000, Ohad Oz wrote:
>> > Patch allows Mellanox devices on system with no PCI, but with I2C only.
>> >
>>
>> Did you test mlx5 device on such system? Did it work for you?
>
> Yes, I did. With PCI config set to disable mlx5 drivers are not built.
> Only the following:
> /build/drivers/net/Ethernet/mellanox/mlxsw/mlxsw_core.ko
> /build/drivers/net/Ethernet/mellanox /mlxsw/mlxsw_i2c.ko
> /build/drivers/net/Ethernet/mellanox /mlxsw/mlxsw_minimal.ko
>
> While with both options on all drivers are built inc mlx5.
I'm not so sure he's interested if things build or not.
He's asking you if you actually used a Mellanox device with this
driver with PCI disabled and only I2C available.
^ permalink raw reply
* Re: [PATCH 0/3] NCSI VLAN Filtering Support
From: Samuel Mendoza-Jonas @ 2017-08-14 1:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel, openbmc, joel, benh, gwshan, ratagupt
In-Reply-To: <20170811.144823.1928647996484981190.davem@davemloft.net>
On Fri, 2017-08-11 at 14:48 -0700, David Miller wrote:
> From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> Date: Fri, 11 Aug 2017 16:16:45 +1000
>
> > This series (mainly patch 3) adds VLAN filtering to the NCSI implementation.
> > A fair amount of code already exists in the NCSI stack for VLAN filtering but
> > none of it is actually hooked up. This goes the final mile and fixes a few
> > bugs in the existing code found along the way (patch 2).
> >
> > Patch 1 adds the appropriate flag to the ftgmac100 driver to enable filtering
> > as it's a large consumer of NCSI (and what I've been testing on).
>
> This patch series does not apply cleanly to net-next, you need to respin.
Sorry about that, silly Friday mistake :)
I noticed I've also squashed an ftgmac100 change into the wrong patch,
I'll fix that up along with the respin.
Cheers,
Sam
^ permalink raw reply
* [PATCH net-next v2 0/3] NCSI VLAN Filtering Support
From: Samuel Mendoza-Jonas @ 2017-08-14 1:29 UTC (permalink / raw)
To: David S . Miller, netdev, linux-kernel, OpenBMC Maillist
Cc: Samuel Mendoza-Jonas, Joel Stanley, Benjamin Herrenschmidt,
Gavin Shan, ratagupt
This series (mainly patch 2) adds VLAN filtering to the NCSI implementation.
A fair amount of code already exists in the NCSI stack for VLAN filtering but
none of it is actually hooked up. This goes the final mile and fixes a few
bugs in the existing code found along the way (patch 1).
Patch 3 adds the appropriate flag and callbacks to the ftgmac100 driver to
enable filtering as it's a large consumer of NCSI (and what I've been
testing on).
Samuel Mendoza-Jonas (3):
net/ncsi: Fix several packet definitions
net/ncsi: Configure VLAN tag filter
ftgmac100: Support NCSI VLAN filtering when available
drivers/net/ethernet/faraday/ftgmac100.c | 5 +
include/net/ncsi.h | 2 +
net/ncsi/internal.h | 11 ++
net/ncsi/ncsi-cmd.c | 10 +-
net/ncsi/ncsi-manage.c | 295 ++++++++++++++++++++++++++++++-
net/ncsi/ncsi-pkt.h | 2 +-
net/ncsi/ncsi-rsp.c | 12 +-
7 files changed, 326 insertions(+), 11 deletions(-)
--
2.14.0
^ permalink raw reply
* [PATCH net-next v2 1/3] net/ncsi: Fix several packet definitions
From: Samuel Mendoza-Jonas @ 2017-08-14 1:29 UTC (permalink / raw)
To: David S . Miller, netdev, linux-kernel, OpenBMC Maillist
Cc: Samuel Mendoza-Jonas, Joel Stanley, Benjamin Herrenschmidt,
Gavin Shan, ratagupt
In-Reply-To: <20170814012952.13740-1-sam@mendozajonas.com>
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
v2: Rebased on latest net-next
net/ncsi/ncsi-cmd.c | 10 +++++-----
net/ncsi/ncsi-pkt.h | 2 +-
net/ncsi/ncsi-rsp.c | 3 ++-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 5e03ed190e18..7567ca63aae2 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -139,9 +139,9 @@ static int ncsi_cmd_handler_svf(struct sk_buff *skb,
struct ncsi_cmd_svf_pkt *cmd;
cmd = skb_put_zero(skb, sizeof(*cmd));
- cmd->vlan = htons(nca->words[0]);
- cmd->index = nca->bytes[2];
- cmd->enable = nca->bytes[3];
+ cmd->vlan = htons(nca->words[1]);
+ cmd->index = nca->bytes[6];
+ cmd->enable = nca->bytes[7];
ncsi_cmd_build_header(&cmd->cmd.common, nca);
return 0;
@@ -153,7 +153,7 @@ static int ncsi_cmd_handler_ev(struct sk_buff *skb,
struct ncsi_cmd_ev_pkt *cmd;
cmd = skb_put_zero(skb, sizeof(*cmd));
- cmd->mode = nca->bytes[0];
+ cmd->mode = nca->bytes[3];
ncsi_cmd_build_header(&cmd->cmd.common, nca);
return 0;
@@ -228,7 +228,7 @@ static struct ncsi_cmd_handler {
{ NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae },
{ NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl },
{ NCSI_PKT_CMD_GLS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_SVF, 4, ncsi_cmd_handler_svf },
+ { NCSI_PKT_CMD_SVF, 8, ncsi_cmd_handler_svf },
{ NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev },
{ NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_SMA, 8, ncsi_cmd_handler_sma },
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 3ea49ed0a935..91b4b66438df 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -104,7 +104,7 @@ struct ncsi_cmd_svf_pkt {
unsigned char index; /* VLAN table index */
unsigned char enable; /* Enable or disable */
__be32 checksum; /* Checksum */
- unsigned char pad[14];
+ unsigned char pad[18];
};
/* Enable VLAN */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 087db775b3dc..c1a191d790e2 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -354,7 +354,8 @@ static int ncsi_rsp_handler_svf(struct ncsi_request *nr)
/* Add or remove the VLAN filter */
if (!(cmd->enable & 0x1)) {
- ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index);
+ /* HW indexes from 1 */
+ ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index - 1);
} else {
vlan = ntohs(cmd->vlan);
ret = ncsi_add_filter(nc, NCSI_FILTER_VLAN, &vlan);
--
2.14.0
^ permalink raw reply related
* [PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter
From: Samuel Mendoza-Jonas @ 2017-08-14 1:29 UTC (permalink / raw)
To: David S . Miller, netdev, linux-kernel, OpenBMC Maillist
Cc: Samuel Mendoza-Jonas, Joel Stanley, Benjamin Herrenschmidt,
Gavin Shan, ratagupt
In-Reply-To: <20170814012952.13740-1-sam@mendozajonas.com>
Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI
stack process new VLAN tags and configure the channel VLAN filter
appropriately.
Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent
for each one, meaning the ncsi_dev_state_config_svf state must be
repeated. An internal list of VLAN tags is maintained, and compared
against the current channel's ncsi_channel_filter in order to keep track
within the state. VLAN filters are removed in a similar manner, with the
introduction of the ncsi_dev_state_config_clear_vids state. The maximum
number of VLAN tag filters is determined by the "Get Capabilities"
response from the channel.
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
include/net/ncsi.h | 2 +
net/ncsi/internal.h | 11 ++
net/ncsi/ncsi-manage.c | 295 ++++++++++++++++++++++++++++++++++++++++++++++++-
net/ncsi/ncsi-rsp.c | 9 +-
4 files changed, 313 insertions(+), 4 deletions(-)
diff --git a/include/net/ncsi.h b/include/net/ncsi.h
index 68680baac0fd..1f96af46df49 100644
--- a/include/net/ncsi.h
+++ b/include/net/ncsi.h
@@ -28,6 +28,8 @@ struct ncsi_dev {
};
#ifdef CONFIG_NET_NCSI
+int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid);
+int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid);
struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*notifier)(struct ncsi_dev *nd));
int ncsi_start_dev(struct ncsi_dev *nd);
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 1308a56f2591..af3d636534ef 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -180,6 +180,7 @@ struct ncsi_channel {
#define NCSI_CHANNEL_INACTIVE 1
#define NCSI_CHANNEL_ACTIVE 2
#define NCSI_CHANNEL_INVISIBLE 3
+ bool reconfigure_needed;
spinlock_t lock; /* Protect filters etc */
struct ncsi_package *package;
struct ncsi_channel_version version;
@@ -235,6 +236,9 @@ enum {
ncsi_dev_state_probe_dp,
ncsi_dev_state_config_sp = 0x0301,
ncsi_dev_state_config_cis,
+ ncsi_dev_state_config_clear_vids,
+ ncsi_dev_state_config_svf,
+ ncsi_dev_state_config_ev,
ncsi_dev_state_config_sma,
ncsi_dev_state_config_ebf,
#if IS_ENABLED(CONFIG_IPV6)
@@ -253,6 +257,12 @@ enum {
ncsi_dev_state_suspend_done
};
+struct vlan_vid {
+ struct list_head list;
+ __be16 proto;
+ u16 vid;
+};
+
struct ncsi_dev_priv {
struct ncsi_dev ndev; /* Associated NCSI device */
unsigned int flags; /* NCSI device flags */
@@ -276,6 +286,7 @@ struct ncsi_dev_priv {
struct work_struct work; /* For channel management */
struct packet_type ptype; /* NCSI packet Rx handler */
struct list_head node; /* Form NCSI device list */
+ struct list_head vlan_vids; /* List of active VLAN IDs */
};
struct ncsi_cmd_arg {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index a3bd5fa8ad09..3cbd4328f142 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table)
return sizes[table];
}
+u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index)
+{
+ struct ncsi_channel_filter *ncf;
+ int size;
+
+ ncf = nc->filters[table];
+ if (!ncf)
+ return NULL;
+
+ size = ncsi_filter_size(table);
+ if (size < 0)
+ return NULL;
+
+ return ncf->data + size * index;
+}
+
int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
{
struct ncsi_channel_filter *ncf;
@@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
index = -1;
while ((index = find_next_bit(bitmap, ncf->total, index + 1))
< ncf->total) {
- if (!memcmp(ncf->data + size * index, data, size)) {
+ if (!data || !memcmp(ncf->data + size * index, data, size)) {
spin_unlock_irqrestore(&nc->lock, flags);
return index;
}
@@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
nd->state = ncsi_dev_state_functional;
}
+/* Check the VLAN filter bitmap for a set filter, and construct a
+ * "Set VLAN Filter - Disable" packet if found.
+ */
+static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
+ struct ncsi_cmd_arg *nca)
+{
+ int index;
+ u16 vid;
+
+ index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL);
+ if (index < 0) {
+ /* Filter table empty */
+ return -1;
+ }
+
+ vid = *(u16 *)ncsi_get_filter(nc, NCSI_FILTER_VLAN, index);
+ netdev_printk(KERN_DEBUG, ndp->ndev.dev,
+ "ncsi: removed vlan tag %u at index %d\n",
+ vid, index + 1);
+ ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index);
+
+ nca->type = NCSI_PKT_CMD_SVF;
+ nca->words[1] = vid;
+ /* HW filter index starts at 1 */
+ nca->bytes[6] = index + 1;
+ nca->bytes[7] = 0x00;
+ return 0;
+}
+
+/* Find an outstanding VLAN tag and constuct a "Set VLAN Filter - Enable"
+ * packet.
+ */
+static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
+ struct ncsi_cmd_arg *nca)
+{
+ struct vlan_vid *vlan = NULL;
+ int index = 0;
+
+ list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
+ index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, &vlan->vid);
+ if (index < 0) {
+ /* New tag to add */
+ netdev_printk(KERN_DEBUG, ndp->ndev.dev,
+ "ncsi: new vlan id to set: %u\n",
+ vlan->vid);
+ break;
+ }
+ netdev_printk(KERN_DEBUG, ndp->ndev.dev,
+ "vid %u already at filter pos %d\n",
+ vlan->vid, index);
+ }
+
+ if (!vlan || index >= 0) {
+ netdev_printk(KERN_DEBUG, ndp->ndev.dev,
+ "no vlan ids left to set\n");
+ return -1;
+ }
+
+ index = ncsi_add_filter(nc, NCSI_FILTER_VLAN, &vlan->vid);
+ if (index < 0) {
+ netdev_err(ndp->ndev.dev,
+ "Failed to add new VLAN tag, error %d\n", index);
+ return -1;
+ }
+
+ netdev_printk(KERN_DEBUG, ndp->ndev.dev,
+ "ncsi: set vid %u in packet, index %u\n",
+ vlan->vid, index + 1);
+ nca->type = NCSI_PKT_CMD_SVF;
+ nca->words[1] = vlan->vid;
+ /* HW filter index starts at 1 */
+ nca->bytes[6] = index + 1;
+ nca->bytes[7] = 0x01;
+
+ return 0;
+}
+
static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
{
struct ncsi_dev *nd = &ndp->ndev;
@@ -683,8 +776,11 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
if (ret)
goto error;
- nd->state = ncsi_dev_state_config_sma;
+ nd->state = ncsi_dev_state_config_clear_vids;
break;
+ case ncsi_dev_state_config_clear_vids:
+ case ncsi_dev_state_config_svf:
+ case ncsi_dev_state_config_ev:
case ncsi_dev_state_config_sma:
case ncsi_dev_state_config_ebf:
#if IS_ENABLED(CONFIG_IPV6)
@@ -699,11 +795,40 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
nca.package = np->id;
nca.channel = nc->id;
+ /* Clear any active filters on the channel before setting */
+ if (nd->state == ncsi_dev_state_config_clear_vids) {
+ ret = clear_one_vid(ndp, nc, &nca);
+ if (ret) {
+ nd->state = ncsi_dev_state_config_svf;
+ schedule_work(&ndp->work);
+ break;
+ }
+ /* Repeat */
+ nd->state = ncsi_dev_state_config_clear_vids;
+ /* Add known VLAN tags to the filter */
+ } else if (nd->state == ncsi_dev_state_config_svf) {
+ ret = set_one_vid(ndp, nc, &nca);
+ if (ret) {
+ nd->state = ncsi_dev_state_config_ev;
+ schedule_work(&ndp->work);
+ break;
+ }
+ /* Repeat */
+ nd->state = ncsi_dev_state_config_svf;
+ /* Enable/Disable the VLAN filter */
+ } else if (nd->state == ncsi_dev_state_config_ev) {
+ if (list_empty(&ndp->vlan_vids)) {
+ nca.type = NCSI_PKT_CMD_DV;
+ } else {
+ nca.type = NCSI_PKT_CMD_EV;
+ nca.bytes[3] = NCSI_CAP_VLAN_NO;
+ }
+ nd->state = ncsi_dev_state_config_sma;
+ } else if (nd->state == ncsi_dev_state_config_sma) {
/* Use first entry in unicast filter table. Note that
* the MAC filter table starts from entry 1 instead of
* 0.
*/
- if (nd->state == ncsi_dev_state_config_sma) {
nca.type = NCSI_PKT_CMD_SMA;
for (index = 0; index < 6; index++)
nca.bytes[index] = dev->dev_addr[index];
@@ -751,6 +876,26 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
break;
case ncsi_dev_state_config_done:
spin_lock_irqsave(&nc->lock, flags);
+ if (nc->reconfigure_needed) {
+ /* This channel's configuration has been updated
+ * part-way during the config state - start the
+ * channel configuration over
+ */
+ nc->reconfigure_needed = false;
+ nc->state = NCSI_CHANNEL_INVISIBLE;
+ spin_unlock_irqrestore(&nc->lock, flags);
+
+ spin_lock_irqsave(&ndp->lock, flags);
+ nc->state = NCSI_CHANNEL_INACTIVE;
+ list_add_tail_rcu(&nc->link, &ndp->channel_queue);
+ spin_unlock_irqrestore(&ndp->lock, flags);
+
+ netdev_printk(KERN_DEBUG, dev,
+ "Dirty NCSI channel state reset\n");
+ ncsi_process_next_channel(ndp);
+ break;
+ }
+
if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
hot_nc = nc;
nc->state = NCSI_CHANNEL_ACTIVE;
@@ -1191,6 +1336,149 @@ static struct notifier_block ncsi_inet6addr_notifier = {
};
#endif /* CONFIG_IPV6 */
+static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
+{
+ struct ncsi_dev *nd = &ndp->ndev;
+ struct ncsi_channel *nc;
+ struct ncsi_package *np;
+ unsigned long flags;
+ unsigned int n = 0;
+
+ NCSI_FOR_EACH_PACKAGE(ndp, np) {
+ NCSI_FOR_EACH_CHANNEL(np, nc) {
+ spin_lock_irqsave(&nc->lock, flags);
+
+ /* Channels may be busy, mark dirty instead of
+ * kicking if;
+ * a) not ACTIVE (configured)
+ * b) in the channel_queue (to be configured)
+ * c) it's ndev is in the config state
+ */
+ if (nc->state != NCSI_CHANNEL_ACTIVE) {
+ if ((ndp->ndev.state & 0xff00) ==
+ ncsi_dev_state_config ||
+ !list_empty(&nc->link)) {
+ netdev_printk(KERN_DEBUG, nd->dev,
+ "ncsi: channel %p marked dirty\n",
+ nc);
+ nc->reconfigure_needed = true;
+ }
+ spin_unlock_irqrestore(&nc->lock, flags);
+ continue;
+ }
+
+ spin_unlock_irqrestore(&nc->lock, flags);
+
+ ncsi_stop_channel_monitor(nc);
+ spin_lock_irqsave(&nc->lock, flags);
+ nc->state = NCSI_CHANNEL_INVISIBLE;
+ spin_unlock_irqrestore(&nc->lock, flags);
+
+ spin_lock_irqsave(&ndp->lock, flags);
+ nc->state = NCSI_CHANNEL_INACTIVE;
+ list_add_tail_rcu(&nc->link, &ndp->channel_queue);
+ spin_unlock_irqrestore(&ndp->lock, flags);
+
+ netdev_printk(KERN_DEBUG, nd->dev,
+ "ncsi: kicked channel %p\n", nc);
+ n++;
+ }
+ }
+
+ return n;
+}
+
+int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+ struct ncsi_channel_filter *ncf;
+ struct ncsi_dev_priv *ndp;
+ unsigned int n_vids = 0;
+ struct vlan_vid *vlan;
+ struct ncsi_dev *nd;
+ bool found = false;
+
+ if (vid == 0)
+ return 0;
+
+ nd = ncsi_find_dev(dev);
+ if (!nd) {
+ netdev_warn(dev, "ncsi: No net_device?\n");
+ return 0;
+ }
+
+ ndp = TO_NCSI_DEV_PRIV(nd);
+ ncf = ndp->hot_channel->filters[NCSI_FILTER_VLAN];
+
+ /* Add the VLAN id to our internal list */
+ list_for_each_entry_rcu(vlan, &ndp->vlan_vids, list) {
+ n_vids++;
+ if (vlan->vid == vid) {
+ netdev_printk(KERN_DEBUG, dev,
+ "vid %u already registered\n", vid);
+ return 0;
+ }
+ }
+
+ if (n_vids >= ncf->total) {
+ netdev_info(dev,
+ "NCSI Channel supports up to %u VLAN tags but %u are already set\n",
+ ncf->total, n_vids);
+ return -EINVAL;
+ }
+
+ vlan = kzalloc(sizeof(*vlan), GFP_KERNEL);
+ if (!vlan)
+ return -ENOMEM;
+
+ vlan->proto = proto;
+ vlan->vid = vid;
+ list_add_rcu(&vlan->list, &ndp->vlan_vids);
+
+ netdev_printk(KERN_DEBUG, dev, "Added new vid %u\n", vid);
+
+ found = ncsi_kick_channels(ndp) != 0;
+
+ return found ? ncsi_process_next_channel(ndp) : 0;
+}
+
+int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
+{
+ struct vlan_vid *vlan, *tmp;
+ struct ncsi_dev_priv *ndp;
+ struct ncsi_dev *nd;
+ bool found = false;
+
+ if (vid == 0)
+ return 0;
+
+ nd = ncsi_find_dev(dev);
+ if (!nd) {
+ netdev_warn(dev, "ncsi: no net_device?\n");
+ return 0;
+ }
+
+ ndp = TO_NCSI_DEV_PRIV(nd);
+
+ /* Remove the VLAN id from our internal list */
+ list_for_each_entry_safe(vlan, tmp, &ndp->vlan_vids, list)
+ if (vlan->vid == vid) {
+ netdev_printk(KERN_DEBUG, dev,
+ "vid %u found, removing\n", vid);
+ list_del_rcu(&vlan->list);
+ found = true;
+ kfree(vlan);
+ }
+
+ if (!found) {
+ netdev_err(dev, "ncsi: vid %u wasn't registered!\n", vid);
+ return -EINVAL;
+ }
+
+ found = ncsi_kick_channels(ndp) != 0;
+
+ return found ? ncsi_process_next_channel(ndp) : 0;
+}
+
struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
void (*handler)(struct ncsi_dev *ndev))
{
@@ -1215,6 +1503,7 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
nd->handler = handler;
ndp->pending_req_num = 0;
INIT_LIST_HEAD(&ndp->channel_queue);
+ INIT_LIST_HEAD(&ndp->vlan_vids);
INIT_WORK(&ndp->work, ncsi_dev_work);
/* Initialize private NCSI device */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index c1a191d790e2..265b9a892d41 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -694,7 +694,14 @@ static int ncsi_rsp_handler_gc(struct ncsi_request *nr)
ncf->index = i;
ncf->total = cnt;
- ncf->bitmap = 0x0ul;
+ if (i == NCSI_FILTER_VLAN) {
+ /* Set VLAN filters active so they are cleared in
+ * first configuration state
+ */
+ ncf->bitmap = U64_MAX;
+ } else {
+ ncf->bitmap = 0x0ul;
+ }
nc->filters[i] = ncf;
}
--
2.14.0
^ permalink raw reply related
* [PATCH net-next v2 3/3] ftgmac100: Support NCSI VLAN filtering when available
From: Samuel Mendoza-Jonas @ 2017-08-14 1:29 UTC (permalink / raw)
To: David S . Miller, netdev, linux-kernel, OpenBMC Maillist
Cc: Samuel Mendoza-Jonas, Joel Stanley, Benjamin Herrenschmidt,
Gavin Shan, ratagupt
In-Reply-To: <20170814012952.13740-1-sam@mendozajonas.com>
Register the ndo_vlan_rx_{add,kill}_vid callbacks and set the
NETIF_F_HW_VLAN_CTAG_FILTER if NCSI is available.
This allows the VLAN core to notify the NCSI driver when changes occur
so that the remote NCSI channel can be properly configured to filter on
the set VLAN tags.
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
v2: Moved ftgmac100 change into same patch and reordered
drivers/net/ethernet/faraday/ftgmac100.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 34dae51effd4..05fe7123d5ae 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1623,6 +1623,8 @@ static const struct net_device_ops ftgmac100_netdev_ops = {
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = ftgmac100_poll_controller,
#endif
+ .ndo_vlan_rx_add_vid = ncsi_vlan_rx_add_vid,
+ .ndo_vlan_rx_kill_vid = ncsi_vlan_rx_kill_vid,
};
static int ftgmac100_setup_mdio(struct net_device *netdev)
@@ -1837,6 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX |
NETIF_F_HW_VLAN_CTAG_TX;
+ if (priv->use_ncsi)
+ netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+
/* AST2400 doesn't have working HW checksum generation */
if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
netdev->hw_features &= ~NETIF_F_HW_CSUM;
--
2.14.0
^ permalink raw reply related
* Re: [PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter
From: Joel Stanley @ 2017-08-14 2:09 UTC (permalink / raw)
To: Samuel Mendoza-Jonas
Cc: David S . Miller, netdev, Linux Kernel Mailing List,
OpenBMC Maillist, Benjamin Herrenschmidt, Gavin Shan,
Ratan K Gupta
In-Reply-To: <20170814012952.13740-3-sam@mendozajonas.com>
On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas
<sam@mendozajonas.com> wrote:
> Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI
> stack process new VLAN tags and configure the channel VLAN filter
> appropriately.
> Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent
> for each one, meaning the ncsi_dev_state_config_svf state must be
> repeated. An internal list of VLAN tags is maintained, and compared
> against the current channel's ncsi_channel_filter in order to keep track
> within the state. VLAN filters are removed in a similar manner, with the
> introduction of the ncsi_dev_state_config_clear_vids state. The maximum
> number of VLAN tag filters is determined by the "Get Capabilities"
> response from the channel.
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
I've given this some testing, but there are a few things I saw below
that we should sort out.
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table)
> return sizes[table];
> }
>
> +u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index)
> +{
> + struct ncsi_channel_filter *ncf;
> + int size;
> +
> + ncf = nc->filters[table];
> + if (!ncf)
> + return NULL;
> +
> + size = ncsi_filter_size(table);
> + if (size < 0)
> + return NULL;
> +
> + return ncf->data + size * index;
> +}
> +
> int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
> {
> struct ncsi_channel_filter *ncf;
> @@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
> index = -1;
> while ((index = find_next_bit(bitmap, ncf->total, index + 1))
> < ncf->total) {
> - if (!memcmp(ncf->data + size * index, data, size)) {
> + if (!data || !memcmp(ncf->data + size * index, data, size)) {
Not clear why this check is required?
> spin_unlock_irqrestore(&nc->lock, flags);
> return index;
> }
> @@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
> nd->state = ncsi_dev_state_functional;
> }
>
> +/* Check the VLAN filter bitmap for a set filter, and construct a
> + * "Set VLAN Filter - Disable" packet if found.
> + */
> +static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
> + struct ncsi_cmd_arg *nca)
> +{
> + int index;
> + u16 vid;
> +
> + index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL);
> + if (index < 0) {
> + /* Filter table empty */
> + return -1;
> + }
> +
> + vid = *(u16 *)ncsi_get_filter(nc, NCSI_FILTER_VLAN, index);
You just added this function that returns a pointer to a u32. It's
strange to see the only call site then throw half of it away.
Also, ncsi_get_filter can return NULL.
> + netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> + "ncsi: removed vlan tag %u at index %d\n",
> + vid, index + 1);
> + ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index);
> +
> + nca->type = NCSI_PKT_CMD_SVF;
> + nca->words[1] = vid;
> + /* HW filter index starts at 1 */
> + nca->bytes[6] = index + 1;
> + nca->bytes[7] = 0x00;
> + return 0;
> +}
> @@ -751,6 +876,26 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> break;
> case ncsi_dev_state_config_done:
> spin_lock_irqsave(&nc->lock, flags);
> + if (nc->reconfigure_needed) {
> + /* This channel's configuration has been updated
> + * part-way during the config state - start the
> + * channel configuration over
> + */
> + nc->reconfigure_needed = false;
> + nc->state = NCSI_CHANNEL_INVISIBLE;
> + spin_unlock_irqrestore(&nc->lock, flags);
> +
> + spin_lock_irqsave(&ndp->lock, flags);
> + nc->state = NCSI_CHANNEL_INACTIVE;
This looks strange. What's nc->state up to? Does setting it to
NCSI_CHANNEL_INVISIBLE have any affect?
The locking is confusing too.
> + list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> + spin_unlock_irqrestore(&ndp->lock, flags);
> +
> + netdev_printk(KERN_DEBUG, dev,
> + "Dirty NCSI channel state reset\n");
> + ncsi_process_next_channel(ndp);
> + break;
> + }
> +
> if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
> hot_nc = nc;
> nc->state = NCSI_CHANNEL_ACTIVE;
> @@ -1191,6 +1336,149 @@ static struct notifier_block ncsi_inet6addr_notifier = {
> };
> #endif /* CONFIG_IPV6 */
>
> +static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
> +{
> + struct ncsi_dev *nd = &ndp->ndev;
> + struct ncsi_channel *nc;
> + struct ncsi_package *np;
> + unsigned long flags;
> + unsigned int n = 0;
> +
> + NCSI_FOR_EACH_PACKAGE(ndp, np) {
> + NCSI_FOR_EACH_CHANNEL(np, nc) {
> + spin_lock_irqsave(&nc->lock, flags);
> +
> + /* Channels may be busy, mark dirty instead of
> + * kicking if;
> + * a) not ACTIVE (configured)
> + * b) in the channel_queue (to be configured)
> + * c) it's ndev is in the config state
> + */
> + if (nc->state != NCSI_CHANNEL_ACTIVE) {
> + if ((ndp->ndev.state & 0xff00) ==
> + ncsi_dev_state_config ||
> + !list_empty(&nc->link)) {
> + netdev_printk(KERN_DEBUG, nd->dev,
> + "ncsi: channel %p marked dirty\n",
> + nc);
> + nc->reconfigure_needed = true;
> + }
> + spin_unlock_irqrestore(&nc->lock, flags);
> + continue;
> + }
> +
> + spin_unlock_irqrestore(&nc->lock, flags);
> +
> + ncsi_stop_channel_monitor(nc);
> + spin_lock_irqsave(&nc->lock, flags);
> + nc->state = NCSI_CHANNEL_INVISIBLE;
> + spin_unlock_irqrestore(&nc->lock, flags);
> +
> + spin_lock_irqsave(&ndp->lock, flags);
> + nc->state = NCSI_CHANNEL_INACTIVE;
This is a similar pattern to ncsi_configure_channel. I suspect the
answer there will be the same as here.
> + list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> + spin_unlock_irqrestore(&ndp->lock, flags);
> +
> + netdev_printk(KERN_DEBUG, nd->dev,
> + "ncsi: kicked channel %p\n", nc);
> + n++;
> + }
> + }
^ permalink raw reply
* Re: [PATCH net-next v2 1/3] net/ncsi: Fix several packet definitions
From: Joel Stanley @ 2017-08-14 2:10 UTC (permalink / raw)
To: Samuel Mendoza-Jonas
Cc: David S . Miller, netdev, Linux Kernel Mailing List,
OpenBMC Maillist, Benjamin Herrenschmidt, Gavin Shan,
Ratan K Gupta
In-Reply-To: <20170814012952.13740-2-sam@mendozajonas.com>
On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas
<sam@mendozajonas.com> wrote:
I asked Sam if these should be backported to stable and he said:
> These are straight up bugs except... without my changes we never call
> this code. As Ben says as time provides a lot of the current definitions
> need to be gone over, there's a few command/response code paths that are
> never triggered and could be broken in similar ways.
So we're okay here.
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Cheers,
Joel
> ---
> v2: Rebased on latest net-next
>
> net/ncsi/ncsi-cmd.c | 10 +++++-----
> net/ncsi/ncsi-pkt.h | 2 +-
> net/ncsi/ncsi-rsp.c | 3 ++-
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> index 5e03ed190e18..7567ca63aae2 100644
> --- a/net/ncsi/ncsi-cmd.c
> +++ b/net/ncsi/ncsi-cmd.c
> @@ -139,9 +139,9 @@ static int ncsi_cmd_handler_svf(struct sk_buff *skb,
> struct ncsi_cmd_svf_pkt *cmd;
>
> cmd = skb_put_zero(skb, sizeof(*cmd));
> - cmd->vlan = htons(nca->words[0]);
> - cmd->index = nca->bytes[2];
> - cmd->enable = nca->bytes[3];
> + cmd->vlan = htons(nca->words[1]);
> + cmd->index = nca->bytes[6];
> + cmd->enable = nca->bytes[7];
> ncsi_cmd_build_header(&cmd->cmd.common, nca);
>
> return 0;
> @@ -153,7 +153,7 @@ static int ncsi_cmd_handler_ev(struct sk_buff *skb,
> struct ncsi_cmd_ev_pkt *cmd;
>
> cmd = skb_put_zero(skb, sizeof(*cmd));
> - cmd->mode = nca->bytes[0];
> + cmd->mode = nca->bytes[3];
> ncsi_cmd_build_header(&cmd->cmd.common, nca);
>
> return 0;
> @@ -228,7 +228,7 @@ static struct ncsi_cmd_handler {
> { NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae },
> { NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl },
> { NCSI_PKT_CMD_GLS, 0, ncsi_cmd_handler_default },
> - { NCSI_PKT_CMD_SVF, 4, ncsi_cmd_handler_svf },
> + { NCSI_PKT_CMD_SVF, 8, ncsi_cmd_handler_svf },
> { NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev },
> { NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_SMA, 8, ncsi_cmd_handler_sma },
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index 3ea49ed0a935..91b4b66438df 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -104,7 +104,7 @@ struct ncsi_cmd_svf_pkt {
> unsigned char index; /* VLAN table index */
> unsigned char enable; /* Enable or disable */
> __be32 checksum; /* Checksum */
> - unsigned char pad[14];
> + unsigned char pad[18];
> };
>
> /* Enable VLAN */
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index 087db775b3dc..c1a191d790e2 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -354,7 +354,8 @@ static int ncsi_rsp_handler_svf(struct ncsi_request *nr)
>
> /* Add or remove the VLAN filter */
> if (!(cmd->enable & 0x1)) {
> - ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index);
> + /* HW indexes from 1 */
> + ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index - 1);
> } else {
> vlan = ntohs(cmd->vlan);
> ret = ncsi_add_filter(nc, NCSI_FILTER_VLAN, &vlan);
> --
> 2.14.0
>
^ permalink raw reply
* Re: [PATCH net-next v2 3/3] ftgmac100: Support NCSI VLAN filtering when available
From: Joel Stanley @ 2017-08-14 2:16 UTC (permalink / raw)
To: Samuel Mendoza-Jonas
Cc: David S . Miller, netdev, Linux Kernel Mailing List,
OpenBMC Maillist, Benjamin Herrenschmidt, Gavin Shan,
Ratan K Gupta
In-Reply-To: <20170814012952.13740-4-sam@mendozajonas.com>
On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas
<sam@mendozajonas.com> wrote:
> Register the ndo_vlan_rx_{add,kill}_vid callbacks and set the
> NETIF_F_HW_VLAN_CTAG_FILTER if NCSI is available.
> This allows the VLAN core to notify the NCSI driver when changes occur
> so that the remote NCSI channel can be properly configured to filter on
> the set VLAN tags.
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
I'm not a vlan expert, so I asked why this was only being set when
doing NCSI. The answer was:
> There is no point setting it when not doing ncsi. The HW doesn't have a
> filter in that case (we do have HW vlan tag extraction and injection,
> which my driver supports, but that's different flags).
Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> v2: Moved ftgmac100 change into same patch and reordered
>
> drivers/net/ethernet/faraday/ftgmac100.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 34dae51effd4..05fe7123d5ae 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1623,6 +1623,8 @@ static const struct net_device_ops ftgmac100_netdev_ops = {
> #ifdef CONFIG_NET_POLL_CONTROLLER
> .ndo_poll_controller = ftgmac100_poll_controller,
> #endif
> + .ndo_vlan_rx_add_vid = ncsi_vlan_rx_add_vid,
> + .ndo_vlan_rx_kill_vid = ncsi_vlan_rx_kill_vid,
> };
>
> static int ftgmac100_setup_mdio(struct net_device *netdev)
> @@ -1837,6 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
> NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX |
> NETIF_F_HW_VLAN_CTAG_TX;
>
> + if (priv->use_ncsi)
> + netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
> +
> /* AST2400 doesn't have working HW checksum generation */
> if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> netdev->hw_features &= ~NETIF_F_HW_CSUM;
> --
> 2.14.0
>
^ permalink raw reply
* Re: [PATCH net-next 8/8] liquidio: added support for ethtool --set-ring feature
From: David Miller @ 2017-08-14 2:43 UTC (permalink / raw)
To: felix.manlunas
Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
intiyaz.basha
In-Reply-To: <20170812012930.GA2614@felix-thinkpad.cavium.com>
From: Felix Manlunas <felix.manlunas@cavium.com>
Date: Fri, 11 Aug 2017 18:29:30 -0700
> + if ((ering->rx_mini_pending) || (ering->rx_jumbo_pending))
> + return -EINVAL;
Lots of unnecessary parenthesis, please remove.
> + if ((rx_count == rx_count_old) && (tx_count == tx_count_old))
> + return 0;
Likewise.
^ permalink raw reply
* Re: [PATCH net-next] rtnelink: Move link dump consistency check out of the loop
From: David Miller @ 2017-08-14 2:44 UTC (permalink / raw)
To: jkbs; +Cc: netdev
In-Reply-To: <20170809153912.19733-1-jkbs@redhat.com>
From: Jakub Sitnicki <jkbs@redhat.com>
Date: Wed, 9 Aug 2017 17:39:12 +0200
> Calls to rtnl_dump_ifinfo() are protected by RTNL lock. So are the
> {list,unlist}_netdevice() calls where we bump the net->dev_base_seq
> number.
>
> For this reason net->dev_base_seq can't change under out feet while
> we're looping over links in rtnl_dump_ifinfo(). So move the check for
> net->dev_base_seq change (since the last time we were called) out of the
> loop.
>
> This way we avoid giving a wrong impression that there are concurrent
> updates to the link list going on while we're iterating over them.
>
> Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCHv2 net-next] gre: introduce native tunnel support for ERSPAN
From: David Miller @ 2017-08-14 2:53 UTC (permalink / raw)
To: u9012063; +Cc: netdev, mvohra, kuznet, yoshfuji
In-Reply-To: <1502311986-36324-1-git-send-email-u9012063@gmail.com>
From: William Tu <u9012063@gmail.com>
Date: Wed, 9 Aug 2017 13:53:05 -0700
> The patch adds ERSPAN type II tunnel support. The implementation
> is based on the draft at [1]. One of the purposes is for Linux
> box to be able to receive ERSPAN monitoring traffic sent from
> the Cisco switch, by creating a ERSPAN tunnel device.
> In addition, the patch also adds ERSPAN TX, so traffic can
> also be encapsulated into ERSPAN and sent out.
>
> The implementation reuses the key as ERSPAN session ID, and
> field 'erspan' as ERSPAN Index fields:
> ./ip link add dev ers11 type erspan seq key 100 erspan 123 \
> local 172.16.1.200 remote 172.16.1.100
>
> [1] https://tools.ietf.org/html/draft-foschiano-erspan-01
> [2] iproute patch: http://marc.info/?l=linux-netdev&m=150231090207544&w=2
>
> Signed-off-by: William Tu <u9012063@gmail.com>
> Signed-off-by: Meenakshi Vohra <mvohra@vmware.com>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> ---
> v1->v2:
> Add missing erspan.h header
>
> ---
> include/net/erspan.h | 62 +++++++++++
> include/net/ip_tunnels.h | 3 +
> include/uapi/linux/if_ether.h | 1 +
> include/uapi/linux/if_tunnel.h | 1 +
> net/ipv4/ip_gre.c | 248 +++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 315 insertions(+)
> create mode 100644 include/net/erspan.h
>
> diff --git a/include/net/erspan.h b/include/net/erspan.h
> new file mode 100644
> index 000000000000..cafe387a2cae
> --- /dev/null
> +++ b/include/net/erspan.h
> @@ -0,0 +1,62 @@
> +#ifndef __LINUX_ERSPAN_H
> +#define __LINUX_ERSPAN_H
> +
> +/*
> + * GRE header for ERSPAN encapsulation (8 octets [34:41]) -- 8 bytes
> + * 0 1 2 3
> + * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * |0|0|0|1|0|00000|000000000|00000| Protocol Type for ERSPAN |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Sequence Number (increments per packet per session) |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + *
> + * Note that in the above GRE header [RFC1701] out of the C, R, K, S,
> + * s, Recur, Flags, Version fields only S (bit 03) is set to 1. The
> + * other fields are set to zero, so only a sequence number follows.
> + *
> + * ERSPAN Type II header (8 octets [42:49])
> + * 0 1 2 3
> + * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Ver | VLAN | COS | En|T| Session ID |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | Reserved | Index |
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + *
> + * GRE proto ERSPAN type II = 0x88BE, type III = 0x22EB
> + */
> +
> +#define ERSPAN_VERSION 0x1
> +
> +#define VER_MASK 0xf000
> +#define VLAN_MASK 0x0fff
> +#define COS_MASK 0xe000
> +#define EN_MASK 0x1800
> +#define BOS_MASK 0x1800 //?
> +#define T_MASK 0x0400
> +#define ID_MASK 0x03ff
> +#define INDEX_MASK 0xfffff
> +
> +enum erspan_encap_type {
> + ERSPAN_ENCAP_NOVLAN = 0x0, /* originally without VLAN tag */
> + ERSPAN_ENCAP_ISL = 0x1, /* originally ISL encapsulated */
> + ERSPAN_ENCAP_8021Q = 0x2, /* originally 802.1Q encapsulated */
> + ERSPAN_ENCAP_INFRAME = 0x3, /* VLAN tag perserved in frame */
> +};
> +
> +struct erspan_metadata {
> + __be32 index; /* type II */
> +};
> +
> +struct erspanhdr {
> + __be16 ver_vlan;
> +#define VER_OFFSET 12
> + __be16 session_id;
> +#define COS_OFFSET 13
> +#define EN_OFFSET 11
> +#define T_OFFSET 10
> + struct erspan_metadata md;
> +};
> +
> +#endif
> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
> index 520809912f03..625c29329372 100644
> --- a/include/net/ip_tunnels.h
> +++ b/include/net/ip_tunnels.h
> @@ -115,6 +115,9 @@ struct ip_tunnel {
> u32 o_seqno; /* The last output seqno */
> int tun_hlen; /* Precalculated header length */
>
> + /* This field used only by ERSPAN */
> + u32 index; /* ERSPAN type II index */
> +
> struct dst_cache dst_cache;
>
> struct ip_tunnel_parm parms;
> diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
> index 5bc9bfd816b7..efeb1190c2ca 100644
> --- a/include/uapi/linux/if_ether.h
> +++ b/include/uapi/linux/if_ether.h
> @@ -66,6 +66,7 @@
> #define ETH_P_ATALK 0x809B /* Appletalk DDP */
> #define ETH_P_AARP 0x80F3 /* Appletalk AARP */
> #define ETH_P_8021Q 0x8100 /* 802.1Q VLAN Extended Header */
> +#define ETH_P_ERSPAN 0x88BE /* ERSPAN type II */
> #define ETH_P_IPX 0x8137 /* IPX over DIX */
> #define ETH_P_IPV6 0x86DD /* IPv6 over bluebook */
> #define ETH_P_PAUSE 0x8808 /* IEEE Pause frames. See 802.3 31B */
> diff --git a/include/uapi/linux/if_tunnel.h b/include/uapi/linux/if_tunnel.h
> index 6792d1967d31..2e520883c054 100644
> --- a/include/uapi/linux/if_tunnel.h
> +++ b/include/uapi/linux/if_tunnel.h
> @@ -134,6 +134,7 @@ enum {
> IFLA_GRE_COLLECT_METADATA,
> IFLA_GRE_IGNORE_DF,
> IFLA_GRE_FWMARK,
> + IFLA_GRE_ERSPAN_INDEX,
> __IFLA_GRE_MAX,
> };
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 7a7829e839c2..e15d5f01cdb5 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -48,6 +48,7 @@
> #include <net/rtnetlink.h>
> #include <net/gre.h>
> #include <net/dst_metadata.h>
> +#include <net/erspan.h>
>
> /*
> Problems & solutions
> @@ -115,6 +116,7 @@ static int ipgre_tunnel_init(struct net_device *dev);
>
> static unsigned int ipgre_net_id __read_mostly;
> static unsigned int gre_tap_net_id __read_mostly;
> +static unsigned int erspan_net_id __read_mostly;
>
> static void ipgre_err(struct sk_buff *skb, u32 info,
> const struct tnl_ptk_info *tpi)
> @@ -246,6 +248,48 @@ static void gre_err(struct sk_buff *skb, u32 info)
> ipgre_err(skb, info, &tpi);
> }
>
> +static int erspan_rcv(struct sk_buff *skb, struct tnl_ptk_info *tpi,
> + int gre_hdr_len)
> +{
> + struct net *net = dev_net(skb->dev);
> + struct ip_tunnel_net *itn;
> + struct ip_tunnel *tunnel;
> + struct metadata_dst *tun_dst = NULL;
> + const struct iphdr *iph;
> + struct erspanhdr *ershdr;
> + __be32 index;
> + __be32 session_id;
> +
> + itn = net_generic(net, erspan_net_id);
> + iph = ip_hdr(skb);
> + ershdr = (struct erspanhdr *)(skb->data + gre_hdr_len);
You're not guaranteed the this ershdr area is pulled linearly in the
SKB. Only the GRE header and it's options have that guarantee.
So you'll need to add appropriate pskb_may_pull() checks here then
reaload all of the packet pointers (including 'iph') afterwards.
^ permalink raw reply
* Re: [PATCH net-next V2 0/3] XDP support for tap
From: David Miller @ 2017-08-14 2:56 UTC (permalink / raw)
To: jasowang; +Cc: mst, netdev, linux-kernel, kubakici
In-Reply-To: <1502451678-17358-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Fri, 11 Aug 2017 19:41:15 +0800
> Hi all:
>
> This series tries to implement XDP support for tap. Two path were
> implemented:
>
> - fast path: small & non-gso packet, For performance reason we do it
> at page level and use build_skb() to create skb if necessary.
> - slow path: big or gso packet, we don't want to lose the capability
> compared to generic XDP, so we export some generic xdp helpers and
> do it after skb was created.
>
> xdp1 shows about 41% improvement, xdp_redirect shows about 60%
> improvement.
>
> Changes from V1:
> - fix the race between xdp set and free
> - don't hold extra refcount
> - add XDP_REDIRECT support
>
> Please review.
Series applied, thanks Jason.
^ permalink raw reply
* Re: [PATCH net-next v3] vxlan: change vxlan_[config_]validate() to use netlink_ext_ack for error reporting
From: David Miller @ 2017-08-14 3:00 UTC (permalink / raw)
To: girish.moodalbail; +Cc: pshelar, netdev, mschiffer, jbenc, roopa
In-Reply-To: <1502490059-24229-1-git-send-email-girish.moodalbail@oracle.com>
From: Girish Moodalbail <girish.moodalbail@oracle.com>
Date: Fri, 11 Aug 2017 15:20:59 -0700
> The kernel log is not where users expect error messages for netlink
> requests; as we have extended acks now, we can replace pr_debug() with
> NL_SET_ERR_MSG_ATTR().
>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v2] bonding: ratelimit failed speed/duplex update warning
From: David Miller @ 2017-08-14 3:02 UTC (permalink / raw)
To: futur.andy; +Cc: netdev, joe
In-Reply-To: <20170811223655.11446-1-futur.andy@googlemail.com>
From: Andreas Born <futur.andy@googlemail.com>
Date: Sat, 12 Aug 2017 00:36:55 +0200
> bond_miimon_commit() handles the UP transition for each slave of a bond
> in the case of MII. It is triggered 10 times per second for the default
> MII Polling interval of 100ms. For device drivers that do not implement
> __ethtool_get_link_ksettings() the call to bond_update_speed_duplex()
> fails persistently while the MII status could remain UP. That is, in
> this and other cases where the speed/duplex update keeps failing over a
> longer period of time while the MII state is UP, a warning is printed
> every MII polling interval.
>
> To address these excessive warnings net_ratelimit() should be used.
> Printing a warning once would not be sufficient since the call to
> bond_update_speed_duplex() could recover to succeed and fail again
> later. In that case there would be no new indication what went wrong.
>
> Fixes: b5bf0f5b16b9c (bonding: correctly update link status during mii-commit phase)
> Signed-off-by: Andreas Born <futur.andy@googlemail.com>
> ---
> Changes in v2:
> * swapped pr_warn_ratelimited() for net_ratelimit()
Applied and you'll be happy to know I queued this up for -stable too :)
^ permalink raw reply
* Re: [PATCH net-next] net: ipv4: remove unnecessary check on orig_oif
From: David Miller @ 2017-08-14 3:03 UTC (permalink / raw)
To: dsahern; +Cc: netdev
In-Reply-To: <1502496122-27784-1-git-send-email-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Fri, 11 Aug 2017 17:02:02 -0700
> rt_iif is going to be set to either 0 or orig_oif. If orig_oif
> is 0 it amounts to the same end result so remove the check.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied, thanks David.
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: vrf: Support for local traffic with sockets bound to enslaved devices
From: David Miller @ 2017-08-14 3:05 UTC (permalink / raw)
To: dsahern; +Cc: netdev
In-Reply-To: <1502496675-13349-1-git-send-email-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Fri, 11 Aug 2017 17:11:13 -0700
> This set gets local traffic working for sockets bound to enslaved
> devices. The local rtable and rt6_info added in June 2016 to get
> local traffic in VRFs working is no longer needed and actually
> keeps local traffic for sockets bound to an enslaved device from
> working. Patch 1 removes them.
>
> Patch 2 adds a fix up for IPv4 IP_PKTINFO to return rt_iif for
> packets sent over the VRF device. This is similar to the handling
> of loopback.
Series applied, thanks David.
^ permalink raw reply
* Re: [PATCH] of_mdio: merge branch tails in of_phy_register_fixed_link()
From: David Miller @ 2017-08-14 3:09 UTC (permalink / raw)
To: sergei.shtylyov
Cc: andrew, f.fainelli, robh+dt, frowand.list, netdev, devicetree
In-Reply-To: <20170812210321.520045884@cogentembedded.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sun, 13 Aug 2017 00:03:06 +0300
> Looks like gcc isn't always able to figure out that 3 *if* branches in
> of_phy_register_fixed_link() calling fixed_phy_register() at their ends
> are similar enough and thus can be merged. The "manual" merge saves 40
> bytes of the object code (AArch64 gcc 4.8.5), and still saves 12 bytes
> even if gcc was able to merge the branch tails (ARM gcc 4.8.5)...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Applied, but if two instances of the "same" compiler just with
different targets changes the optimization, it could be because of a
tradeoff which is specific to parameters expressed in that target's
backend.
So in the future we should probably back away from trying to "help"
the compiler in this way.
^ permalink raw reply
* Re: [PATCH] virtio-net: make array guest_offloads static
From: David Miller @ 2017-08-14 3:10 UTC (permalink / raw)
To: colin.king; +Cc: mst, netdev, kernel-janitors, linux-kernel, virtualization
In-Reply-To: <20170812214553.6073-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Sat, 12 Aug 2017 22:45:53 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> The array guest_offloads is local to the source and does not need to
> be in global scope, so make it static. Also tweak formatting.
>
> Cleans up sparse warnings:
> symbol 'guest_offloads' was not declared. Should it be static?
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH] tap: make struct tap_fops static
From: David Miller @ 2017-08-14 3:11 UTC (permalink / raw)
To: colin.king
Cc: sainath.grandhi, jasowang, rolf.neugebauer, willemb, zhangshengju,
netdev, kernel-janitors, linux-kernel
In-Reply-To: <20170812215231.6272-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Sat, 12 Aug 2017 22:52:31 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> The structure tap_fops is local to the source and does not need to
> be in global scope, so make it static.
>
> Cleans up sparse warning:
> symbol 'tap_fops' was not declared. Should it be static?
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH 0/4] constify net platform_device_id
From: David Miller @ 2017-08-14 3:12 UTC (permalink / raw)
To: arvind.yadav.cs
Cc: wg, mkl, madalin.bucur, sergei.shtylyov, grygorii.strashko,
linux-kernel, netdev, linux-can
In-Reply-To: <1502622705-11169-1-git-send-email-arvind.yadav.cs@gmail.com>
From: Arvind Yadav <arvind.yadav.cs@gmail.com>
Date: Sun, 13 Aug 2017 16:41:44 +0530
> platform_device_id are not supposed to change at runtime. All functions
> working with platform_device_id provided by <linux/platform_device.h>
> work with const platform_device_id. So mark the non-const structs as const.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter
From: Samuel Mendoza-Jonas @ 2017-08-14 4:23 UTC (permalink / raw)
To: Joel Stanley
Cc: David S . Miller, netdev, Linux Kernel Mailing List,
OpenBMC Maillist, Benjamin Herrenschmidt, Gavin Shan,
Ratan K Gupta
In-Reply-To: <CACPK8XfD=4=Axh+waO0KMRMntLBBWrb86f92NgX134U72wofzw@mail.gmail.com>
On Mon, 2017-08-14 at 11:39 +0930, Joel Stanley wrote:
> On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas
> <sam@mendozajonas.com> wrote:
> > Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI
> > stack process new VLAN tags and configure the channel VLAN filter
> > appropriately.
> > Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent
> > for each one, meaning the ncsi_dev_state_config_svf state must be
> > repeated. An internal list of VLAN tags is maintained, and compared
> > against the current channel's ncsi_channel_filter in order to keep track
> > within the state. VLAN filters are removed in a similar manner, with the
> > introduction of the ncsi_dev_state_config_clear_vids state. The maximum
> > number of VLAN tag filters is determined by the "Get Capabilities"
> > response from the channel.
> >
> > Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
>
> I've given this some testing, but there are a few things I saw below
> that we should sort out.
>
> > --- a/net/ncsi/ncsi-manage.c
> > +++ b/net/ncsi/ncsi-manage.c
> > @@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table)
> > return sizes[table];
> > }
> >
> > +u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index)
> > +{
> > + struct ncsi_channel_filter *ncf;
> > + int size;
> > +
> > + ncf = nc->filters[table];
> > + if (!ncf)
> > + return NULL;
> > +
> > + size = ncsi_filter_size(table);
> > + if (size < 0)
> > + return NULL;
> > +
> > + return ncf->data + size * index;
> > +}
> > +
> > int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
> > {
> > struct ncsi_channel_filter *ncf;
> > @@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data)
> > index = -1;
> > while ((index = find_next_bit(bitmap, ncf->total, index + 1))
> > < ncf->total) {
> > - if (!memcmp(ncf->data + size * index, data, size)) {
> > + if (!data || !memcmp(ncf->data + size * index, data, size)) {
>
> Not clear why this check is required?
Right, this could use a comment. This is a small workaround to having a
way to finding an arbitrary active filter, below in clear_one_vid(). We
pass NULL as a way of saying "return any enabled filter".
>
> > spin_unlock_irqrestore(&nc->lock, flags);
> > return index;
> > }
> > @@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv *ndp)
> > nd->state = ncsi_dev_state_functional;
> > }
> >
> > +/* Check the VLAN filter bitmap for a set filter, and construct a
> > + * "Set VLAN Filter - Disable" packet if found.
> > + */
> > +static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
> > + struct ncsi_cmd_arg *nca)
> > +{
> > + int index;
> > + u16 vid;
> > +
> > + index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL);
> > + if (index < 0) {
> > + /* Filter table empty */
> > + return -1;
> > + }
> > +
> > + vid = *(u16 *)ncsi_get_filter(nc, NCSI_FILTER_VLAN, index);
>
> You just added this function that returns a pointer to a u32. It's
> strange to see the only call site then throw half of it away.
The problem here is that the single ncsi_channel_filter struct is used to
represent several different filters, and the filter data is stored in a
u32 data[] type. We cast to u16 in clear_one_vid because we know it's a
VLAN tag (NCSI_FILTER_VLAN), although we probably should call
ncsi_filter_size() to be sure.
>
> Also, ncsi_get_filter can return NULL.
That is indeed a problem though!
>
> > + netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> > + "ncsi: removed vlan tag %u at index %d\n",
> > + vid, index + 1);
> > + ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index);
> > +
> > + nca->type = NCSI_PKT_CMD_SVF;
> > + nca->words[1] = vid;
> > + /* HW filter index starts at 1 */
> > + nca->bytes[6] = index + 1;
> > + nca->bytes[7] = 0x00;
> > + return 0;
> > +}
> > @@ -751,6 +876,26 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> > break;
> > case ncsi_dev_state_config_done:
> > spin_lock_irqsave(&nc->lock, flags);
> > + if (nc->reconfigure_needed) {
> > + /* This channel's configuration has been updated
> > + * part-way during the config state - start the
> > + * channel configuration over
> > + */
> > + nc->reconfigure_needed = false;
> > + nc->state = NCSI_CHANNEL_INVISIBLE;
> > + spin_unlock_irqrestore(&nc->lock, flags);
> > +
> > + spin_lock_irqsave(&ndp->lock, flags);
> > + nc->state = NCSI_CHANNEL_INACTIVE;
>
> This looks strange. What's nc->state up to? Does setting it to
> NCSI_CHANNEL_INVISIBLE have any affect?
>
> The locking is confusing too.
The locking IS confusing!
Here I've followed existing convention from a similar case in ncsi-aen,
but I'm not convinced it's correct. Likely the following would still have
the desired effect:
nc->reconfigure_needed = false;
nc->state = NCSI_CHANNEL_INACTIVE;
spin_unlock_irqrestore(&nc->lock, flags);
spin_lock_irqsave(&ndp->lock, flags);
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
And something similar in ncsi_kick_channels().
Ideally I would ask the author Gavin, but in lieu of that I'll need to
pick it apart a bit to see if the ncsi-aen code is doing something
important or is just wrong.
>
> > + list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> > + spin_unlock_irqrestore(&ndp->lock, flags);
> > +
> > + netdev_printk(KERN_DEBUG, dev,
> > + "Dirty NCSI channel state reset\n");
> > + ncsi_process_next_channel(ndp);
> > + break;
> > + }
> > +
> > if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) {
> > hot_nc = nc;
> > nc->state = NCSI_CHANNEL_ACTIVE;
> > @@ -1191,6 +1336,149 @@ static struct notifier_block ncsi_inet6addr_notifier = {
> > };
> > #endif /* CONFIG_IPV6 */
> >
> > +static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
> > +{
> > + struct ncsi_dev *nd = &ndp->ndev;
> > + struct ncsi_channel *nc;
> > + struct ncsi_package *np;
> > + unsigned long flags;
> > + unsigned int n = 0;
> > +
> > + NCSI_FOR_EACH_PACKAGE(ndp, np) {
> > + NCSI_FOR_EACH_CHANNEL(np, nc) {
> > + spin_lock_irqsave(&nc->lock, flags);
> > +
> > + /* Channels may be busy, mark dirty instead of
> > + * kicking if;
> > + * a) not ACTIVE (configured)
> > + * b) in the channel_queue (to be configured)
> > + * c) it's ndev is in the config state
> > + */
> > + if (nc->state != NCSI_CHANNEL_ACTIVE) {
> > + if ((ndp->ndev.state & 0xff00) ==
> > + ncsi_dev_state_config ||
> > + !list_empty(&nc->link)) {
> > + netdev_printk(KERN_DEBUG, nd->dev,
> > + "ncsi: channel %p marked dirty\n",
> > + nc);
> > + nc->reconfigure_needed = true;
> > + }
> > + spin_unlock_irqrestore(&nc->lock, flags);
> > + continue;
> > + }
> > +
> > + spin_unlock_irqrestore(&nc->lock, flags);
> > +
> > + ncsi_stop_channel_monitor(nc);
> > + spin_lock_irqsave(&nc->lock, flags);
> > + nc->state = NCSI_CHANNEL_INVISIBLE;
> > + spin_unlock_irqrestore(&nc->lock, flags);
> > +
> > + spin_lock_irqsave(&ndp->lock, flags);
> > + nc->state = NCSI_CHANNEL_INACTIVE;
>
> This is a similar pattern to ncsi_configure_channel. I suspect the
> answer there will be the same as here.
>
> > + list_add_tail_rcu(&nc->link, &ndp->channel_queue);
> > + spin_unlock_irqrestore(&ndp->lock, flags);
> > +
> > + netdev_printk(KERN_DEBUG, nd->dev,
> > + "ncsi: kicked channel %p\n", nc);
> > + n++;
> > + }
> > + }
^ permalink raw reply
* Re: [PATCH net] ipv4: route: fix inet_rtm_getroute induced crash
From: David Ahern @ 2017-08-14 4:34 UTC (permalink / raw)
To: Florian Westphal, netdev; +Cc: Roopa Prabhu
In-Reply-To: <20170813225258.16001-1-fw@strlen.de>
On 8/13/17 4:52 PM, Florian Westphal wrote:
> "ip route get $daddr iif eth0 from $saddr" causes:
> BUG: KASAN: use-after-free in ip_route_input_rcu+0x1535/0x1b50
> Call Trace:
> ip_route_input_rcu+0x1535/0x1b50
> ip_route_input_noref+0xf9/0x190
> tcp_v4_early_demux+0x1a4/0x2b0
> ip_rcv+0xbcb/0xc05
> __netif_receive_skb+0x9c/0xd0
> netif_receive_skb_internal+0x5a8/0x890
>
> Problem is that inet_rtm_getroute calls either ip_route_input_rcu (if an
> iif was provided) or ip_route_output_key_hash_rcu.
>
> But ip_route_input_rcu, unlike ip_route_output_key_hash_rcu, already
> associates the dst_entry with the skb. This clears the SKB_DST_NOREF
> bit (i.e. skb_dst_drop will release/free the entry while it should not).
>
> Thus only set the dst if we called ip_route_output_key_hash_rcu().
>
> I tested this patch by running:
> while true;do ip r get 10.0.1.2;done > /dev/null &
> while true;do ip r get 10.0.1.2 iif eth0 from 10.0.1.1;done > /dev/null &
> ... and saw no crash or memory leak.
>
> Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
> Cc: David Ahern <dsahern@gmail.com>
> Fixes: ba52d61e0ff ("ipv4: route: restore skb_dst_set in inet_rtm_getroute")
> Signed-off-by: Florian Westphal <fw@strlen.de>
Have looked at the change in detail, but are you sure that is the
correct Fixes?
Running these:
while true;do ip r get 10.1.1.3;done > /dev/null &
while true;do ip r get 10.1.1.3 iif eth0 from 192.16.1.1;done >
/dev/null &
at various commits:
ffe95ecf3a2 - KASAN backtraces
374d801522f - works fine
ba52d61e0ff - negative refcnt messages
a5e2ee5da47 - works fine
^ permalink raw reply
* Re: [patch v1 1/2] Allow Mellanox network vendor to be configured if only I2C bus is configured
From: Leon Romanovsky @ 2017-08-14 5:20 UTC (permalink / raw)
To: David Miller; +Cc: ohado, netdev, jiri, saeedm, vadimp, system-sw-low-level
In-Reply-To: <20170813.172521.1720357048468458417.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 1879 bytes --]
On Sun, Aug 13, 2017 at 05:25:21PM -0700, David Miller wrote:
> From: Ohad Oz <ohado@mellanox.com>
> Date: Sun, 13 Aug 2017 15:26:56 +0000
>
> >
> >
> >> -----Original Message-----
> >> From: Leon Romanovsky [mailto:leon@kernel.org]
> >> Sent: Saturday, August 12, 2017 5:37 PM
> >> To: Ohad Oz <ohado@mellanox.com>
> >> Cc: davem@davemloft.net; netdev@vger.kernel.org; jiri@resnulli.us; Saeed
> >> Mahameed <saeedm@mellanox.com>; Vadim Pasternak
> >> <vadimp@mellanox.com>; system-sw-low-level <system-sw-low-
> >> level@mellanox.com>
> >> Subject: Re: [patch v1 1/2] Allow Mellanox network vendor to be configured
> >> if only I2C bus is configured
> >>
> >> On Thu, Aug 10, 2017 at 05:11:51PM +0000, Ohad Oz wrote:
> >> > Patch allows Mellanox devices on system with no PCI, but with I2C only.
> >> >
> >>
> >> Did you test mlx5 device on such system? Did it work for you?
> >
> > Yes, I did. With PCI config set to disable mlx5 drivers are not built.
> > Only the following:
> > /build/drivers/net/Ethernet/mellanox/mlxsw/mlxsw_core.ko
> > /build/drivers/net/Ethernet/mellanox /mlxsw/mlxsw_i2c.ko
> > /build/drivers/net/Ethernet/mellanox /mlxsw/mlxsw_minimal.ko
> >
> > While with both options on all drivers are built inc mlx5.
>
> I'm not so sure he's interested if things build or not.
>
> He's asking you if you actually used a Mellanox device with this
> driver with PCI disabled and only I2C available.
Thanks David, you are absolutely right.
The commit message and more important, Kconfig entry, is misleading the potential
users by promising them that all devices under drivers/net/ethernet/mellanox
folder are working with i2c.
I have no technical objection to the idea of this patch, but it needs to be resubmitted after
Ohad ensures that mlx4/mlx5 are not visible in menuconfig for i2c
systems and he fixes Kconfig description together with commit log.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net] ipv4: route: fix inet_rtm_getroute induced crash
From: Florian Westphal @ 2017-08-14 5:20 UTC (permalink / raw)
To: David Ahern; +Cc: Florian Westphal, netdev, Roopa Prabhu
In-Reply-To: <17031977-3e7a-9a7f-e53c-538cb030e496@gmail.com>
David Ahern <dsahern@gmail.com> wrote:
> On 8/13/17 4:52 PM, Florian Westphal wrote:
> > "ip route get $daddr iif eth0 from $saddr" causes:
> > BUG: KASAN: use-after-free in ip_route_input_rcu+0x1535/0x1b50
> > Call Trace:
> > ip_route_input_rcu+0x1535/0x1b50
> > ip_route_input_noref+0xf9/0x190
> > tcp_v4_early_demux+0x1a4/0x2b0
> > ip_rcv+0xbcb/0xc05
> > __netif_receive_skb+0x9c/0xd0
> > netif_receive_skb_internal+0x5a8/0x890
> >
> > Problem is that inet_rtm_getroute calls either ip_route_input_rcu (if an
> > iif was provided) or ip_route_output_key_hash_rcu.
> >
> > But ip_route_input_rcu, unlike ip_route_output_key_hash_rcu, already
> > associates the dst_entry with the skb. This clears the SKB_DST_NOREF
> > bit (i.e. skb_dst_drop will release/free the entry while it should not).
> >
> > Thus only set the dst if we called ip_route_output_key_hash_rcu().
> >
> > I tested this patch by running:
> > while true;do ip r get 10.0.1.2;done > /dev/null &
> > while true;do ip r get 10.0.1.2 iif eth0 from 10.0.1.1;done > /dev/null &
> > ... and saw no crash or memory leak.
> >
> > Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
> > Cc: David Ahern <dsahern@gmail.com>
> > Fixes: ba52d61e0ff ("ipv4: route: restore skb_dst_set in inet_rtm_getroute")
> > Signed-off-by: Florian Westphal <fw@strlen.de>
>
> Have looked at the change in detail, but are you sure that is the
> correct Fixes?
I'm reasonably sure, yes:
if (iif) {
ip_route_input_rcu // 1 might get NOREF dst
} else {
ip_route_output_key_hash_rcu // 2 always takes dst ref
}
skb_dst_set /* 3 loses NOREF in case of 1) */
> Running these:
> while true;do ip r get 10.1.1.3;done > /dev/null &
> while true;do ip r get 10.1.1.3 iif eth0 from 192.16.1.1;done >
> /dev/null &
>
> at various commits:
> ffe95ecf3a2 - KASAN backtraces
Right, this is broken state (has both ba52d61e0ff and 3765d35ed8b9)
> 374d801522f - works fine
This is fine, it lacks 3765d35ed8b9:
both branches take a reference on dst so '3' above has no side effect.
> ba52d61e0ff - negative refcnt messages
AFAICS this is before dst gc removal, I guess (but did not
check) that KASAN vs. refcount just comes from this.
> a5e2ee5da47 - works fine
Should cause a memory leak when iif is not given (ref on dst is
taken but not released in case of 2), ba52d61e0ff cured this but
adds the problem described here).
Does that make it clearer?
^ 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