[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <55a01b55.88da8c0a.8b52.ffffe04e@mx.google.com>
Date: Fri, 10 Jul 2015 12:21:57 -0700 (PDT)
From: Yasuaki Ishimatsu <yasu.isimatu@...il.com>
To: Taku Izumi <izumi.taku@...fujitsu.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net,
platform-driver-x86@...r.kernel.org, dvhart@...radead.org,
rkhan@...hat.com, alexander.h.duyck@...hat.com,
linux-acpi@...r.kernel.org, joe@...ches.com,
sergei.shtylyov@...entembedded.com, stephen@...workplumber.org,
Yasuaki Ishimatsu <yasu.isimatu@...il.com>
Subject: Re: [PATCH v2 08/22] fjes: net_device_ops.ndo_start_xmit
Hi Izumi-san,
On Wed, 24 Jun 2015 11:55:40 +0900
Taku Izumi <izumi.taku@...fujitsu.com> wrote:
> This patch adds net_device_ops.ndo_start_xmit callback,
> which is called when sending packets.
>
> Signed-off-by: Taku Izumi <izumi.taku@...fujitsu.com>
> ---
> drivers/net/fjes/fjes.h | 1 +
> drivers/net/fjes/fjes_hw.c | 58 ++++++++++++++
> drivers/net/fjes/fjes_hw.h | 12 +++
> drivers/net/fjes/fjes_main.c | 177 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 248 insertions(+)
>
> diff --git a/drivers/net/fjes/fjes.h b/drivers/net/fjes/fjes.h
> index f182ed3..7af4304 100644
> --- a/drivers/net/fjes/fjes.h
> +++ b/drivers/net/fjes/fjes.h
> @@ -29,6 +29,7 @@
> #define FJES_ACPI_SYMBOL "Extended Socket"
> #define FJES_MAX_QUEUES 1
> #define FJES_TX_RETRY_INTERVAL (20 * HZ)
> +#define FJES_TX_RETRY_TIMEOUT (100)
> #define FJES_OPEN_ZONE_UPDATE_WAIT (300) /* msec */
>
> /* board specific private data structure */
> diff --git a/drivers/net/fjes/fjes_hw.c b/drivers/net/fjes/fjes_hw.c
> index a17cb96..b2e7509 100644
> --- a/drivers/net/fjes/fjes_hw.c
> +++ b/drivers/net/fjes/fjes_hw.c
> @@ -793,3 +793,61 @@ int fjes_hw_wait_epstop(struct fjes_hw *hw)
> return (wait_time < FJES_COMMAND_EPSTOP_WAIT_TIMEOUT * 1000)
> ? 0 : -EBUSY;
> }
> +
> +bool fjes_hw_check_epbuf_version(struct epbuf_handler *epbh, u32 version)
> +{
> + union ep_buffer_info *info = epbh->info;
> +
> + return (info->common.version == version);
> +}
> +
> +bool fjes_hw_check_mtu(struct epbuf_handler *epbh, u32 mtu)
> +{
> + union ep_buffer_info *info = epbh->info;
> +
> + return (info->v1i.frame_max == FJES_MTU_TO_FRAME_SIZE(mtu));
> +}
> +
> +bool fjes_hw_check_vlan_id(struct epbuf_handler *epbh, u16 vlan_id)
> +{
> + union ep_buffer_info *info = epbh->info;
> + int i;
> + bool ret = false;
> +
> + if (vlan_id == 0) {
> + ret = true;
> + } else {
> + for (i = 0; i < EP_BUFFER_SUPPORT_VLAN_MAX; i++) {
> + if (vlan_id == info->v1i.vlan_id[i]) {
> + ret = true;
> + break;
> + }
> + }
> + }
> + return ret;
> +}
> +
> +int fjes_hw_epbuf_tx_pkt_send(struct epbuf_handler *epbh,
> + void *frame, size_t size)
> +{
> + union ep_buffer_info *info = epbh->info;
> + struct esmem_frame *ring_frame;
> +
> + if (EP_RING_FULL(info->v1i.head, info->v1i.tail, info->v1i.count_max))
> + return -ENOBUFS;
> +
> + ring_frame =
> + (struct esmem_frame *)&(epbh->
> + ring[EP_RING_INDEX
> + (info->v1i.tail - 1,
> + info->v1i.count_max) *
> + info->v1i.frame_max]);
> +
> + ring_frame->frame_size = size;
> + memcpy((void *)(ring_frame->frame_data), (void *)frame, size);
> +
> + EP_RING_INDEX_INC(epbh->info->v1i.tail, info->v1i.count_max);
> +
> + return 0;
> +}
> +
> diff --git a/drivers/net/fjes/fjes_hw.h b/drivers/net/fjes/fjes_hw.h
> index 823d802..95b9732 100644
> --- a/drivers/net/fjes/fjes_hw.h
> +++ b/drivers/net/fjes/fjes_hw.h
> @@ -49,6 +49,9 @@ struct fjes_hw;
>
> #define FJES_ZONING_ZONE_TYPE_NONE (0xFF)
>
> +#define FJES_TX_DELAY_SEND_NONE (0)
> +#define FJES_TX_DELAY_SEND_PENDING (1)
> +
> #define FJES_RX_STOP_REQ_NONE (0x0)
> #define FJES_RX_STOP_REQ_DONE (0x1)
> #define FJES_RX_STOP_REQ_REQUEST (0x2)
> @@ -60,6 +63,11 @@ struct fjes_hw;
>
> #define EP_RING_NUM(buffer_size, frame_size) \
> (u32)((buffer_size) / (frame_size))
> +#define EP_RING_INDEX(_num, _max) (((_num) + (_max)) % (_max))
> +#define EP_RING_INDEX_INC(_num, _max) \
> + ((_num) = EP_RING_INDEX((_num) + 1, (_max)))
> +#define EP_RING_FULL(_head, _tail, _max) \
> + (0 == EP_RING_INDEX(((_tail) - (_head)), (_max)))
>
> #define FJES_MTU_TO_BUFFER_SIZE(mtu) \
> (ETH_HLEN + VLAN_HLEN + (mtu) + ETH_FCS_LEN)
> @@ -308,5 +316,9 @@ enum ep_partner_status
>
> bool fjes_hw_epid_is_same_zone(struct fjes_hw *, int);
> int fjes_hw_epid_is_shared(struct fjes_device_shared_info *, int);
> +bool fjes_hw_check_epbuf_version(struct epbuf_handler *, u32);
> +bool fjes_hw_check_mtu(struct epbuf_handler *, u32);
> +bool fjes_hw_check_vlan_id(struct epbuf_handler *, u16);
> +int fjes_hw_epbuf_tx_pkt_send(struct epbuf_handler *, void *, size_t);
>
> #endif /* FJES_HW_H_ */
> diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
> index 832439b..f2ef286 100644
> --- a/drivers/net/fjes/fjes_main.c
> +++ b/drivers/net/fjes/fjes_main.c
> @@ -51,6 +51,7 @@ static int fjes_open(struct net_device *);
> static int fjes_close(struct net_device *);
> static int fjes_setup_resources(struct fjes_adapter *);
> static void fjes_free_resources(struct fjes_adapter *);
> +static netdev_tx_t fjes_xmit_frame(struct sk_buff *, struct net_device *);
> static irqreturn_t fjes_intr(int, void*);
>
> static int fjes_acpi_add(struct acpi_device *);
> @@ -215,6 +216,7 @@ static void fjes_free_irq(struct fjes_adapter *adapter)
> static const struct net_device_ops fjes_netdev_ops = {
> .ndo_open = fjes_open,
> .ndo_stop = fjes_close,
> + .ndo_start_xmit = fjes_xmit_frame,
> };
>
> /* fjes_open - Called when a network interface is made active */
> @@ -405,6 +407,181 @@ static void fjes_free_resources(struct fjes_adapter *adapter)
> }
> }
>
> +static int fjes_tx_send(struct fjes_adapter *adapter, int dest,
> + void *data, size_t len)
> +{
> + int retval;
> +
> + retval =
> + fjes_hw_epbuf_tx_pkt_send(&adapter->hw.ep_shm_info[dest].tx,
> + data, len);
> + if (retval)
> + return retval;
> +
> + adapter->hw.ep_shm_info[dest].tx.info->v1i.tx_status =
> + FJES_TX_DELAY_SEND_PENDING;
> +
> + retval = 0;
> + return retval;
> +}
> +
> +static netdev_tx_t
> +fjes_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
> +{
> + struct fjes_adapter *adapter = netdev_priv(netdev);
> + struct fjes_hw *hw = &adapter->hw;
> + enum ep_partner_status pstatus;
> +
> + int len;
> + char *data, shortpkt[VLAN_ETH_HLEN];
> +
> + u16 queue_no = 0;
> + struct netdev_queue *cur_queue = netdev_get_tx_queue(netdev, queue_no);
> + netdev_tx_t ret = NETDEV_TX_OK;
> + struct ethhdr *eth;
> + int dest_epid;
> +
> + int max_epid, my_epid;
> + u16 vlan_id = 0;
> + bool vlan = false;
> + bool is_multi = false;
> +
> + eth = (struct ethhdr *)skb->data;
> + my_epid = hw->my_epid;
> +
> + vlan = (vlan_get_tag(skb, &vlan_id) == 0) ? true : false;
> +
> + data = skb->data;
> + len = skb->len;
> +
> + if (is_multicast_ether_addr(eth->h_dest)) {
> + dest_epid = 0;
> + max_epid = hw->max_epid;
> + is_multi = true;
> + } else if (is_local_ether_addr(eth->h_dest)) {
> + dest_epid = eth->h_dest[ETH_ALEN - 1];
> + max_epid = dest_epid + 1;
> +
> + if ((eth->h_dest[0] == 0x02) &&
> + (0x00 == (eth->h_dest[1] | eth->h_dest[2] |
> + eth->h_dest[3] | eth->h_dest[4])) &&
> + (dest_epid < hw->max_epid)) {
> + ;
> + } else {
> + dest_epid = 0;
> + max_epid = 0;
> + ret = NETDEV_TX_OK;
> +
> + adapter->stats64.tx_packets += 1;
> + hw->ep_shm_info[my_epid].net_stats.tx_packets += 1;
> + adapter->stats64.tx_bytes += len;
> + hw->ep_shm_info[my_epid].net_stats.tx_bytes += len;
There are same codes in the fjes_xmit_frame().
How about create a new function to unify them?
> + }
> + } else {
> + dest_epid = 0;
> + max_epid = 0;
> + ret = NETDEV_TX_OK;
> +
> + adapter->stats64.tx_packets += 1;
> + hw->ep_shm_info[my_epid].net_stats.tx_packets += 1;
> + adapter->stats64.tx_bytes += len;
> + hw->ep_shm_info[my_epid].net_stats.tx_bytes += len;
> + }
> +
> + for (; dest_epid < max_epid; dest_epid++) {
> + if (my_epid == dest_epid)
> + continue;
> +
> + pstatus = fjes_hw_get_partner_ep_status(hw, dest_epid);
> + if (pstatus != EP_PARTNER_SHARED) {
> + ret = NETDEV_TX_OK;
> + } else if (!fjes_hw_check_epbuf_version(
> + &adapter->hw.ep_shm_info[dest_epid].rx, 0)) {
> + /* version is NOT 0 */
> + adapter->stats64.tx_carrier_errors += 1;
> + hw->ep_shm_info[my_epid].net_stats
> + .tx_carrier_errors += 1;
> +
> + ret = NETDEV_TX_OK;
> + } else if (!fjes_hw_check_mtu(
> + &adapter->hw.ep_shm_info[dest_epid].rx,
> + netdev->mtu)) {
> + adapter->stats64.tx_dropped += 1;
> + hw->ep_shm_info[my_epid].net_stats.tx_dropped += 1;
> + adapter->stats64.tx_errors += 1;
> + hw->ep_shm_info[my_epid].net_stats.tx_errors += 1;
There are same codes in the fjes_xmit_frame().
How about create a new function to unify them?
> +
> + ret = NETDEV_TX_OK;
> + } else if (vlan &&
> + !fjes_hw_check_vlan_id(
> + &adapter->hw.ep_shm_info[dest_epid].rx,
> + vlan_id)) {
> + ret = NETDEV_TX_OK;
> + } else {
> + if (len < VLAN_ETH_HLEN) {
> + memset(shortpkt, 0, VLAN_ETH_HLEN);
> + memcpy(shortpkt, skb->data, skb->len);
> + len = VLAN_ETH_HLEN;
> + data = shortpkt;
> + }
> +
> + if (adapter->tx_retry_count == 0) {
> + adapter->tx_start_jiffies = jiffies;
> + adapter->tx_retry_count = 1;
> + } else {
> + adapter->tx_retry_count++;
> + }
> +
> + if (fjes_tx_send(adapter, dest_epid, data, len)) {
> + if (is_multi) {
> + ret = NETDEV_TX_OK;
> + } else if (
> + ((long)jiffies -
> + (long)adapter->tx_start_jiffies) >=
> + FJES_TX_RETRY_TIMEOUT) {
> + adapter->stats64.tx_fifo_errors += 1;
> + hw->ep_shm_info[my_epid].net_stats
> + .tx_fifo_errors += 1;
> + adapter->stats64.tx_errors += 1;
> + hw->ep_shm_info[my_epid].net_stats
> + .tx_errors += 1;
> +
> + ret = NETDEV_TX_OK;
> + } else {
> + netdev->trans_start = jiffies;
> + netif_tx_stop_queue(cur_queue);
> +
> + ret = NETDEV_TX_BUSY;
Even if ret set NETDEV_TX_BUSY, it may be changed to NETDEV_TX_OK
in the for loop. Is it OK?
> + }
> + } else {
> + if (!is_multi) {
> + adapter->stats64.tx_packets += 1;
> + hw->ep_shm_info[my_epid].net_stats
> + .tx_packets += 1;
> + adapter->stats64.tx_bytes += len;
> + hw->ep_shm_info[my_epid].net_stats
> + .tx_bytes += len;
> + }
> +
> + adapter->tx_retry_count = 0;
> + ret = NETDEV_TX_OK;
> + }
> + }
> + }
> +
> + if (ret == NETDEV_TX_OK) {
> + dev_kfree_skb(skb);
> + if (is_multi) {
> + adapter->stats64.tx_packets += 1;
> + hw->ep_shm_info[my_epid].net_stats.tx_packets += 1;
> + adapter->stats64.tx_bytes += 1;
adapter->stats64.tx_bytes +=len?
Thanks,
Yasuaki Ishimatsu
> + hw->ep_shm_info[my_epid].net_stats.tx_bytes += len;
> + }
> + }
> +
> + return ret;
> +}
> +
> static irqreturn_t fjes_intr(int irq, void *data)
> {
> struct fjes_adapter *adapter = data;
> --
> 1.8.3.1
>
--
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