lkml.org 
[lkml]   [2012]   [Jul]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC 2/2] net: Add support for NTB virtual ethernet device
    On Sat, Jul 14, 2012 at 01:14:03AM +0200, Jiri Pirko wrote:
    > Fri, Jul 13, 2012 at 11:45:00PM CEST, jon.mason@intel.com wrote:
    > >A virtual ethernet device that uses the NTB transport API to send/receive data.
    > >
    > >Signed-off-by: Jon Mason <jon.mason@intel.com>
    > >---
    > > drivers/net/Kconfig | 4 +
    > > drivers/net/Makefile | 1 +
    > > drivers/net/ntb_netdev.c | 411 ++++++++++++++++++++++++++++++++++++++++++++++
    > > 3 files changed, 416 insertions(+), 0 deletions(-)
    > > create mode 100644 drivers/net/ntb_netdev.c
    > >
    > >diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
    > >index 0c2bd80..9bf8a71 100644
    > >--- a/drivers/net/Kconfig
    > >+++ b/drivers/net/Kconfig
    > >@@ -178,6 +178,10 @@ config NETPOLL_TRAP
    > > config NET_POLL_CONTROLLER
    > > def_bool NETPOLL
    > >
    > >+config NTB_NETDEV
    > >+ tristate "Virtual Ethernet over NTB"
    > >+ depends on NTB
    > >+
    > > config RIONET
    > > tristate "RapidIO Ethernet over messaging driver support"
    > > depends on RAPIDIO
    > >diff --git a/drivers/net/Makefile b/drivers/net/Makefile
    > >index 3d375ca..9890148 100644
    > >--- a/drivers/net/Makefile
    > >+++ b/drivers/net/Makefile
    > >@@ -69,3 +69,4 @@ obj-$(CONFIG_USB_IPHETH) += usb/
    > > obj-$(CONFIG_USB_CDC_PHONET) += usb/
    > >
    > > obj-$(CONFIG_HYPERV_NET) += hyperv/
    > >+obj-$(CONFIG_NTB_NETDEV) += ntb_netdev.o
    > >diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
    > >new file mode 100644
    > >index 0000000..bcbd9d4
    > >--- /dev/null
    > >+++ b/drivers/net/ntb_netdev.c
    > >@@ -0,0 +1,411 @@
    > >+/*
    > >+ * This file is provided under a dual BSD/GPLv2 license. When using or
    > >+ * redistributing this file, you may do so under either license.
    > >+ *
    > >+ * GPL LICENSE SUMMARY
    > >+ *
    > >+ * Copyright(c) 2012 Intel Corporation. All rights reserved.
    > >+ *
    > >+ * This program is free software; you can redistribute it and/or modify
    > >+ * it under the terms of version 2 of the GNU General Public License as
    > >+ * published by the Free Software Foundation.
    > >+ *
    > >+ * This program is distributed in the hope that it will be useful, but
    > >+ * WITHOUT ANY WARRANTY; without even the implied warranty of
    > >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
    > >+ * General Public License for more details.
    > >+ *
    > >+ * You should have received a copy of the GNU General Public License
    > >+ * along with this program; if not, write to the Free Software
    > >+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
    > >+ * The full GNU General Public License is included in this distribution
    > >+ * in the file called LICENSE.GPL.
    > >+ *
    > >+ * BSD LICENSE
    > >+ *
    > >+ * Copyright(c) 2012 Intel Corporation. All rights reserved.
    > >+ *
    > >+ * Redistribution and use in source and binary forms, with or without
    > >+ * modification, are permitted provided that the following conditions
    > >+ * are met:
    > >+ *
    > >+ * * Redistributions of source code must retain the above copyright
    > >+ * notice, this list of conditions and the following disclaimer.
    > >+ * * Redistributions in binary form must reproduce the above copy
    > >+ * notice, this list of conditions and the following disclaimer in
    > >+ * the documentation and/or other materials provided with the
    > >+ * distribution.
    > >+ * * Neither the name of Intel Corporation nor the names of its
    > >+ * contributors may be used to endorse or promote products derived
    > >+ * from this software without specific prior written permission.
    > >+ *
    > >+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
    > >+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
    > >+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
    > >+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
    > >+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
    > >+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
    > >+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
    > >+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
    > >+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
    > >+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
    > >+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    > >+ *
    > >+ * Intel PCIe NTB Network Linux driver
    > >+ *
    > >+ * Contact Information:
    > >+ * Jon Mason <jon.mason@intel.com>
    > >+ */
    > >+#include <linux/etherdevice.h>
    > >+#include <linux/ethtool.h>
    > >+#include <linux/module.h>
    > >+#include <linux/ntb.h>
    > >+
    > >+#define NTB_NETDEV_VER "0.4"
    >
    > Is it really necessary to provide this in-file versioning? Doesn't
    > kernel version itself do the trick?

    Not necessarily. This may be distributed as a package outside of the kernel and the version is useful for debug.

    >
    > >+
    > >+MODULE_DESCRIPTION(KBUILD_MODNAME);
    > >+MODULE_VERSION(NTB_NETDEV_VER);
    > >+MODULE_LICENSE("Dual BSD/GPL");
    > >+MODULE_AUTHOR("Intel Corporation");
    > >+
    > >+struct ntb_netdev {
    > >+ struct net_device *ndev;
    > >+ struct ntb_transport_qp *qp;
    > >+};
    > >+
    > >+#define NTB_TX_TIMEOUT_MS 1000
    > >+#define NTB_RXQ_SIZE 100
    > >+
    > >+static struct net_device *netdev;
    > >+
    > >+static void ntb_netdev_event_handler(int status)
    > >+{
    > >+ struct ntb_netdev *dev = netdev_priv(netdev);
    > >+
    > >+ pr_debug("%s: Event %x, Link %x\n", KBUILD_MODNAME, status,
    > >+ ntb_transport_link_query(dev->qp));
    > >+
    > >+ /* Currently, only link status event is supported */
    > >+ if (status)
    > >+ netif_carrier_on(netdev);
    > >+ else
    > >+ netif_carrier_off(netdev);
    > >+}
    > >+
    > >+static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp)
    > >+{
    > >+ struct net_device *ndev = netdev;
    > >+ struct sk_buff *skb;
    > >+ int len, rc;
    > >+
    > >+ while ((skb = ntb_transport_rx_dequeue(qp, &len))) {
    > >+ pr_debug("%s: %d byte payload received\n", __func__, len);
    > >+
    > >+ skb_put(skb, len);
    > >+ skb->protocol = eth_type_trans(skb, ndev);
    > >+ skb->ip_summed = CHECKSUM_NONE;
    > >+
    > >+ if (netif_rx(skb) == NET_RX_DROP) {
    > >+ ndev->stats.rx_errors++;
    > >+ ndev->stats.rx_dropped++;
    > >+ } else {
    > >+ ndev->stats.rx_packets++;
    > >+ ndev->stats.rx_bytes += len;
    > >+ }
    > >+
    > >+ skb = netdev_alloc_skb(ndev, ndev->mtu + ETH_HLEN);
    > >+ if (!skb) {
    > >+ ndev->stats.rx_errors++;
    > >+ ndev->stats.rx_frame_errors++;
    > >+ pr_err("%s: No skb\n", __func__);
    > >+ break;
    > >+ }
    > >+
    > >+ rc = ntb_transport_rx_enqueue(qp, skb, skb->data,
    > >+ ndev->mtu + ETH_HLEN);
    > >+ if (rc) {
    > >+ ndev->stats.rx_errors++;
    > >+ ndev->stats.rx_fifo_errors++;
    > >+ pr_err("%s: error re-enqueuing\n", __func__);
    > >+ break;
    > >+ }
    > >+ }
    > >+}
    > >+
    > >+static void ntb_netdev_tx_handler(struct ntb_transport_qp *qp)
    > >+{
    > >+ struct net_device *ndev = netdev;
    > >+ struct sk_buff *skb;
    > >+ int len;
    > >+
    > >+ while ((skb = ntb_transport_tx_dequeue(qp, &len))) {
    > >+ ndev->stats.tx_packets++;
    > >+ ndev->stats.tx_bytes += skb->len;
    > >+ dev_kfree_skb(skb);
    > >+ }
    > >+
    > >+ if (netif_queue_stopped(ndev))
    > >+ netif_wake_queue(ndev);
    > >+}
    > >+
    > >+static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
    > >+ struct net_device *ndev)
    > >+{
    > >+ struct ntb_netdev *dev = netdev_priv(ndev);
    > >+ int rc;
    > >+
    > >+ pr_debug("%s: ntb_transport_tx_enqueue\n", KBUILD_MODNAME);
    > >+
    > >+ rc = ntb_transport_tx_enqueue(dev->qp, skb, skb->data, skb->len);
    > >+ if (rc)
    > >+ goto err;
    > >+
    > >+ return NETDEV_TX_OK;
    > >+
    > >+err:
    > >+ ndev->stats.tx_dropped++;
    > >+ ndev->stats.tx_errors++;
    > >+ netif_stop_queue(ndev);
    > >+ return NETDEV_TX_BUSY;
    > >+}
    > >+
    > >+static int ntb_netdev_open(struct net_device *ndev)
    > >+{
    > >+ struct ntb_netdev *dev = netdev_priv(ndev);
    > >+ struct sk_buff *skb;
    > >+ int rc, i, len;
    > >+
    > >+ /* Add some empty rx bufs */
    > >+ for (i = 0; i < NTB_RXQ_SIZE; i++) {
    > >+ skb = netdev_alloc_skb(ndev, ndev->mtu + ETH_HLEN);
    > >+ if (!skb) {
    > >+ rc = -ENOMEM;
    > >+ goto err;
    > >+ }
    > >+
    > >+ rc = ntb_transport_rx_enqueue(dev->qp, skb, skb->data,
    > >+ ndev->mtu + ETH_HLEN);
    > >+ if (rc == -EINVAL)
    > >+ goto err;
    > >+ }
    > >+
    > >+ netif_carrier_off(ndev);
    > >+ ntb_transport_link_up(dev->qp);
    > >+
    > >+ return 0;
    > >+
    > >+err:
    > >+ while ((skb = ntb_transport_rx_remove(dev->qp, &len)))
    > >+ kfree(skb);
    > >+ return rc;
    > >+}
    > >+
    > >+static int ntb_netdev_close(struct net_device *ndev)
    > >+{
    > >+ struct ntb_netdev *dev = netdev_priv(ndev);
    > >+ struct sk_buff *skb;
    > >+ int len;
    > >+
    > >+ ntb_transport_link_down(dev->qp);
    > >+
    > >+ while ((skb = ntb_transport_rx_remove(dev->qp, &len)))
    > >+ kfree(skb);
    > >+
    > >+ return 0;
    > >+}
    > >+
    > >+static int ntb_netdev_change_mtu(struct net_device *ndev, int new_mtu)
    > >+{
    > >+ struct ntb_netdev *dev = netdev_priv(ndev);
    > >+ struct sk_buff *skb;
    > >+ int len, rc;
    > >+
    > >+ if (new_mtu > ntb_transport_max_size(dev->qp) - ETH_HLEN)
    > >+ return -EINVAL;
    > >+
    > >+ if (!netif_running(ndev)) {
    > >+ ndev->mtu = new_mtu;
    > >+ return 0;
    > >+ }
    > >+
    > >+ /* Bring down the link and dispose of posted rx entries */
    > >+ ntb_transport_link_down(dev->qp);
    > >+
    > >+ if (ndev->mtu < new_mtu) {
    > >+ int i;
    > >+
    > >+ for (i = 0; (skb = ntb_transport_rx_remove(dev->qp, &len)); i++)
    > >+ kfree(skb);
    > >+
    > >+ for (; i; i--) {
    > >+ skb = netdev_alloc_skb(ndev, new_mtu + ETH_HLEN);
    > >+ if (!skb) {
    > >+ rc = -ENOMEM;
    > >+ goto err;
    > >+ }
    > >+
    > >+ rc = ntb_transport_rx_enqueue(dev->qp, skb, skb->data,
    > >+ new_mtu + ETH_HLEN);
    > >+ if (rc) {
    > >+ kfree(skb);
    > >+ goto err;
    > >+ }
    > >+ }
    > >+ }
    > >+
    > >+ ndev->mtu = new_mtu;
    > >+
    > >+ ntb_transport_link_up(dev->qp);
    > >+
    > >+ return 0;
    > >+
    > >+err:
    > >+ ntb_transport_link_down(dev->qp);
    > >+
    > >+ while ((skb = ntb_transport_rx_remove(dev->qp, &len)))
    > >+ kfree(skb);
    > >+
    > >+ pr_err("Error changing MTU, device inoperable\n");
    >
    > Would be maybe better to use netdev_err here (and on similar other
    > places)

    Good point

    >
    > Also, it might be good to provide rollback in case any of
    > netdev_alloc_skb() fails.
    >
    > >+ return rc;
    > >+}
    > >+
    > >+static void ntb_netdev_tx_timeout(struct net_device *ndev)
    > >+{
    > >+ if (netif_running(ndev))
    > >+ netif_wake_queue(ndev);
    > >+}
    > >+
    > >+static const struct net_device_ops ntb_netdev_ops = {
    > >+ .ndo_open = ntb_netdev_open,
    > >+ .ndo_stop = ntb_netdev_close,
    > >+ .ndo_start_xmit = ntb_netdev_start_xmit,
    > >+ .ndo_change_mtu = ntb_netdev_change_mtu,
    > >+ .ndo_tx_timeout = ntb_netdev_tx_timeout,
    > >+ .ndo_set_mac_address = eth_mac_addr,
    >
    > Does your device support mac change while it's up and running?

    It's virtual ethernet, so there is no hardware limitation, only what is acceptable for the remote side to receive.

    >
    > >+};
    > >+
    > >+static void ntb_get_drvinfo(__attribute__((unused)) struct net_device *dev,
    > >+ struct ethtool_drvinfo *info)
    > >+{
    > >+ strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
    > >+ strlcpy(info->version, NTB_NETDEV_VER, sizeof(info->version));
    > >+}
    > >+
    > >+static const char ntb_nic_stats[][ETH_GSTRING_LEN] = {
    > >+ "rx_packets", "rx_bytes", "rx_errors", "rx_dropped", "rx_length_errors",
    > >+ "rx_frame_errors", "rx_fifo_errors",
    > >+ "tx_packets", "tx_bytes", "tx_errors", "tx_dropped",
    > >+};
    > >+
    > >+static int ntb_get_stats_count(__attribute__((unused)) struct net_device *dev)
    > >+{
    > >+ return ARRAY_SIZE(ntb_nic_stats);
    > >+}
    > >+
    > >+static int ntb_get_sset_count(struct net_device *dev, int sset)
    > >+{
    > >+ switch (sset) {
    > >+ case ETH_SS_STATS:
    > >+ return ntb_get_stats_count(dev);
    > >+ default:
    > >+ return -EOPNOTSUPP;
    > >+ }
    > >+}
    > >+
    > >+static void ntb_get_strings(__attribute__((unused)) struct net_device *dev,
    > >+ u32 sset, u8 *data)
    > >+{
    > >+ switch (sset) {
    > >+ case ETH_SS_STATS:
    > >+ memcpy(data, *ntb_nic_stats, sizeof(ntb_nic_stats));
    > >+ }
    > >+}
    > >+
    > >+static void
    > >+ntb_get_ethtool_stats(struct net_device *dev,
    > >+ __attribute__((unused)) struct ethtool_stats *stats,
    > >+ u64 *data)
    > >+{
    > >+ int i = 0;
    > >+
    > >+ data[i++] = dev->stats.rx_packets;
    > >+ data[i++] = dev->stats.rx_bytes;
    > >+ data[i++] = dev->stats.rx_errors;
    > >+ data[i++] = dev->stats.rx_dropped;
    > >+ data[i++] = dev->stats.rx_length_errors;
    > >+ data[i++] = dev->stats.rx_frame_errors;
    > >+ data[i++] = dev->stats.rx_fifo_errors;
    > >+ data[i++] = dev->stats.tx_packets;
    > >+ data[i++] = dev->stats.tx_bytes;
    > >+ data[i++] = dev->stats.tx_errors;
    > >+ data[i++] = dev->stats.tx_dropped;
    > >+}
    > >+
    > >+static const struct ethtool_ops ntb_ethtool_ops = {
    > >+ .get_drvinfo = ntb_get_drvinfo,
    > >+ .get_sset_count = ntb_get_sset_count,
    > >+ .get_strings = ntb_get_strings,
    > >+ .get_ethtool_stats = ntb_get_ethtool_stats,
    > >+ .get_link = ethtool_op_get_link,
    > >+};
    > >+
    > >+static int __init ntb_netdev_init_module(void)
    > >+{
    > >+ struct ntb_netdev *dev;
    > >+ int rc;
    > >+
    > >+ pr_info("%s: Probe\n", KBUILD_MODNAME);
    > >+
    > >+ netdev = alloc_etherdev(sizeof(struct ntb_netdev));
    >
    > I might be missing something but this place (module init) does not seems
    > like a good place to do alloc_etherdev(). Do you want to support only
    > one netdevice instance?
    >
    > Anyway, I think that using "static netdev" should be avoided in any case.
    >

    It would fail the probe if there is no underlying ntb hardware, but it would make sense to check for that before allocing the etherdev.

    Thanks for the comments!


    > >+ if (!netdev)
    > >+ return -ENOMEM;
    > >+
    > >+ dev = netdev_priv(netdev);
    > >+ dev->ndev = netdev;
    > >+ netdev->features = NETIF_F_HIGHDMA;
    > >+
    > >+ netdev->hw_features = netdev->features;
    > >+ netdev->watchdog_timeo = msecs_to_jiffies(NTB_TX_TIMEOUT_MS);
    > >+
    > >+ random_ether_addr(netdev->perm_addr);
    > >+ memcpy(netdev->dev_addr, netdev->perm_addr, netdev->addr_len);
    > >+
    > >+ netdev->netdev_ops = &ntb_netdev_ops;
    > >+ SET_ETHTOOL_OPS(netdev, &ntb_ethtool_ops);
    > >+
    > >+ dev->qp = ntb_transport_create_queue(ntb_netdev_rx_handler,
    > >+ ntb_netdev_tx_handler,
    > >+ ntb_netdev_event_handler);
    > >+ if (!dev->qp) {
    > >+ rc = -EIO;
    > >+ goto err;
    > >+ }
    > >+
    > >+ netdev->mtu = ntb_transport_max_size(dev->qp) - ETH_HLEN;
    > >+
    > >+ rc = register_netdev(netdev);
    > >+ if (rc)
    > >+ goto err1;
    > >+
    > >+ pr_info("%s: %s created\n", KBUILD_MODNAME, netdev->name);
    > >+ return 0;
    > >+
    > >+err1:
    > >+ ntb_transport_free_queue(dev->qp);
    > >+err:
    > >+ free_netdev(netdev);
    > >+ return rc;
    > >+}
    > >+module_init(ntb_netdev_init_module);
    > >+
    > >+static void __exit ntb_netdev_exit_module(void)
    > >+{
    > >+ struct ntb_netdev *dev = netdev_priv(netdev);
    > >+
    > >+ unregister_netdev(netdev);
    > >+ ntb_transport_free_queue(dev->qp);
    > >+ free_netdev(netdev);
    > >+
    > >+ pr_info("%s: Driver removed\n", KBUILD_MODNAME);
    > >+}
    > >+module_exit(ntb_netdev_exit_module);
    > >--
    > >1.7.5.4
    > >
    > >--
    > >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


    \
     
     \ /
      Last update: 2012-07-14 08:21    [W:0.064 / U:152.540 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site