[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAE4R7bAk_nUCuqcJxuAAr6pOqNX7zXWv1zFZDVGwBi7_nk8=kQ@mail.gmail.com>
Date: Sat, 25 Jul 2015 19:45:07 -0700
From: Scott Feldman <sfeldma@...il.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: Netdev <netdev@...r.kernel.org>,
"David S. Miller" <davem@...emloft.net>, idosch@...lanox.com,
eladr@...lanox.com,
"ogerlitz@...lanox.com" <ogerlitz@...lanox.com>,
Roopa Prabhu <roopa@...ulusnetworks.com>,
Florian Fainelli <f.fainelli@...il.com>,
Thomas Graf <tgraf@...g.ch>,
Alexei Starovoitov <ast@...mgrid.com>,
Jamal Hadi Salim <jhs@...atatu.com>,
Daniel Borkmann <daniel@...earbox.net>,
john fastabend <john.fastabend@...il.com>,
"simon.horman@...ronome.com" <simon.horman@...ronome.com>,
John Linville <linville@...driver.com>,
Andy Gospodarek <andy@...yhouse.net>,
Shrijeet Mukherjee <shm@...ulusnetworks.com>,
"nhorman@...driver.com" <nhorman@...driver.com>,
Jiri Pirko <jiri@...lanox.com>
Subject: Re: [patch net-next 4/4] mlxsw: Introduce Mellanox SwitchX-2 ASIC support
On Thu, Jul 23, 2015 at 8:43 AM, Jiri Pirko <jiri@...nulli.us> wrote:
> From: Jiri Pirko <jiri@...lanox.com>
>
> Benefit from the previously introduced Mellanox Switch infrastructure and
> add driver for SwitchX-2 ASIC. Note that this driver is very simple now.
> It implements bare minimum for getting device to work on slow-path.
> Fast-path offload functionality is going to be added soon.
>
> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
> Signed-off-by: Ido Schimmel <idosch@...lanox.com>
> Signed-off-by: Elad Raz <eladr@...lanox.com>
[cut]
> +static netdev_tx_t mlxsw_sx_port_xmit(struct sk_buff *skb,
> + struct net_device *dev)
> +{
> + struct mlxsw_sx_port *mlxsw_sx_port = netdev_priv(dev);
> + struct mlxsw_sx *mlxsw_sx = mlxsw_sx_port->mlxsw_sx;
> + struct mlxsw_sx_port_pcpu_stats *pcpu_stats;
> + const struct mlxsw_tx_info tx_info = {
> + .local_port = mlxsw_sx_port->local_port,
> + .is_emad = false,
> + };
> + int err;
> +
> + if (unlikely(skb_headroom(skb) < MLXSW_TXHDR_LEN)) {
Does this happen at all since dev->hard_header_len was set in probe to
add MLXSW_TXHDR_LEN?
> + struct sk_buff *skb_new;
> +
> + skb_new = skb_realloc_headroom(skb, MLXSW_TXHDR_LEN);
> + dev_kfree_skb_any(skb);
> + if (!skb_new) {
> + this_cpu_inc(mlxsw_sx_port->pcpu_stats->tx_dropped);
> + return NETDEV_TX_OK;
> + }
> + skb = skb_new;
> + }
> + mlxsw_sx_txhdr_construct(skb, &tx_info);
> + err = mlxsw_core_skb_transmit(mlxsw_sx, skb, &tx_info);
> + if (err == -EAGAIN)
> + return NETDEV_TX_BUSY;
I think there is a problem here when returning NETDEV_TX_BUSY when
original skb might have been freed above in the headroom check. (ref
Documentation/networking/driver.txt).
[cut]
> +static int mlxsw_sx_port_dev_addr_get(struct mlxsw_sx_port *mlxsw_sx_port)
> +{
> + struct mlxsw_sx *mlxsw_sx = mlxsw_sx_port->mlxsw_sx;
> + struct net_device *dev = mlxsw_sx_port->dev;
> + char ppad_pl[MLXSW_REG_PPAD_LEN];
> + int err;
> +
> + mlxsw_reg_ppad_pack(ppad_pl, false, 0);
> + err = mlxsw_reg_query(mlxsw_sx->core, MLXSW_REG(ppad), ppad_pl);
> + if (err)
> + return err;
> + mlxsw_reg_ppad_mac_memcpy_from(ppad_pl, dev->dev_addr);
> + /* The last byte in base mac address is always 0 */
> + dev->dev_addr[ETH_ALEN - 1] += mlxsw_sx_port->local_port;
If MLXSW_PORT_MAX_PORTS > 256, you'll wrap this. Is dev_addr[ETH_ALEN
- 2] available to carry into?
> + return 0;
> +}
> +
[cut]
> +static int mlxsw_sx_port_create(struct mlxsw_sx *mlxsw_sx, u8 local_port)
> +{
> + struct mlxsw_sx_port *mlxsw_sx_port;
> + struct net_device *dev;
> + bool usable;
> + int err;
> +
> + dev = alloc_etherdev(sizeof(struct mlxsw_sx_port));
> + if (!dev)
> + return -ENOMEM;
> + mlxsw_sx_port = netdev_priv(dev);
> + mlxsw_sx_port->dev = dev;
> + mlxsw_sx_port->mlxsw_sx = mlxsw_sx;
> + mlxsw_sx_port->local_port = local_port;
> +
> + mlxsw_sx_port->pcpu_stats =
> + netdev_alloc_pcpu_stats(struct mlxsw_sx_port_pcpu_stats);
> + if (!mlxsw_sx_port->pcpu_stats) {
> + err = -ENOMEM;
> + goto err_alloc_stats;
> + }
> +
> + dev->netdev_ops = &mlxsw_sx_port_netdev_ops;
> + dev->ethtool_ops = &mlxsw_sx_port_ethtool_ops;
> + dev->switchdev_ops = &mlxsw_sx_port_switchdev_ops;
> +
> + err = mlxsw_sx_port_dev_addr_get(mlxsw_sx_port);
> + if (err) {
> + dev_err(mlxsw_sx->bus_info->dev, "Port %d: Unable to get port mac address\n",
> + mlxsw_sx_port->local_port);
> + goto err_dev_addr_get;
> + }
> +
> + netif_carrier_off(dev);
> +
> + dev->features |= NETIF_F_NETNS_LOCAL | NETIF_F_LLTX |
Not supposed to use LLTX in new drivers, according to
include/linux/netdev_features.h.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists