* [PATCH 2/6] ibmveth: Implement ethtool hooks to enable/disable checksum offload
[not found] <11864293333377-patch-mail.ibm.com>
@ 2007-08-06 19:42 ` Brian King
2007-08-07 21:48 ` Jeff Garzik
2007-08-06 19:42 ` [PATCH 3/6] ibmveth: Add ethtool TSO handlers Brian King
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Brian King @ 2007-08-06 19:42 UTC (permalink / raw)
To: santil; +Cc: rcjenn, netdev, linuxppc-dev, brking
This patch adds the appropriate ethtool hooks to allow for enabling/disabling
of hypervisor assisted checksum offload for TCP.
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---
linux-2.6-bjking1/drivers/net/ibmveth.c | 118 +++++++++++++++++++++++++++++++-
linux-2.6-bjking1/drivers/net/ibmveth.h | 1
2 files changed, 117 insertions(+), 2 deletions(-)
diff -puN drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool 2007-08-01 14:55:14.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-01 14:55:14.000000000 -0500
@@ -641,12 +641,125 @@ static u32 netdev_get_link(struct net_de
return 1;
}
+static void ibmveth_set_rx_csum_flags(struct net_device *dev, u32 data)
+{
+ struct ibmveth_adapter *adapter = dev->priv;
+
+ if (data)
+ adapter->rx_csum = 1;
+ else {
+ adapter->rx_csum = 0;
+ dev->features &= ~NETIF_F_IP_CSUM;
+ }
+}
+
+static void ibmveth_set_tx_csum_flags(struct net_device *dev, u32 data)
+{
+ struct ibmveth_adapter *adapter = dev->priv;
+
+ if (data) {
+ dev->features |= NETIF_F_IP_CSUM;
+ adapter->rx_csum = 1;
+ } else
+ dev->features &= ~NETIF_F_IP_CSUM;
+}
+
+static int ibmveth_set_csum_offload(struct net_device *dev, u32 data,
+ void (*done) (struct net_device *, u32))
+{
+ struct ibmveth_adapter *adapter = dev->priv;
+ union ibmveth_illan_attributes set_attr, clr_attr, ret_attr;
+ long ret;
+ int rc1 = 0, rc2 = 0;
+ int restart = 0;
+
+ if (netif_running(dev)) {
+ restart = 1;
+ adapter->pool_config = 1;
+ ibmveth_close(dev);
+ adapter->pool_config = 0;
+ }
+
+ set_attr.desc = 0;
+ clr_attr.desc = 0;
+
+ if (data)
+ set_attr.fields.tcp_csum_offload_ipv4 = 1;
+ else
+ clr_attr.fields.tcp_csum_offload_ipv4 = 1;
+
+ ret = h_illan_attributes(adapter->vdev->unit_address, 0, 0, &ret_attr.desc);
+
+ if (ret == H_SUCCESS && !ret_attr.fields.active_trunk &&
+ !ret_attr.fields.trunk_priority &&
+ ret_attr.fields.csum_offload_padded_pkt_support) {
+ ret = h_illan_attributes(adapter->vdev->unit_address, clr_attr.desc,
+ set_attr.desc, &ret_attr.desc);
+
+ if (ret != H_SUCCESS) {
+ rc1 = -EIO;
+ ibmveth_error_printk("unable to change checksum offload settings."
+ " %d rc=%ld\n", data, ret);
+
+ ret = h_illan_attributes(adapter->vdev->unit_address,
+ set_attr.desc, clr_attr.desc, &ret_attr.desc);
+ } else
+ done(dev, data);
+ } else {
+ rc1 = -EIO;
+ ibmveth_error_printk("unable to change checksum offload settings."
+ " %d rc=%ld ret_attr=%lx\n", data, ret, ret_attr.desc);
+ }
+
+ if (restart)
+ rc2 = ibmveth_open(dev);
+
+ return rc1 ? rc1 : rc2;
+}
+
+static int ibmveth_set_rx_csum(struct net_device *dev, u32 data)
+{
+ struct ibmveth_adapter *adapter = dev->priv;
+
+ if ((data && adapter->rx_csum) || (!data && !adapter->rx_csum))
+ return 0;
+
+ return ibmveth_set_csum_offload(dev, data, ibmveth_set_rx_csum_flags);
+}
+
+static int ibmveth_set_tx_csum(struct net_device *dev, u32 data)
+{
+ struct ibmveth_adapter *adapter = dev->priv;
+ int rc = 0;
+
+ if (data && (dev->features & NETIF_F_IP_CSUM))
+ return 0;
+ if (!data && !(dev->features & NETIF_F_IP_CSUM))
+ return 0;
+
+ if (data && !adapter->rx_csum)
+ rc = ibmveth_set_csum_offload(dev, data, ibmveth_set_tx_csum_flags);
+ else
+ ibmveth_set_tx_csum_flags(dev, data);
+
+ return rc;
+}
+
+static u32 ibmveth_get_rx_csum(struct net_device *dev)
+{
+ struct ibmveth_adapter *adapter = dev->priv;
+ return adapter->rx_csum;
+}
+
static const struct ethtool_ops netdev_ethtool_ops = {
.get_drvinfo = netdev_get_drvinfo,
.get_settings = netdev_get_settings,
.get_link = netdev_get_link,
.get_sg = ethtool_op_get_sg,
.get_tx_csum = ethtool_op_get_tx_csum,
+ .set_tx_csum = ibmveth_set_tx_csum,
+ .get_rx_csum = ibmveth_get_rx_csum,
+ .set_rx_csum = ibmveth_set_rx_csum
};
static int ibmveth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
@@ -1104,9 +1217,10 @@ static int __devinit ibmveth_probe(struc
ret = h_illan_attributes(dev->unit_address, 0, set_attr.desc,
&ret_attr.desc);
- if (ret == H_SUCCESS)
+ if (ret == H_SUCCESS) {
+ adapter->rx_csum = 1;
netdev->features |= NETIF_F_IP_CSUM;
- else
+ } else
ret = h_illan_attributes(dev->unit_address, set_attr.desc,
0, &ret_attr.desc);
}
diff -puN drivers/net/ibmveth.h~ibmveth_csum_offload_ethtool drivers/net/ibmveth.h
--- linux-2.6/drivers/net/ibmveth.h~ibmveth_csum_offload_ethtool 2007-08-01 14:55:14.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h 2007-08-01 14:55:14.000000000 -0500
@@ -137,6 +137,7 @@ struct ibmveth_adapter {
struct ibmveth_buff_pool rx_buff_pool[IbmVethNumBufferPools];
struct ibmveth_rx_q rx_queue;
int pool_config;
+ int rx_csum;
/* adapter specific stats */
u64 replenish_task_cycles;
_
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/6] ibmveth: Implement ethtool hooks to enable/disable checksum offload
2007-08-06 19:42 ` [PATCH 2/6] ibmveth: Implement ethtool hooks to enable/disable checksum offload Brian King
@ 2007-08-07 21:48 ` Jeff Garzik
2007-08-07 22:07 ` Brian King
0 siblings, 1 reply; 15+ messages in thread
From: Jeff Garzik @ 2007-08-07 21:48 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
Brian King wrote:
> This patch adds the appropriate ethtool hooks to allow for enabling/disabling
> of hypervisor assisted checksum offload for TCP.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> linux-2.6-bjking1/drivers/net/ibmveth.c | 118 +++++++++++++++++++++++++++++++-
> linux-2.6-bjking1/drivers/net/ibmveth.h | 1
> 2 files changed, 117 insertions(+), 2 deletions(-)
>
> diff -puN drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool drivers/net/ibmveth.c
> --- linux-2.6/drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool 2007-08-01 14:55:14.000000000 -0500
> +++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-01 14:55:14.000000000 -0500
> @@ -641,12 +641,125 @@ static u32 netdev_get_link(struct net_de
> return 1;
> }
>
> +static void ibmveth_set_rx_csum_flags(struct net_device *dev, u32 data)
> +{
> + struct ibmveth_adapter *adapter = dev->priv;
> +
> + if (data)
> + adapter->rx_csum = 1;
> + else {
> + adapter->rx_csum = 0;
> + dev->features &= ~NETIF_F_IP_CSUM;
why does this RX-related code clear a TX-related flag?
otherwise looks OK
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/6] ibmveth: Implement ethtool hooks to enable/disable checksum offload
2007-08-07 21:48 ` Jeff Garzik
@ 2007-08-07 22:07 ` Brian King
2007-08-07 22:16 ` Jeff Garzik
0 siblings, 1 reply; 15+ messages in thread
From: Brian King @ 2007-08-07 22:07 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linuxppc-dev, rcjenn, santil, netdev
Jeff Garzik wrote:
> Brian King wrote:
>> This patch adds the appropriate ethtool hooks to allow for enabling/disabling
>> of hypervisor assisted checksum offload for TCP.
>>
>> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
>> ---
>>
>> linux-2.6-bjking1/drivers/net/ibmveth.c | 118 +++++++++++++++++++++++++++++++-
>> linux-2.6-bjking1/drivers/net/ibmveth.h | 1
>> 2 files changed, 117 insertions(+), 2 deletions(-)
>>
>> diff -puN drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool drivers/net/ibmveth.c
>> --- linux-2.6/drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool 2007-08-01 14:55:14.000000000 -0500
>> +++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-01 14:55:14.000000000 -0500
>> @@ -641,12 +641,125 @@ static u32 netdev_get_link(struct net_de
>> return 1;
>> }
>>
>> +static void ibmveth_set_rx_csum_flags(struct net_device *dev, u32 data)
>> +{
>> + struct ibmveth_adapter *adapter = dev->priv;
>> +
>> + if (data)
>> + adapter->rx_csum = 1;
>> + else {
>> + adapter->rx_csum = 0;
>> + dev->features &= ~NETIF_F_IP_CSUM;
>
> why does this RX-related code clear a TX-related flag?
Its related to how the pSeries firmware works. The firmware provides an interface
to enable "checksum offload", which means both tx and rx checksum offload from
the firmware's point of view. The firmware does not support enabling checksum
offload for only rx. If I disable it for rx I have to disable it for tx as
well, otherwise the firmware will reject all future tx buffers I throw at it
that are not checksummed.
-Brian
--
Brian King
Linux on Power Virtualization
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 2/6] ibmveth: Implement ethtool hooks to enable/disable checksum offload
2007-08-07 22:07 ` Brian King
@ 2007-08-07 22:16 ` Jeff Garzik
0 siblings, 0 replies; 15+ messages in thread
From: Jeff Garzik @ 2007-08-07 22:16 UTC (permalink / raw)
To: brking; +Cc: linuxppc-dev, rcjenn, santil, netdev
Brian King wrote:
> Jeff Garzik wrote:
>> Brian King wrote:
>>> This patch adds the appropriate ethtool hooks to allow for enabling/disabling
>>> of hypervisor assisted checksum offload for TCP.
>>>
>>> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
>>> ---
>>>
>>> linux-2.6-bjking1/drivers/net/ibmveth.c | 118 +++++++++++++++++++++++++++++++-
>>> linux-2.6-bjking1/drivers/net/ibmveth.h | 1
>>> 2 files changed, 117 insertions(+), 2 deletions(-)
>>>
>>> diff -puN drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool drivers/net/ibmveth.c
>>> --- linux-2.6/drivers/net/ibmveth.c~ibmveth_csum_offload_ethtool 2007-08-01 14:55:14.000000000 -0500
>>> +++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-01 14:55:14.000000000 -0500
>>> @@ -641,12 +641,125 @@ static u32 netdev_get_link(struct net_de
>>> return 1;
>>> }
>>>
>>> +static void ibmveth_set_rx_csum_flags(struct net_device *dev, u32 data)
>>> +{
>>> + struct ibmveth_adapter *adapter = dev->priv;
>>> +
>>> + if (data)
>>> + adapter->rx_csum = 1;
>>> + else {
>>> + adapter->rx_csum = 0;
>>> + dev->features &= ~NETIF_F_IP_CSUM;
>> why does this RX-related code clear a TX-related flag?
>
> Its related to how the pSeries firmware works. The firmware provides an interface
> to enable "checksum offload", which means both tx and rx checksum offload from
> the firmware's point of view. The firmware does not support enabling checksum
> offload for only rx. If I disable it for rx I have to disable it for tx as
> well, otherwise the firmware will reject all future tx buffers I throw at it
> that are not checksummed.
ACK once you add a comment describing this :)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/6] ibmveth: Add ethtool TSO handlers
[not found] <11864293333377-patch-mail.ibm.com>
2007-08-06 19:42 ` [PATCH 2/6] ibmveth: Implement ethtool hooks to enable/disable checksum offload Brian King
@ 2007-08-06 19:42 ` Brian King
2007-08-07 21:50 ` Jeff Garzik
2007-08-06 19:42 ` [PATCH 4/6] ibmveth: Add ethtool driver stats hooks Brian King
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Brian King @ 2007-08-06 19:42 UTC (permalink / raw)
To: santil; +Cc: rcjenn, netdev, linuxppc-dev, brking
Add handlers for get_tso and get_ufo to prevent errors being printed
by ethtool.
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---
drivers/net/ibmveth.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff -puN drivers/net/ibmveth.c~ibmveth_ethtool_get_tso drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_ethtool_get_tso 2007-07-19 11:18:38.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-07-19 11:18:38.000000000 -0500
@@ -759,7 +759,9 @@ static const struct ethtool_ops netdev_e
.get_tx_csum = ethtool_op_get_tx_csum,
.set_tx_csum = ibmveth_set_tx_csum,
.get_rx_csum = ibmveth_get_rx_csum,
- .set_rx_csum = ibmveth_set_rx_csum
+ .set_rx_csum = ibmveth_set_rx_csum,
+ .get_tso = ethtool_op_get_tso,
+ .get_ufo = ethtool_op_get_ufo
};
static int ibmveth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
_
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/6] ibmveth: Add ethtool TSO handlers
2007-08-06 19:42 ` [PATCH 3/6] ibmveth: Add ethtool TSO handlers Brian King
@ 2007-08-07 21:50 ` Jeff Garzik
0 siblings, 0 replies; 15+ messages in thread
From: Jeff Garzik @ 2007-08-07 21:50 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
Brian King wrote:
> Add handlers for get_tso and get_ufo to prevent errors being printed
> by ethtool.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> drivers/net/ibmveth.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff -puN drivers/net/ibmveth.c~ibmveth_ethtool_get_tso drivers/net/ibmveth.c
> --- linux-2.6/drivers/net/ibmveth.c~ibmveth_ethtool_get_tso 2007-07-19 11:18:38.000000000 -0500
> +++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-07-19 11:18:38.000000000 -0500
> @@ -759,7 +759,9 @@ static const struct ethtool_ops netdev_e
> .get_tx_csum = ethtool_op_get_tx_csum,
> .set_tx_csum = ibmveth_set_tx_csum,
> .get_rx_csum = ibmveth_get_rx_csum,
> - .set_rx_csum = ibmveth_set_rx_csum
> + .set_rx_csum = ibmveth_set_rx_csum,
> + .get_tso = ethtool_op_get_tso,
> + .get_ufo = ethtool_op_get_ufo
ACK, once you add a comma to the end of the final initializer
As you see from this patch, the practice of -not- having commas at the
end of a list of struct initializers is not patch-friendly, since you
must touch an unrelated line each time you patch the end of the struct.
For named initializers particularly, the lack of a comma is even more
useless.
So, it might tweak some C perfectionists, but adding that
seemingly-useless comma at the end of the last entry reduces maintenance
headache and makes patch reviews slightly more clear.
Jeff
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/6] ibmveth: Add ethtool driver stats hooks
[not found] <11864293333377-patch-mail.ibm.com>
2007-08-06 19:42 ` [PATCH 2/6] ibmveth: Implement ethtool hooks to enable/disable checksum offload Brian King
2007-08-06 19:42 ` [PATCH 3/6] ibmveth: Add ethtool TSO handlers Brian King
@ 2007-08-06 19:42 ` Brian King
2007-08-07 21:50 ` Jeff Garzik
2007-08-06 19:42 ` [PATCH 5/6] ibmveth: Remove dead frag processing code Brian King
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Brian King @ 2007-08-06 19:42 UTC (permalink / raw)
To: santil; +Cc: rcjenn, netdev, linuxppc-dev, brking
Add ethtool hooks to ibmveth to retrieve driver statistics.
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---
drivers/net/ibmveth.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff -puN drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats 2007-07-19 11:18:41.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-07-19 11:18:41.000000000 -0500
@@ -112,6 +112,28 @@ MODULE_DESCRIPTION("IBM i/pSeries Virtua
MODULE_LICENSE("GPL");
MODULE_VERSION(ibmveth_driver_version);
+struct ibmveth_stat {
+ char name[ETH_GSTRING_LEN];
+ int offset;
+};
+
+#define IBMVETH_STAT_OFF(stat) offsetof(struct ibmveth_adapter, stat)
+#define IBMVETH_GET_STAT(a, off) *((u64 *)(((unsigned long)(a)) + off))
+
+struct ibmveth_stat ibmveth_stats[] = {
+ { "replenish_task_cycles", IBMVETH_STAT_OFF(replenish_task_cycles) },
+ { "replenish_no_mem", IBMVETH_STAT_OFF(replenish_no_mem) },
+ { "replenish_add_buff_failure", IBMVETH_STAT_OFF(replenish_add_buff_failure) },
+ { "replenish_add_buff_success", IBMVETH_STAT_OFF(replenish_add_buff_success) },
+ { "rx_invalid_buffer", IBMVETH_STAT_OFF(rx_invalid_buffer) },
+ { "rx_no_buffer", IBMVETH_STAT_OFF(rx_no_buffer) },
+ { "tx_multidesc_send", IBMVETH_STAT_OFF(tx_multidesc_send) },
+ { "tx_linearized", IBMVETH_STAT_OFF(tx_linearized) },
+ { "tx_linearize_failed", IBMVETH_STAT_OFF(tx_linearize_failed) },
+ { "tx_map_failed", IBMVETH_STAT_OFF(tx_map_failed) },
+ { "tx_send_failed", IBMVETH_STAT_OFF(tx_send_failed) }
+};
+
/* simple methods of getting data from the current rxq entry */
static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
{
@@ -751,6 +773,32 @@ static u32 ibmveth_get_rx_csum(struct ne
return adapter->rx_csum;
}
+static void ibmveth_get_strings(struct net_device *dev, u32 stringset, u8 *data)
+{
+ int i;
+
+ if (stringset != ETH_SS_STATS)
+ return;
+
+ for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++, data += ETH_GSTRING_LEN)
+ memcpy(data, ibmveth_stats[i].name, ETH_GSTRING_LEN);
+}
+
+static int ibmveth_get_stats_count(struct net_device *dev)
+{
+ return ARRAY_SIZE(ibmveth_stats);
+}
+
+static void ibmveth_get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *stats, u64 *data)
+{
+ int i;
+ struct ibmveth_adapter *adapter = dev->priv;
+
+ for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++)
+ data[i] = IBMVETH_GET_STAT(adapter, ibmveth_stats[i].offset);
+}
+
static const struct ethtool_ops netdev_ethtool_ops = {
.get_drvinfo = netdev_get_drvinfo,
.get_settings = netdev_get_settings,
@@ -761,7 +809,10 @@ static const struct ethtool_ops netdev_e
.get_rx_csum = ibmveth_get_rx_csum,
.set_rx_csum = ibmveth_set_rx_csum,
.get_tso = ethtool_op_get_tso,
- .get_ufo = ethtool_op_get_ufo
+ .get_ufo = ethtool_op_get_ufo,
+ .get_strings = ibmveth_get_strings,
+ .get_stats_count = ibmveth_get_stats_count,
+ .get_ethtool_stats = ibmveth_get_ethtool_stats
};
static int ibmveth_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
diff -puN drivers/net/ibmveth.h~ibmveth_ethtool_driver_stats drivers/net/ibmveth.h
_
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 4/6] ibmveth: Add ethtool driver stats hooks
2007-08-06 19:42 ` [PATCH 4/6] ibmveth: Add ethtool driver stats hooks Brian King
@ 2007-08-07 21:50 ` Jeff Garzik
0 siblings, 0 replies; 15+ messages in thread
From: Jeff Garzik @ 2007-08-07 21:50 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
Brian King wrote:
> Add ethtool hooks to ibmveth to retrieve driver statistics.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> drivers/net/ibmveth.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff -puN drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats drivers/net/ibmveth.c
> --- linux-2.6/drivers/net/ibmveth.c~ibmveth_ethtool_driver_stats 2007-07-19 11:18:41.000000000 -0500
> +++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-07-19 11:18:41.000000000 -0500
> @@ -112,6 +112,28 @@ MODULE_DESCRIPTION("IBM i/pSeries Virtua
> MODULE_LICENSE("GPL");
> MODULE_VERSION(ibmveth_driver_version);
>
> +struct ibmveth_stat {
> + char name[ETH_GSTRING_LEN];
> + int offset;
> +};
> +
> +#define IBMVETH_STAT_OFF(stat) offsetof(struct ibmveth_adapter, stat)
> +#define IBMVETH_GET_STAT(a, off) *((u64 *)(((unsigned long)(a)) + off))
> +
> +struct ibmveth_stat ibmveth_stats[] = {
> + { "replenish_task_cycles", IBMVETH_STAT_OFF(replenish_task_cycles) },
> + { "replenish_no_mem", IBMVETH_STAT_OFF(replenish_no_mem) },
> + { "replenish_add_buff_failure", IBMVETH_STAT_OFF(replenish_add_buff_failure) },
> + { "replenish_add_buff_success", IBMVETH_STAT_OFF(replenish_add_buff_success) },
> + { "rx_invalid_buffer", IBMVETH_STAT_OFF(rx_invalid_buffer) },
> + { "rx_no_buffer", IBMVETH_STAT_OFF(rx_no_buffer) },
> + { "tx_multidesc_send", IBMVETH_STAT_OFF(tx_multidesc_send) },
> + { "tx_linearized", IBMVETH_STAT_OFF(tx_linearized) },
> + { "tx_linearize_failed", IBMVETH_STAT_OFF(tx_linearize_failed) },
> + { "tx_map_failed", IBMVETH_STAT_OFF(tx_map_failed) },
> + { "tx_send_failed", IBMVETH_STAT_OFF(tx_send_failed) }
> +};
> +
> /* simple methods of getting data from the current rxq entry */
> static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
> {
> @@ -751,6 +773,32 @@ static u32 ibmveth_get_rx_csum(struct ne
> return adapter->rx_csum;
> }
>
> +static void ibmveth_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> +{
> + int i;
> +
> + if (stringset != ETH_SS_STATS)
> + return;
> +
> + for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++, data += ETH_GSTRING_LEN)
> + memcpy(data, ibmveth_stats[i].name, ETH_GSTRING_LEN);
> +}
> +
> +static int ibmveth_get_stats_count(struct net_device *dev)
> +{
> + return ARRAY_SIZE(ibmveth_stats);
> +}
> +
> +static void ibmveth_get_ethtool_stats(struct net_device *dev,
> + struct ethtool_stats *stats, u64 *data)
> +{
> + int i;
> + struct ibmveth_adapter *adapter = dev->priv;
> +
> + for (i = 0; i < ARRAY_SIZE(ibmveth_stats); i++)
> + data[i] = IBMVETH_GET_STAT(adapter, ibmveth_stats[i].offset);
> +}
> +
> static const struct ethtool_ops netdev_ethtool_ops = {
> .get_drvinfo = netdev_get_drvinfo,
> .get_settings = netdev_get_settings,
> @@ -761,7 +809,10 @@ static const struct ethtool_ops netdev_e
> .get_rx_csum = ibmveth_get_rx_csum,
> .set_rx_csum = ibmveth_set_rx_csum,
> .get_tso = ethtool_op_get_tso,
> - .get_ufo = ethtool_op_get_ufo
> + .get_ufo = ethtool_op_get_ufo,
> + .get_strings = ibmveth_get_strings,
> + .get_stats_count = ibmveth_get_stats_count,
> + .get_ethtool_stats = ibmveth_get_ethtool_stats
ACK, modulo comma-at-end-of-initializer per previous email
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/6] ibmveth: Remove dead frag processing code
[not found] <11864293333377-patch-mail.ibm.com>
` (2 preceding siblings ...)
2007-08-06 19:42 ` [PATCH 4/6] ibmveth: Add ethtool driver stats hooks Brian King
@ 2007-08-06 19:42 ` Brian King
2007-08-06 19:42 ` [PATCH 6/6] ibmveth: Remove use of bitfields Brian King
2007-08-07 21:46 ` [PATCH 1/6] ibmveth: Enable TCP checksum offload Jeff Garzik
5 siblings, 0 replies; 15+ messages in thread
From: Brian King @ 2007-08-06 19:42 UTC (permalink / raw)
To: santil; +Cc: rcjenn, netdev, linuxppc-dev, brking
Removes dead frag processing code from ibmveth. Since NETIF_F_SG was
not set, this code was never executed. Also, since the ibmveth
interface can only handle 6 fragments, core networking code would need
to be modified in order to efficiently enable this support.
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---
linux-2.6-bjking1/drivers/net/ibmveth.c | 100 +++++---------------------------
linux-2.6-bjking1/drivers/net/ibmveth.h | 5 -
2 files changed, 17 insertions(+), 88 deletions(-)
diff -puN drivers/net/ibmveth.c~ibmveth_remove_frag drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_remove_frag 2007-08-06 14:11:08.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-06 14:11:39.000000000 -0500
@@ -28,7 +28,6 @@
/**************************************************************************/
/*
TODO:
- - remove frag processing code - no longer needed
- add support for sysfs
- possibly remove procfs support
*/
@@ -127,9 +126,6 @@ struct ibmveth_stat ibmveth_stats[] = {
{ "replenish_add_buff_success", IBMVETH_STAT_OFF(replenish_add_buff_success) },
{ "rx_invalid_buffer", IBMVETH_STAT_OFF(rx_invalid_buffer) },
{ "rx_no_buffer", IBMVETH_STAT_OFF(rx_no_buffer) },
- { "tx_multidesc_send", IBMVETH_STAT_OFF(tx_multidesc_send) },
- { "tx_linearized", IBMVETH_STAT_OFF(tx_linearized) },
- { "tx_linearize_failed", IBMVETH_STAT_OFF(tx_linearize_failed) },
{ "tx_map_failed", IBMVETH_STAT_OFF(tx_map_failed) },
{ "tx_send_failed", IBMVETH_STAT_OFF(tx_send_failed) }
};
@@ -825,9 +821,8 @@ static int ibmveth_ioctl(struct net_devi
static int ibmveth_start_xmit(struct sk_buff *skb, struct net_device *netdev)
{
struct ibmveth_adapter *adapter = netdev->priv;
- union ibmveth_buf_desc desc[IbmVethMaxSendFrags];
+ union ibmveth_buf_desc desc;
unsigned long lpar_rc;
- int nfrags = 0, curfrag;
unsigned long correlator;
unsigned long flags;
unsigned int retry_count;
@@ -837,25 +832,11 @@ static int ibmveth_start_xmit(struct sk_
unsigned int tx_send_failed = 0;
unsigned int tx_map_failed = 0;
-
- if ((skb_shinfo(skb)->nr_frags + 1) > IbmVethMaxSendFrags) {
- tx_dropped++;
- goto out;
- }
-
- memset(&desc, 0, sizeof(desc));
-
- /* nfrags = number of frags after the initial fragment */
- nfrags = skb_shinfo(skb)->nr_frags;
-
- if(nfrags)
- adapter->tx_multidesc_send++;
-
- /* map the initial fragment */
- desc[0].fields.length = nfrags ? skb->len - skb->data_len : skb->len;
- desc[0].fields.address = dma_map_single(&adapter->vdev->dev, skb->data,
- desc[0].fields.length, DMA_TO_DEVICE);
- desc[0].fields.valid = 1;
+ desc.desc = 0;
+ desc.fields.length = skb->len;
+ desc.fields.address = dma_map_single(&adapter->vdev->dev, skb->data,
+ desc.fields.length, DMA_TO_DEVICE);
+ desc.fields.valid = 1;
if (skb->ip_summed == CHECKSUM_PARTIAL &&
ip_hdr(skb)->protocol != IPPROTO_TCP && skb_checksum_help(skb)) {
@@ -867,75 +848,34 @@ static int ibmveth_start_xmit(struct sk_
if (skb->ip_summed == CHECKSUM_PARTIAL) {
unsigned char *buf = skb_transport_header(skb) + skb->csum_offset;
- desc[0].fields.no_csum = 1;
- desc[0].fields.csum_good = 1;
+ desc.fields.no_csum = 1;
+ desc.fields.csum_good = 1;
/* Need to zero out the checksum */
buf[0] = 0;
buf[1] = 0;
}
- if(dma_mapping_error(desc[0].fields.address)) {
- ibmveth_error_printk("tx: unable to map initial fragment\n");
+ if (dma_mapping_error(desc.fields.address)) {
+ ibmveth_error_printk("tx: unable to map xmit buffer\n");
tx_map_failed++;
tx_dropped++;
goto out;
}
- curfrag = nfrags;
-
- /* map fragments past the initial portion if there are any */
- while(curfrag--) {
- skb_frag_t *frag = &skb_shinfo(skb)->frags[curfrag];
- desc[curfrag+1].fields.address
- = dma_map_single(&adapter->vdev->dev,
- page_address(frag->page) + frag->page_offset,
- frag->size, DMA_TO_DEVICE);
- desc[curfrag+1].fields.length = frag->size;
- desc[curfrag+1].fields.valid = 1;
- if (skb->ip_summed == CHECKSUM_PARTIAL) {
- desc[curfrag+1].fields.no_csum = 1;
- desc[curfrag+1].fields.csum_good = 1;
- }
-
- if(dma_mapping_error(desc[curfrag+1].fields.address)) {
- ibmveth_error_printk("tx: unable to map fragment %d\n", curfrag);
- tx_map_failed++;
- tx_dropped++;
- /* Free all the mappings we just created */
- while(curfrag < nfrags) {
- dma_unmap_single(&adapter->vdev->dev,
- desc[curfrag+1].fields.address,
- desc[curfrag+1].fields.length,
- DMA_TO_DEVICE);
- curfrag++;
- }
- goto out;
- }
- }
-
/* send the frame. Arbitrarily set retrycount to 1024 */
correlator = 0;
retry_count = 1024;
do {
lpar_rc = h_send_logical_lan(adapter->vdev->unit_address,
- desc[0].desc,
- desc[1].desc,
- desc[2].desc,
- desc[3].desc,
- desc[4].desc,
- desc[5].desc,
- correlator,
- &correlator);
+ desc.desc, 0, 0, 0, 0, 0,
+ correlator, &correlator);
} while ((lpar_rc == H_BUSY) && (retry_count--));
if(lpar_rc != H_SUCCESS && lpar_rc != H_DROPPED) {
- int i;
ibmveth_error_printk("tx: h_send_logical_lan failed with rc=%ld\n", lpar_rc);
- for(i = 0; i < 6; i++) {
- ibmveth_error_printk("tx: desc[%i] valid=%d, len=%d, address=0x%d\n", i,
- desc[i].fields.valid, desc[i].fields.length, desc[i].fields.address);
- }
+ ibmveth_error_printk("tx: valid=%d, len=%d, address=0x%08x\n",
+ desc.fields.valid, desc.fields.length, desc.fields.address);
tx_send_failed++;
tx_dropped++;
} else {
@@ -944,11 +884,8 @@ static int ibmveth_start_xmit(struct sk_
netdev->trans_start = jiffies;
}
- do {
- dma_unmap_single(&adapter->vdev->dev,
- desc[nfrags].fields.address,
- desc[nfrags].fields.length, DMA_TO_DEVICE);
- } while(--nfrags >= 0);
+ dma_unmap_single(&adapter->vdev->dev, desc.fields.address,
+ desc.fields.length, DMA_TO_DEVICE);
out: spin_lock_irqsave(&adapter->stats_lock, flags);
adapter->stats.tx_dropped += tx_dropped;
@@ -1360,10 +1297,7 @@ static int ibmveth_seq_show(struct seq_f
firmware_mac[3], firmware_mac[4], firmware_mac[5]);
seq_printf(seq, "\nAdapter Statistics:\n");
- seq_printf(seq, " TX: skbuffs linearized: %ld\n", adapter->tx_linearized);
- seq_printf(seq, " multi-descriptor sends: %ld\n", adapter->tx_multidesc_send);
- seq_printf(seq, " skb_linearize failures: %ld\n", adapter->tx_linearize_failed);
- seq_printf(seq, " vio_map_single failres: %ld\n", adapter->tx_map_failed);
+ seq_printf(seq, " TX: vio_map_single failres: %ld\n", adapter->tx_map_failed);
seq_printf(seq, " send failures: %ld\n", adapter->tx_send_failed);
seq_printf(seq, " RX: replenish task cycles: %ld\n", adapter->replenish_task_cycles);
seq_printf(seq, " alloc_skb_failures: %ld\n", adapter->replenish_no_mem);
diff -puN drivers/net/ibmveth.h~ibmveth_remove_frag drivers/net/ibmveth.h
--- linux-2.6/drivers/net/ibmveth.h~ibmveth_remove_frag 2007-08-06 14:11:08.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h 2007-08-06 14:11:39.000000000 -0500
@@ -25,8 +25,6 @@
#ifndef _IBMVETH_H
#define _IBMVETH_H
-#define IbmVethMaxSendFrags 6
-
/* constants for H_MULTICAST_CTRL */
#define IbmVethMcastReceptionModifyBit 0x80000UL
#define IbmVethMcastReceptionEnableBit 0x20000UL
@@ -146,9 +144,6 @@ struct ibmveth_adapter {
u64 replenish_add_buff_success;
u64 rx_invalid_buffer;
u64 rx_no_buffer;
- u64 tx_multidesc_send;
- u64 tx_linearized;
- u64 tx_linearize_failed;
u64 tx_map_failed;
u64 tx_send_failed;
spinlock_t stats_lock;
_
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 6/6] ibmveth: Remove use of bitfields
[not found] <11864293333377-patch-mail.ibm.com>
` (3 preceding siblings ...)
2007-08-06 19:42 ` [PATCH 5/6] ibmveth: Remove dead frag processing code Brian King
@ 2007-08-06 19:42 ` Brian King
2007-08-07 21:56 ` Jeff Garzik
2007-08-07 21:46 ` [PATCH 1/6] ibmveth: Enable TCP checksum offload Jeff Garzik
5 siblings, 1 reply; 15+ messages in thread
From: Brian King @ 2007-08-06 19:42 UTC (permalink / raw)
To: santil; +Cc: rcjenn, netdev, linuxppc-dev, brking
Removes the use of bitfields from the ibmveth driver. This results
in slightly smaller object code.
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
---
linux-2.6-bjking1/drivers/net/ibmveth.c | 90 ++++++++++++++++----------------
linux-2.6-bjking1/drivers/net/ibmveth.h | 56 ++++++++-----------
2 files changed, 68 insertions(+), 78 deletions(-)
diff -puN drivers/net/ibmveth.h~ibmveth_nobitfields drivers/net/ibmveth.h
--- linux-2.6/drivers/net/ibmveth.h~ibmveth_nobitfields 2007-08-06 14:11:11.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h 2007-08-06 14:11:11.000000000 -0500
@@ -39,6 +39,12 @@
#define IbmVethMcastRemoveFilter 0x2UL
#define IbmVethMcastClearFilterTable 0x3UL
+#define IBMVETH_ILLAN_PADDED_PKT_CSUM 0x0000000000002000ULL
+#define IBMVETH_ILLAN_TRUNK_PRI_MASK 0x0000000000000F00ULL
+#define IBMVETH_ILLAN_IPV6_TCP_CSUM 0x0000000000000004ULL
+#define IBMVETH_ILLAN_IPV4_TCP_CSUM 0x0000000000000002ULL
+#define IBMVETH_ILLAN_ACTIVE_TRUNK 0x0000000000000001ULL
+
/* hcall macros */
#define h_register_logical_lan(ua, buflst, rxq, fltlst, mac) \
plpar_hcall_norets(H_REGISTER_LOGICAL_LAN, ua, buflst, rxq, fltlst, mac)
@@ -150,13 +156,13 @@ struct ibmveth_adapter {
};
struct ibmveth_buf_desc_fields {
- u32 valid : 1;
- u32 toggle : 1;
- u32 reserved : 4;
- u32 no_csum : 1;
- u32 csum_good : 1;
- u32 length : 24;
- u32 address;
+ u32 flags_len;
+#define IBMVETH_BUF_VALID 0x80000000
+#define IBMVETH_BUF_TOGGLE 0x40000000
+#define IBMVETH_BUF_NO_CSUM 0x02000000
+#define IBMVETH_BUF_CSUM_GOOD 0x01000000
+#define IBMVETH_BUF_LEN_MASK 0x00FFFFFF
+ u32 address;
};
union ibmveth_buf_desc {
@@ -164,33 +170,17 @@ union ibmveth_buf_desc {
struct ibmveth_buf_desc_fields fields;
};
-struct ibmveth_illan_attributes_fields {
- u32 reserved;
- u32 reserved2 : 18;
- u32 csum_offload_padded_pkt_support : 1;
- u32 reserved3 : 1;
- u32 trunk_priority : 4;
- u32 reserved4 : 5;
- u32 tcp_csum_offload_ipv6 : 1;
- u32 tcp_csum_offload_ipv4 : 1;
- u32 active_trunk : 1;
-};
-
-union ibmveth_illan_attributes {
- u64 desc;
- struct ibmveth_illan_attributes_fields fields;
-};
-
struct ibmveth_rx_q_entry {
- u16 toggle : 1;
- u16 valid : 1;
- u16 reserved : 4;
- u16 no_csum : 1;
- u16 csum_good : 1;
- u16 reserved2 : 8;
- u16 offset;
- u32 length;
- u64 correlator;
+ u32 flags_off;
+#define IBMVETH_RXQ_TOGGLE 0x80000000
+#define IBMVETH_RXQ_TOGGLE_SHIFT 31
+#define IBMVETH_RXQ_VALID 0x40000000
+#define IBMVETH_RXQ_NO_CSUM 0x02000000
+#define IBMVETH_RXQ_CSUM_GOOD 0x01000000
+#define IBMVETH_RXQ_OFF_MASK 0x0000FFFF
+
+ u32 length;
+ u64 correlator;
};
#endif /* _IBMVETH_H */
diff -puN drivers/net/ibmveth.c~ibmveth_nobitfields drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_nobitfields 2007-08-06 14:11:11.000000000 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-06 14:11:11.000000000 -0500
@@ -131,19 +131,29 @@ struct ibmveth_stat ibmveth_stats[] = {
};
/* simple methods of getting data from the current rxq entry */
+static inline u32 ibmveth_rxq_flags(struct ibmveth_adapter *adapter)
+{
+ return adapter->rx_queue.queue_addr[adapter->rx_queue.index].flags_off;
+}
+
+static inline int ibmveth_rxq_toggle(struct ibmveth_adapter *adapter)
+{
+ return (ibmveth_rxq_flags(adapter) & IBMVETH_RXQ_TOGGLE) >> IBMVETH_RXQ_TOGGLE_SHIFT;
+}
+
static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
{
- return (adapter->rx_queue.queue_addr[adapter->rx_queue.index].toggle == adapter->rx_queue.toggle);
+ return (ibmveth_rxq_toggle(adapter) == adapter->rx_queue.toggle);
}
static inline int ibmveth_rxq_buffer_valid(struct ibmveth_adapter *adapter)
{
- return (adapter->rx_queue.queue_addr[adapter->rx_queue.index].valid);
+ return (ibmveth_rxq_flags(adapter) & IBMVETH_RXQ_VALID);
}
static inline int ibmveth_rxq_frame_offset(struct ibmveth_adapter *adapter)
{
- return (adapter->rx_queue.queue_addr[adapter->rx_queue.index].offset);
+ return (ibmveth_rxq_flags(adapter) & IBMVETH_RXQ_OFF_MASK);
}
static inline int ibmveth_rxq_frame_length(struct ibmveth_adapter *adapter)
@@ -153,7 +163,7 @@ static inline int ibmveth_rxq_frame_leng
static inline int ibmveth_rxq_csum_good(struct ibmveth_adapter *adapter)
{
- return (adapter->rx_queue.queue_addr[adapter->rx_queue.index].csum_good);
+ return (ibmveth_rxq_flags(adapter) & IBMVETH_RXQ_CSUM_GOOD);
}
/* setup the initial settings for a buffer pool */
@@ -253,9 +263,7 @@ static void ibmveth_replenish_buffer_poo
correlator = ((u64)pool->index << 32) | index;
*(u64*)skb->data = correlator;
- desc.desc = 0;
- desc.fields.valid = 1;
- desc.fields.length = pool->buff_size;
+ desc.fields.flags_len = IBMVETH_BUF_VALID | pool->buff_size;
desc.fields.address = dma_addr;
lpar_rc = h_add_logical_lan_buffer(adapter->vdev->unit_address, desc.desc);
@@ -396,9 +404,8 @@ static void ibmveth_rxq_recycle_buffer(s
return;
}
- desc.desc = 0;
- desc.fields.valid = 1;
- desc.fields.length = adapter->rx_buff_pool[pool].buff_size;
+ desc.fields.flags_len = IBMVETH_BUF_VALID |
+ adapter->rx_buff_pool[pool].buff_size;
desc.fields.address = adapter->rx_buff_pool[pool].dma_addr[index];
lpar_rc = h_add_logical_lan_buffer(adapter->vdev->unit_address, desc.desc);
@@ -549,9 +556,7 @@ static int ibmveth_open(struct net_devic
memcpy(&mac_address, netdev->dev_addr, netdev->addr_len);
mac_address = mac_address >> 16;
- rxq_desc.desc = 0;
- rxq_desc.fields.valid = 1;
- rxq_desc.fields.length = adapter->rx_queue.queue_len;
+ rxq_desc.fields.flags_len = IBMVETH_BUF_VALID | adapter->rx_queue.queue_len;
rxq_desc.fields.address = adapter->rx_queue.queue_dma;
ibmveth_debug_printk("buffer list @ 0x%p\n", adapter->buffer_list_addr);
@@ -686,7 +691,7 @@ static int ibmveth_set_csum_offload(stru
void (*done) (struct net_device *, u32))
{
struct ibmveth_adapter *adapter = dev->priv;
- union ibmveth_illan_attributes set_attr, clr_attr, ret_attr;
+ u64 set_attr, clr_attr, ret_attr;
long ret;
int rc1 = 0, rc2 = 0;
int restart = 0;
@@ -698,21 +703,21 @@ static int ibmveth_set_csum_offload(stru
adapter->pool_config = 0;
}
- set_attr.desc = 0;
- clr_attr.desc = 0;
+ set_attr = 0;
+ clr_attr = 0;
if (data)
- set_attr.fields.tcp_csum_offload_ipv4 = 1;
+ set_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
else
- clr_attr.fields.tcp_csum_offload_ipv4 = 1;
+ clr_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
- ret = h_illan_attributes(adapter->vdev->unit_address, 0, 0, &ret_attr.desc);
+ ret = h_illan_attributes(adapter->vdev->unit_address, 0, 0, &ret_attr);
- if (ret == H_SUCCESS && !ret_attr.fields.active_trunk &&
- !ret_attr.fields.trunk_priority &&
- ret_attr.fields.csum_offload_padded_pkt_support) {
- ret = h_illan_attributes(adapter->vdev->unit_address, clr_attr.desc,
- set_attr.desc, &ret_attr.desc);
+ if (ret == H_SUCCESS && !(ret_attr & IBMVETH_ILLAN_ACTIVE_TRUNK) &&
+ !(ret_attr & IBMVETH_ILLAN_TRUNK_PRI_MASK) &&
+ (ret_attr & IBMVETH_ILLAN_PADDED_PKT_CSUM)) {
+ ret = h_illan_attributes(adapter->vdev->unit_address, clr_attr,
+ set_attr, &ret_attr);
if (ret != H_SUCCESS) {
rc1 = -EIO;
@@ -720,13 +725,13 @@ static int ibmveth_set_csum_offload(stru
" %d rc=%ld\n", data, ret);
ret = h_illan_attributes(adapter->vdev->unit_address,
- set_attr.desc, clr_attr.desc, &ret_attr.desc);
+ set_attr, clr_attr, &ret_attr);
} else
done(dev, data);
} else {
rc1 = -EIO;
ibmveth_error_printk("unable to change checksum offload settings."
- " %d rc=%ld ret_attr=%lx\n", data, ret, ret_attr.desc);
+ " %d rc=%ld ret_attr=%lx\n", data, ret, ret_attr);
}
if (restart)
@@ -832,11 +837,9 @@ static int ibmveth_start_xmit(struct sk_
unsigned int tx_send_failed = 0;
unsigned int tx_map_failed = 0;
- desc.desc = 0;
- desc.fields.length = skb->len;
+ desc.fields.flags_len = IBMVETH_BUF_VALID | skb->len;
desc.fields.address = dma_map_single(&adapter->vdev->dev, skb->data,
- desc.fields.length, DMA_TO_DEVICE);
- desc.fields.valid = 1;
+ skb->len, DMA_TO_DEVICE);
if (skb->ip_summed == CHECKSUM_PARTIAL &&
ip_hdr(skb)->protocol != IPPROTO_TCP && skb_checksum_help(skb)) {
@@ -848,8 +851,7 @@ static int ibmveth_start_xmit(struct sk_
if (skb->ip_summed == CHECKSUM_PARTIAL) {
unsigned char *buf = skb_transport_header(skb) + skb->csum_offset;
- desc.fields.no_csum = 1;
- desc.fields.csum_good = 1;
+ desc.fields.flags_len |= (IBMVETH_BUF_NO_CSUM | IBMVETH_BUF_CSUM_GOOD);
/* Need to zero out the checksum */
buf[0] = 0;
@@ -875,7 +877,8 @@ static int ibmveth_start_xmit(struct sk_
if(lpar_rc != H_SUCCESS && lpar_rc != H_DROPPED) {
ibmveth_error_printk("tx: h_send_logical_lan failed with rc=%ld\n", lpar_rc);
ibmveth_error_printk("tx: valid=%d, len=%d, address=0x%08x\n",
- desc.fields.valid, desc.fields.length, desc.fields.address);
+ (desc.fields.flags_len & IBMVETH_BUF_VALID) ? 1 : 0,
+ skb->len, desc.fields.address);
tx_send_failed++;
tx_dropped++;
} else {
@@ -885,7 +888,7 @@ static int ibmveth_start_xmit(struct sk_
}
dma_unmap_single(&adapter->vdev->dev, desc.fields.address,
- desc.fields.length, DMA_TO_DEVICE);
+ skb->len, DMA_TO_DEVICE);
out: spin_lock_irqsave(&adapter->stats_lock, flags);
adapter->stats.tx_dropped += tx_dropped;
@@ -1101,7 +1104,7 @@ static int __devinit ibmveth_probe(struc
long ret;
struct net_device *netdev;
struct ibmveth_adapter *adapter = NULL;
- union ibmveth_illan_attributes set_attr, ret_attr;
+ u64 set_attr, ret_attr;
unsigned char *mac_addr_p;
unsigned int *mcastFilterSize_p;
@@ -1196,23 +1199,20 @@ static int __devinit ibmveth_probe(struc
ibmveth_debug_printk("registering netdev...\n");
- ret = h_illan_attributes(dev->unit_address, 0, 0, &ret_attr.desc);
+ ret = h_illan_attributes(dev->unit_address, 0, 0, &ret_attr);
- if (ret == H_SUCCESS && !ret_attr.fields.active_trunk &&
- !ret_attr.fields.trunk_priority &&
- ret_attr.fields.csum_offload_padded_pkt_support) {
- set_attr.desc = 0;
- set_attr.fields.tcp_csum_offload_ipv4 = 1;
+ if (ret == H_SUCCESS && !(ret_attr & IBMVETH_ILLAN_ACTIVE_TRUNK) &&
+ !(ret_attr & IBMVETH_ILLAN_TRUNK_PRI_MASK) &&
+ (ret_attr & IBMVETH_ILLAN_PADDED_PKT_CSUM)) {
+ set_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
- ret = h_illan_attributes(dev->unit_address, 0, set_attr.desc,
- &ret_attr.desc);
+ ret = h_illan_attributes(dev->unit_address, 0, set_attr, &ret_attr);
if (ret == H_SUCCESS) {
adapter->rx_csum = 1;
netdev->features |= NETIF_F_IP_CSUM;
} else
- ret = h_illan_attributes(dev->unit_address, set_attr.desc,
- 0, &ret_attr.desc);
+ ret = h_illan_attributes(dev->unit_address, set_attr, 0, &ret_attr);
}
rc = register_netdev(netdev);
_
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 6/6] ibmveth: Remove use of bitfields
2007-08-06 19:42 ` [PATCH 6/6] ibmveth: Remove use of bitfields Brian King
@ 2007-08-07 21:56 ` Jeff Garzik
0 siblings, 0 replies; 15+ messages in thread
From: Jeff Garzik @ 2007-08-07 21:56 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
Brian King wrote:
> Removes the use of bitfields from the ibmveth driver. This results
> in slightly smaller object code.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> linux-2.6-bjking1/drivers/net/ibmveth.c | 90 ++++++++++++++++----------------
> linux-2.6-bjking1/drivers/net/ibmveth.h | 56 ++++++++-----------
> 2 files changed, 68 insertions(+), 78 deletions(-)
strong ACK :)
Though I also encourage you to avoid #defines for named constants, in
favor of
enum {
IBMVETH_BUF_VALID = (1U << 31),
IBMVETH_BUF_TOGGLE = (1U << 30),
IBMVETH_BUF_NO_CSUM = (1U << 25),
IBMVETH_BUF_CSUM_GOOD = (1U << 24),
IBMVETH_BUF_LEN_MASK = 0x00FFFFFF,
};
This illustrates:
1) The "1 << n" notation is FAR easier to read and compare with data
sheets. You're just adding to the trouble by requiring the reviewer's
brain to convert hex numbers to bits, even if most engineers can do this
in their sleep.
2) The named constants are available to the C compiler, which is more
friendly to debuggers. It also supplies type information to the C compiler.
3) Similar to #2, wading through C pre-processor output is much easier
when the symbols don't disappear.
These are recommendations, not requirements, but I've found these
techniques superior to cpp in many other drivers.
Jeff
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/6] ibmveth: Enable TCP checksum offload
[not found] <11864293333377-patch-mail.ibm.com>
` (4 preceding siblings ...)
2007-08-06 19:42 ` [PATCH 6/6] ibmveth: Remove use of bitfields Brian King
@ 2007-08-07 21:46 ` Jeff Garzik
5 siblings, 0 replies; 15+ messages in thread
From: Jeff Garzik @ 2007-08-07 21:46 UTC (permalink / raw)
To: Brian King; +Cc: linuxppc-dev, rcjenn, santil, netdev
Brian King wrote:
> This patchset enables TCP checksum offload support for IPV4
> on ibmveth. This completely eliminates the generation and checking of
> the checksum for packets that are completely virtual and never
> touch a physical network. A simple TCP_STREAM netperf run on
> a virtual network with maximum mtu set yielded a ~30% increase
> in throughput. This feature is enabled by default on systems that
> support it, but can be disabled with a module option.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
ACK, but does not apply to current netdev-2.6.git#upstream-fixes.
Request resend after the ibmveth fixes hit mainline (24 hours or so
after push, I suppose)
^ permalink raw reply [flat|nested] 15+ messages in thread