On 27-Nov-20 17:56, Andrew Lunn wrote:
OK, but if the caching system is checking one time netlink and one time
ioctl, it means this cache should be in user space, or did you mean to have
this cache in kernel ?
This is all in userspace, in the ethtool code.
What about the global offset that we currently got when user doesn't specify
a page, do you mean that this global offset goes through the optional and
non optional pages that exist and skip the ones that are missing according
to the specific EEPROM ?
ethtool -m|--dump-module-eeprom|--module-info devname [raw on|off] [hex on|off]
[offset N] [length N]
So you mean [offset N] [length N].
Yes, that's the current options and we can either try coding new
implementation for that or just call the current ioctl implementation. The
new code can be triggered once options [bank N] and [Page N] are used.
You cannot rely on the ioctl being available. New drivers won't
implement it, if they have the netlink code. Drivers will convert from
get_module_info to whatever new ndo call you add for netlink.
OK, if I got it right on current API [offset N] [length N] just call ioctl
current implementation, while using the option [raw on] will call new
implementation for new SFPs (CMIS 4). Also using [bank N] and [page N] will
call new implementation for new SFPs.
Not just CMIS. All SFPs.
Andrew
Hi Andrew,
Following this conversation, I wrote some pseudocode checking if I'm on
right path here.
Please review:
struct eeprom_page {
u8 page_number;
u8 bank_number;
u16 offset;
u16 data_length;
u8 *data;
}
Check every requested page, offset and length availability in userspace,
depending on module standard and EEPROM data.
================================================================================
cache_fetch_add(page_number, bank_number, offset, length)
{
ethnla_put_u8 (msgbuff, ETHTOOL_A_EEPROM_PAGE_NUMBER, page_number);
ethnla_put_u8 (msgbuff, ETHTOOL_A_EEPROM_BANK_NUMBER, bank_number);
ethnla_put_u16(msgbuff, ETHTOOL_A_EEPROM_PAGE_OFFSET, offset);
ethnla_put_u16(msgbuff, ETHTOOL_A_EEPROM_PAGE_LENGTH, length);
err = nlsock_send_get_request(eeprom_page_reply_cb) // caches a
page
if (err < 0)
return err;
}
// Any call should be a pair of cache_fetch_add() with error check and only
// then a cache_get(), but for pseudocode this will do
cache_get_page(page_number, bank_number, offset, length)
{
if (!cache_contains(page_number, bank_number, offset, length))
cache_fetch_add(page_number, bank_number, offset, length);
return cache_get(page_number, bank_number);
}
print_cmis_y()
{
page_data = cache_get_page(0, 0, 0, 256)->data;
print_page_zero(page_data);
page_data = cache_get_page(1, 0, 128, 128)->data;
print_page_one(page_data);
}
print_human_readable()
{
spec_id = cache_get_page(0, 0, 0, 128)->data[0];
switch (spec_id) {
case sff_xxxx:
print_sff_xxxx();
case cmis_y:
print_cmis_y();
default:
print_hex();
}
}
getmodule_reply_cb()
{
if (offset || hex || bank_number || page number)
print_hex();
else
// if _human_readable() decoder needs more than page
00, it will
// fetch it on demand
print_human_readable();
}
eeprom_page_reply_cb()
{
cache_add(new_page);
}
page_available(page_number)
{
spec_id = cache_get_page(0, 0, 0, 128)->data[0];
// check bits indicating page availability
switch (spec_id) {
case sff_xxxx:
return is_page_available_sff_xxxx(page_number);
case cmis_y:
return is_page_available_cmis_y(page_number, bank_number);
default:
return -EINVAL;
}
}
nl_getmodule()
{
msg_init();
// request first low page
ret = cache_fetch_add(0, 0, 0, 128);
if (ret < 0) // netlink unsupported, fall back to ioctl
return ret;
netlink_cmd_check();
nl_parser()
// check bits indicating page availability according to used spec
if (page_available(page_number, bank_number)) {
if (cache_contains(page_number, bank_number, offset,
length))
print_page(page_number, bank_number, offset,
length)
else {
ret = cache_fetch_add();
if (ret < 0)
return ret;
print_page(page_number, bank_number, offset,
length);
}
} else {
return -EINVAL;
}
}
Or just proceed to request any page and depend on kernel parameter
validation
================================================================================
getmodule_reply_cb()
{
if (offset || hex || bank_number || page_number)
print_hex();
else
// if _human_readable() decoder needs more than page
00, it will
// fetch it on demand
print_human_readable();
}
nl_getmodule()
{
// request page straight away
netlink_cmd_check();
nl_parser();
/*
* nl_parser() sets page_number, page_bank, offset and length and
* prepares to pass them to the kernel. See
eeprom_page_parse_request()
*/
ret = nlsock_sendmsg(getmodule_reply_cb);
if (ret < 0)
retrun ret;
}
Kernel
This part is the same for both userspace variants
================================================================================
kernel_fallback(request, data)
{
struct ethtool_eeprom eeprom;
if (request->bank_number)
return -EINVAL;
// also take into account page 00 size
eeprom.offset = request->offset + page_size * request->page_number;
eeprom.len = request->length;
eeprom.cmd = ETHTOOL_GMODULEEEPROM;
eeprom.data = data;
err = ops->get_module_eeprom(dev, &eeprom, eeprom.data)
if (err < 0)
return err;
// prepare data for response via netlink like for supported ndo
}
eeprom_page_parse_request()
{
struct eeprom_page_req_info *request;
request->offset = nla_get_u16(tb[ETHTOOL_A_EEPROM_PAGE_OFFSET]);
request->length = nla_get_u16(tb[ETHTOOL_A_EEPROM_PAGE_LENGTH]);
request->page_number =
nla_get_u8(tb[ETHTOOL_A_EEPROM_PAGE_NUMBER]);
request->bank_number =
nla_get_u8(tb[ETHTOOL_A_EEPROM_PAGE_BANK_NUMBER]);
}
eeprom_page_prepare_data(request)
{
u8 data[128];
u8 page_zero_data[128];
if (!ops->get_module_eeprom_page) {
if (ops->get_module_info && ops->get_module_eeprom)
return kernel_fallback(request, &data);
else
return -EOPNOTSUPP;
}
// fetch page 00 for further checks
err = ops->get_module_eeprom_page(dev, 0, 128, 0, 0,
&page_zero_data);
if (err < 0)
return err;
// Checks according to spec, similar to page_available() in
userspace.
// May be ommited and passed directly to the driver for it to
validate
if (is_request_valid(request, &page_zero_data))
err = ops->get_module_eeprom_page(dev, request->offset,
request->length,
request->page_number,
request->bank_number, &data);
else
return -EINVAL;
}
Driver
It is required to implement get_module_eeprom_page() ndo, where it queries
its EEPROM and copies to u8 *data array allocated by the kernel previously.
The ndo has the following prototype:
int get_module_eeprom_page(struct net_device *, u16 offset, u16 length,
u8 page_number, u8 bank_number, u8 *data);
The driver may test whatever it may need in this ndo implementation.
Its parameters may also be passed using a special struct.
===============================================================================
driver_get_module_eeprom_page()
{
vendor_query_module_eeprom(page_number, bank_number, offset,
length, data);
}
Thanks,
Vlad
