net: ethtool: Add capability to retrieve plug-in module EEPROM
Subsystems (3)
RFC PATCH 1/2 net: ethtool: Add capability to retrieve plug-in module EEPROM
Added extensions to the ethtool API to obtain plugin module eeprom data This is useful for end users to be able to determine what the capabilities of the in use module are.
The first provides a new struct ethtool_modinfo that will return the type and size of plug-in module eeprom (such as SFP+) for parsing by userland program.
The second provides the API to get the raw eeprom information using the existing ethtool_eeprom structture to return the data
--- include/linux/ethtool.h | 20 ++++++++++++ net/core/ethtool.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 0 deletions(-)
include/linux/ethtool.h +8 -0
@@ -117,6 +117,14 @@ struct ethtool_eeprom {__u8 data[0 ]; }; +/* for passing plug-in module information */ +struct ethtool_modinfo { +__u32 cmd; +__u32 type; +__u32 eeprom_len; +__u32 reserved[8 ]; +}; +/** * struct ethtool_coalesce - coalescing parameters for IRQs and stats
updates
* @cmd: ETHTOOL_{G,S}COALESCE
@@ -936,6 +944,10 @@ struct ethtool_ops {
int (*get_dump_data)(struct net_device *,
struct ethtool_dump *, void *);
int (*set_dump)(struct net_device *, struct ethtool_dump *);+ int (*get_module_info)(struct net_device *,
+ struct ethtool_modinfo *);
+ int (*get_module_eeprom)(struct net_device *,
+ struct ethtool_eeprom *, u8 *);
};
#endif /* __KERNEL__ */
@@ -1010,6 +1022,8 @@ struct ethtool_ops {
#define ETHTOOL_SET_DUMP 0x0000003e /* Set dump settings */
#define ETHTOOL_GET_DUMP_FLAG 0x0000003f /* Get dump settings */
#define ETHTOOL_GET_DUMP_DATA 0x00000040 /* Get dump data */
+#define ETHTOOL_GMODULEINFO 0x00000041 /* Get plug-in module information */
+#define ETHTOOL_GMODULEEEPROM 0x00000042 /* Get plug-in module eeprom */
/* compatibility with older code */
#define SPARC_ETH_GSET ETHTOOL_GSET
@@ -1159,6 +1173,12 @@ struct ethtool_ops {
#define RX_CLS_LOC_FIRST 0xfffffffe
#define RX_CLS_LOC_LAST 0xfffffffd
+/* EEPROM Standards for plug in modules */
+#define SFF_8079 0x1
+#define SFF_8079_LEN 256
+#define SFF_8472 0x2
+#define SFF_8472_LEN 512
+
/* Reset flags */
/* The reset() operation must clear the flags for the components which
* were actually reset. On successful return, the flags indicate the
net/core/ethtool.c +79 -0
91 lines not on this page · raw message
Signature · 1 line
1.7.7.6
We previously discussed the need for this in person, so I'm going to review this as a submission rather than an RFC.
Quoted 11 lines · Stuart Hodgson
Added extensions to the ethtool API to obtain plugin module eeprom data This is useful for end users to be able to determine what the capabilities of the in use module are.
The first provides a new struct
ethtool_modinfothat will return the type and size of plug-in module eeprom (such as SFP+) for parsing by userland program.The second provides the API to get the raw eeprom information using the existing
ethtool_eepromstructture to return the data
Quoted 6 lines
Signed-off-by Stuart Hodgson <[email protected]>--- include/linux/ethtool.h | 20 ++++++++++++ net/core/ethtool.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 0 deletions(-)
This is line-wrapped; you'll need to take care to avoid that when submitting the patch for real. Tabs have been converted to spaces, which you also need to avoid. See Documentation/email-clients.txt.
Quoted 6 lines
include/linux/ethtool.h +0
-0@@ -117,6 +117,14 @@ struct ethtool_eeprom {__u8 data[0 ];
include/linux/ethtool.h struct ethtool_eeprom { · line 127
Quoted 10 lines
include/linux/ethtool.h +8
-0}; +/* for passing plug-in module information */ +struct ethtool_modinfo { +__u32 cmd; +__u32 type; +__u32 eeprom_len; +__u32 reserved[8 ]; +}; +
This needs a proper kernel-doc comment with a description of each of the fields (except reserved).
include/linux/ethtool.h struct ethtool_ops { · line 950
Quoted 12 lines
include/linux/ethtool.h +0
-0/** * struct ethtool_coalesce - coalescing parameters for IRQs and stats updates
* @cmd: ETHTOOL_{G,S}COALESCEinclude/linux/ethtool.h +4
-0@@ -936,6 +944,10 @@ struct ethtool_ops {int (*get_dump_data)(struct net_device *,struct ethtool_dump *,void *);int (*set_dump)(struct net_device *,struct ethtool_dump *); +int (*get_module_info)(struct net_device *, +struct ethtool_modinfo *); +int (*get_module_eeprom)(struct net_device *, +struct ethtool_eeprom *,u8 *);
These need to be described in the kernel-doc comment for this structure.
Quoted 13 lines
include/linux/ethtool.h +1
-0};#endif /* __KERNEL__ */ @@ -1010,6 +1022,8 @@ struct ethtool_ops {#define ETHTOOL_SET_DUMP 0x0000003e/* Set dump settings */ #define ETHTOOL_GET_DUMP_FLAG 0x0000003f/* Get dump settings */ #define ETHTOOL_GET_DUMP_DATA 0x00000040/* Get dump data */ +#define ETHTOOL_GMODULEINFO 0x00000041/* Get plug-in module information */
include/linux/ethtool.h +1
-0+#define ETHTOOL_GMODULEEEPROM 0x00000042/* Get plug-in module eeprom */ /* compatibility with older code */ #define SPARC_ETH_GSET ETHTOOL_GSET
include/linux/ethtool.h struct ethtool_ops { · line 1181
Quoted 9 lines
include/linux/ethtool.h +6
-0@@ -1159,6 +1173,12 @@ struct ethtool_ops {#define RX_CLS_LOC_FIRST 0xfffffffe#define RX_CLS_LOC_LAST 0xfffffffd +/* EEPROM Standards for plug in modules */ +#define SFF_8079 0x1 +#define SFF_8079_LEN 256 +#define SFF_8472 0x2 +#define SFF_8472_LEN 512 +
I think it's best to include a prefix of 'ethtool_' 'ETHTOOL_' or 'ETH_' in new definitions in ethtool.h. Since these are specific to modules I would suggest a prefix of 'ETH_MODULE_'.
Quoted 7 lines
include/linux/ethtool.h +0
-0/* Reset flags */ /* The reset() operation must clear the flags for the components which * were actually reset. On successful return, the flags indicate the net/core/ethtool.c +0
-0
[...]
Quoted 21 lines
net/core/ethtool.c +21
-0+static int ethtool_get_module_eeprom (struct net_device *dev, +void __user *useraddr) +{ +int ret; +struct ethtool_eeprom eeprom; +struct ethtool_modinfo modinfo; +const struct ethtool_ops *ops = dev->ethtool_ops; +void __user *userbuf = useraddr +sizeof (eeprom); +u8 *data; + +if (!ops->get_module_info || !ops->get_module_eeprom) +return -EOPNOTSUPP ; + +if (copy_from_user (&eeprom, useraddr,sizeof (eeprom))) +return -EFAULT ; + +/* Check for wrap and zero */ +if (eeprom.offset + eeprom.len <= eeprom.offset) +return -EINVAL ; + +/* Get the modinfo to get the length */
net/core/ethtool.c out: · line 1332
Quoted 10 lines
net/core/ethtool.c +10
-0+ ret = ops->get_module_info (dev, &modinfo); +if (ret) +return ret; + +if (eeprom.offset + eeprom.len > modinfo.eeprom_len) +return -EINVAL ; + + data =kmalloc (PAGE_SIZE ,GFP_USER ); +if (!data) +return -ENOMEM ;
What if some device has a larger EEPROM? Surely this length should be eeprom.len.
net/core/ethtool.c out: · line 1345
Quoted 12 lines
net/core/ethtool.c +12
-0+ ret = ops->get_module_eeprom (dev, &eeprom, data); +if (ret) + goto out; + + +if (copy_to_user (userbuf, data, eeprom.len)) { + ret = -EFAULT ; + goto out; + } + +if (copy_to_user (useraddr, &eeprom,sizeof (eeprom))) + ret = -EFAULT ;
[...]
I think you can drop this last copy as there's no information to return in the eeprom structure itself.
Ben.
Signature · 3 lines
Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
On Mon, 2012-04-02 at 18:52 +0100, Ben Hutchings wrote:
[...]
Quoted 14 lines
Quoted 12 lines
net/core/ethtool.c +12
-0+ ret = ops->get_module_eeprom (dev, &eeprom, data); +if (ret) + goto out; + + +if (copy_to_user (userbuf, data, eeprom.len)) { + ret = -EFAULT ; + goto out; + } + +if (copy_to_user (useraddr, &eeprom,sizeof (eeprom))) + ret = -EFAULT ;[...]
Quoted 2 lines
I think you can drop this last copy as there's no information to return in the eeprom structure itself.
[...]
This is not the case because we need to cover short reads.
Ben.
Signature · 3 lines
Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
Quoted 122 lines · Ben Hutchings
We previously discussed the need for this in person, so I'm going to review this as a submission rather than an RFC.
Quoted 11 lines · Stuart Hodgson
Added extensions to the ethtool API to obtain plugin module eeprom data This is useful for end users to be able to determine what the capabilities of the in use module are.
The first provides a new struct
ethtool_modinfothat will return the type and size of plug-in module eeprom (such as SFP+) for parsing by userland program.The second provides the API to get the raw eeprom information using the existing
ethtool_eepromstructture to return the dataQuoted 6 lines
Signed-off-by Stuart Hodgson<[email protected]>--- include/linux/ethtool.h | 20 ++++++++++++ net/core/ethtool.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 0 deletions(-)This is line-wrapped; you'll need to take care to avoid that when submitting the patch for real. Tabs have been converted to spaces, which you also need to avoid. See
Documentation/email-clients.txt.Quoted 6 lines
include/linux/ethtool.h +0
-0@@ -117,6 +117,14 @@ struct ethtool_eeprom {__u8 data[0 ];include/linux/ethtool.h struct ethtool_eeprom { · line 127
Quoted 10 lines
include/linux/ethtool.h +8
-0}; +/* for passing plug-in module information */ +struct ethtool_modinfo { +__u32 cmd; +__u32 type; +__u32 eeprom_len; +__u32 reserved[8 ]; +}; +This needs a proper kernel-doc comment with a description of each of the fields (except reserved).
include/linux/ethtool.h struct ethtool_ops { · line 950
Quoted 12 lines
include/linux/ethtool.h +0
-0/** * struct ethtool_coalesce - coalescing parameters for IRQs and stats updates
* @cmd: ETHTOOL_{G,S}COALESCEinclude/linux/ethtool.h +4
-0@@ -936,6 +944,10 @@ struct ethtool_ops {int (*get_dump_data)(struct net_device *,struct ethtool_dump *,void *);int (*set_dump)(struct net_device *,struct ethtool_dump *); +int (*get_module_info)(struct net_device *, +struct ethtool_modinfo *); +int (*get_module_eeprom)(struct net_device *, +struct ethtool_eeprom *,u8 *);These need to be described in the kernel-doc comment for this structure.
Quoted 13 lines
include/linux/ethtool.h +1
-0};#endif /* __KERNEL__ */ @@ -1010,6 +1022,8 @@ struct ethtool_ops {#define ETHTOOL_SET_DUMP 0x0000003e/* Set dump settings */ #define ETHTOOL_GET_DUMP_FLAG 0x0000003f/* Get dump settings */ #define ETHTOOL_GET_DUMP_DATA 0x00000040/* Get dump data */ +#define ETHTOOL_GMODULEINFO 0x00000041/* Get plug-in module information */
include/linux/ethtool.h +1
-0+#define ETHTOOL_GMODULEEEPROM 0x00000042/* Get plug-in module eeprom */ /* compatibility with older code */ #define SPARC_ETH_GSET ETHTOOL_GSETinclude/linux/ethtool.h struct ethtool_ops { · line 1181
Quoted 9 lines
include/linux/ethtool.h +6
-0@@ -1159,6 +1173,12 @@ struct ethtool_ops {#define RX_CLS_LOC_FIRST 0xfffffffe#define RX_CLS_LOC_LAST 0xfffffffd +/* EEPROM Standards for plug in modules */ +#define SFF_8079 0x1 +#define SFF_8079_LEN 256 +#define SFF_8472 0x2 +#define SFF_8472_LEN 512 +I think it's best to include a prefix of '
ethtool_' 'ETHTOOL_' or 'ETH_' in new definitions inethtool.h. Since these are specific to modules I would suggest a prefix of 'ETH_MODULE_'.Quoted 7 lines
include/linux/ethtool.h +0
-0/* Reset flags */ /* The reset() operation must clear the flags for the components which * were actually reset. On successful return, the flags indicate the net/core/ethtool.c +0
-0[...]
Quoted 26 lines
net/core/ethtool.c +26
-0+static int ethtool_get_module_eeprom (struct net_device *dev, +void __user *useraddr) +{ +int ret; +struct ethtool_eeprom eeprom; +struct ethtool_modinfo modinfo; +const struct ethtool_ops *ops = dev->ethtool_ops; +void __user *userbuf = useraddr +sizeof (eeprom); +u8 *data; + +if (!ops->get_module_info || !ops->get_module_eeprom) +return -EOPNOTSUPP ; + +if (copy_from_user (&eeprom, useraddr,sizeof (eeprom))) +return -EFAULT ; + +/* Check for wrap and zero */ +if (eeprom.offset + eeprom.len<= eeprom.offset) +return -EINVAL ; + +/* Get the modinfo to get the length */ + ret = ops->get_module_info (dev,&modinfo); +if (ret) +return ret; + +if (eeprom.offset + eeprom.len> modinfo.eeprom_len)
Quoted 9 lines
net/core/ethtool.c out: · line 1332
Quoted 5 lines
net/core/ethtool.c +5
-0+return -EINVAL ; + + data =kmalloc (PAGE_SIZE ,GFP_USER ); +if (!data) +return -ENOMEM ;What if some device has a larger EEPROM? Surely this length should be eeprom.len.
Do you mean what if the eeprom length in te device is larger than PAGE_SIZE? If so then it should really use modinfo.eeprom_len since this the size of the data. eeprom.len could be arbitary.
Quoted 14 lines
Quoted 12 lines
net/core/ethtool.c +12
-0+ ret = ops->get_module_eeprom (dev,&eeprom, data); +if (ret) + goto out; + + +if (copy_to_user (userbuf, data, eeprom.len)) { + ret = -EFAULT ; + goto out; + } + +if (copy_to_user (useraddr,&eeprom,sizeof (eeprom))) + ret = -EFAULT ;[...]
Quoted 5 lines
I think you can drop this last copy as there's no information to return in the eeprom structure itself.
Ben.
Other comments have been addressed and will be re-submitted.
Stu
Quoted 1 line · Stuart Hodgson
On 02/04/12 18:52, Ben Hutchings wrote:
[...]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
net/core/ethtool.c +0
-0
[...]
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
net/core/ethtool.c +6
-0+if (eeprom.offset + eeprom.len> modinfo.eeprom_len) +return -EINVAL ; + + data =kmalloc (PAGE_SIZE ,GFP_USER ); +if (!data) +return -ENOMEM ;
Quoted 6 lines
Quoted 3 lines
What if some device has a larger EEPROM? Surely this length should be eeprom.len.
Do you mean what if the eeprom length in te device is larger than
PAGE_SIZE?
Yes.
Quoted 2 lines
If so then it should really use modinfo.eeprom_len since this the size of the data. eeprom.len could be arbitary.
No, eeprom.len is the size of the data and we've already validated it at this point.
Ben.
Quoted 20 lines · Ben Hutchings
Quoted 1 line · Stuart Hodgson
On 02/04/12 18:52, Ben Hutchings wrote:
[...]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
net/core/ethtool.c +0
-0[...]
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
net/core/ethtool.c +6
-0+if (eeprom.offset + eeprom.len> modinfo.eeprom_len) +return -EINVAL ; + + data =kmalloc (PAGE_SIZE ,GFP_USER ); +if (!data) +return -ENOMEM ;Quoted 6 lines
Quoted 3 lines
What if some device has a larger EEPROM? Surely this length should be eeprom.len.
Do you mean what if the eeprom length in te device is larger than
PAGE_SIZE?
Quoted 7 lines
Yes.
Quoted 2 lines
If so then it should really use modinfo.eeprom_len since this the size of the data. eeprom.len could be arbitary.
No, eeprom.len is the size of the data and we've already validated it at this point.
Maybe we should start by refactoring ethtool_get_eeprom() so we can reuse most of its code in ethtool_get_module_eeprom(), rather than having to worry about what the maximum size of a module EEPROM might be and whether we need a loop:
Subject: ethtool: Split ethtool_get_eeprom() to allow for additional EEPROM accessors
We want to support reading module (SFP+, XFP, ...) EEPROMs as well as NIC EEPROMs. They will need a different command number and driver operation, but the structure and arguments will be the same and so we can share most of the code here.
--- net/core/ethtool.c | 24 +++++++++++++++++------- 1 files changed, 17 insertions(+), 7 deletions(-)
net/core/ethtool.c +17 -7
@@ -751,18 +751,17 @@ static int ethtool_get_link(struct net_device *dev, char __user *useraddr)return 0 ; } -static int ethtool_get_eeprom (struct net_device *dev,void __user *useraddr) +static int ethtool_get_any_eeprom (struct net_device *dev,void __user *useraddr, +int (*getter)(struct net_device *, +struct ethtool_eeprom *,u8 *), +u32 total_len) {struct ethtool_eeprom eeprom; -const struct ethtool_ops *ops = dev->ethtool_ops;void __user *userbuf = useraddr +sizeof (eeprom);u32 bytes_remaining;u8 *data;int ret =0 ; -if (!ops->get_eeprom || !ops->get_eeprom_len) -return -EOPNOTSUPP ; -if (copy_from_user (&eeprom, useraddr,sizeof (eeprom)))return -EFAULT ;
@@ -771,7 +770,7 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)return -EINVAL ;/* Check for exceeding total eeprom len */ -if (eeprom.offset + eeprom.len > ops->get_eeprom_len (dev)) +if (eeprom.offset + eeprom.len > total_len)return -EINVAL ; data =kmalloc (PAGE_SIZE ,GFP_USER );
@@ -782,7 +781,7 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)while (bytes_remaining >0 ) { eeprom.len =min (bytes_remaining, (u32 )PAGE_SIZE ); - ret = ops->get_eeprom (dev, &eeprom, data); + ret =getter (dev, &eeprom, data);if (ret)break ;if (copy_to_user (userbuf, data, eeprom.len)) {
@@ -803,6 +802,17 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)return ret; } +static int ethtool_get_eeprom (struct net_device *dev,void __user *useraddr) +{ +const struct ethtool_ops *ops = dev->ethtool_ops; + +if (!ops->get_eeprom || !ops->get_eeprom_len) +return -EOPNOTSUPP ; + +return ethtool_get_any_eeprom (dev, useraddr, ops->get_eeprom, + ops->get_eeprom_len (dev)); +} +static int ethtool_set_eeprom (struct net_device *dev,void __user *useraddr) {struct ethtool_eeprom eeprom;
Signature · 6 lines
1.7.7.6 -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
Quoted 74 lines · Ben Hutchings
Quoted 20 lines · Ben Hutchings
Quoted 1 line · Stuart Hodgson
On 02/04/12 18:52, Ben Hutchings wrote:
[...]
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
net/core/ethtool.c +0
-0[...]
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
net/core/ethtool.c +6
-0+if (eeprom.offset + eeprom.len> modinfo.eeprom_len) +return -EINVAL ; + + data =kmalloc (PAGE_SIZE ,GFP_USER ); +if (!data) +return -ENOMEM ;Quoted 6 lines
Quoted 3 lines
What if some device has a larger EEPROM? Surely this length should be eeprom.len.
Do you mean what if the eeprom length in te device is larger than
PAGE_SIZE?Quoted 7 lines
Yes.
Quoted 2 lines
If so then it should really use modinfo.eeprom_len since this the size of the data. eeprom.len could be arbitary.
No, eeprom.len is the size of the data and we've already validated it at this point.
Maybe we should start by refactoring
ethtool_get_eeprom()so we can reuse most of its code inethtool_get_module_eeprom(), rather than having to worry about what the maximum size of a module EEPROM might be and whether we need a loop:Subject: ethtool: Split
ethtool_get_eeprom()to allow for additional EEPROM accessorsWe want to support reading module (SFP+, XFP, ...) EEPROMs as well as NIC EEPROMs. They will need a different command number and driver operation, but the structure and arguments will be the same and so we can share most of the code here.
Signed-off-by Ben Hutchings<[email protected]>--- net/core/ethtool.c | 24 +++++++++++++++++------- 1 files changed, 17 insertions(+), 7 deletions(-)net/core/ethtool.c +4
-5@@ -751,18 +751,17 @@ static int ethtool_get_link(struct net_device *dev, char __user *useraddr)return 0 ; } -static int ethtool_get_eeprom (struct net_device *dev,void __user *useraddr) +static int ethtool_get_any_eeprom (struct net_device *dev,void __user *useraddr, +int (*getter)(struct net_device *, +struct ethtool_eeprom *,u8 *), +u32 total_len) {struct ethtool_eeprom eeprom; -const struct ethtool_ops *ops = dev->ethtool_ops;void __user *userbuf = useraddr +sizeof (eeprom);u32 bytes_remaining;u8 *data;int ret =0 ; -if (!ops->get_eeprom || !ops->get_eeprom_len) -return -EOPNOTSUPP ; -if (copy_from_user (&eeprom, useraddr,sizeof (eeprom)))return -EFAULT ;
net/core/ethtool.c static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr) · line 776
Quoted 8 lines
net/core/ethtool.c +1
-1@@ -771,7 +770,7 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)return -EINVAL ;/* Check for exceeding total eeprom len */ -if (eeprom.offset + eeprom.len> ops->get_eeprom_len (dev)) +if (eeprom.offset + eeprom.len> total_len)return -EINVAL ; data =kmalloc (PAGE_SIZE ,GFP_USER );
Should this not be eeprom.len?
Quoted 17 lines
net/core/ethtool.c +5
-1@@ -782,7 +781,7 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)while (bytes_remaining>0 ) { eeprom.len =min (bytes_remaining, (u32 )PAGE_SIZE ); - ret = ops->get_eeprom (dev,&eeprom, data); + ret =getter (dev,&eeprom, data);if (ret)break ;if (copy_to_user (userbuf, data, eeprom.len)) {@@ -803,6 +802,17 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)return ret; } +static int ethtool_get_eeprom (struct net_device *dev,void __user *useraddr) +{ +const struct ethtool_ops *ops = dev->ethtool_ops; +
net/core/ethtool.c static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr) · line 818
Quoted 10 lines
net/core/ethtool.c +7
-0+if (!ops->get_eeprom || !ops->get_eeprom_len) +return -EOPNOTSUPP ; + +return ethtool_get_any_eeprom (dev, useraddr, ops->get_eeprom, + ops->get_eeprom_len (dev)); +} +static int ethtool_set_eeprom (struct net_device *dev,void __user *useraddr) {struct ethtool_eeprom eeprom;
This would reduce the code size nicely between the two eeprom fetches.
Stu
Quoted 1 line · Stuart Hodgson
On 12/04/12 00:42, Ben Hutchings wrote:
[...]
Quoted 6 lines
Quoted 6 lines
Maybe we should start by refactoring
ethtool_get_eeprom()so we can reuse most of its code inethtool_get_module_eeprom(), rather than having to worry about what the maximum size of a module EEPROM might be and whether we need a loop:Subject: ethtool: Split
ethtool_get_eeprom()to allow for additional EEPROM accessors
[...]
Quoted 11 lines
net/core/ethtool.c static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr) · line 776
Quoted 9 lines
net/core/ethtool.c +1
-1@@ -771,7 +770,7 @@ static int ethtool_get_eeprom(struct net_device *dev, void __user *useraddr)return -EINVAL ;/* Check for exceeding total eeprom len */ -if (eeprom.offset + eeprom.len> ops->get_eeprom_len (dev)) +if (eeprom.offset + eeprom.len> total_len)return -EINVAL ; data =kmalloc (PAGE_SIZE ,GFP_USER );Should this not be eeprom.len?
[...]
No, because this function loops over PAGE_SIZE chunks. That's presumably necessary for large NIC EEPROMs, and if we reuse it we won't have to wory about whether it's ever necessary for large module EEPROMs.
Ben.
Signature · 3 lines
Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.