lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 29 Dec 2020 17:25:14 +0100 From: Andrew Lunn <andrew@...n.ch> To: Vladyslav Tarasiuk <vladyslavt@...dia.com> Cc: Moshe Shemesh <moshe@...dia.com>, Jakub Kicinski <kuba@...nel.org>, Moshe Shemesh <moshe@...lanox.com>, "David S. Miller" <davem@...emloft.net>, Adrian Pop <pop.adrian61@...il.com>, Michal Kubecek <mkubecek@...e.cz>, netdev@...r.kernel.org, Maxim Mikityanskiy <maximmi@...dia.com> Subject: Re: [PATCH net-next v2 0/2] Add support for DSFP transceiver type > 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; > } I'm wondering about offset and data_length, in this context. I would expect you always ask the kernel for the full page, not part of it. Even when user space asks for just part of a page. That keeps you cache management simpler. But maybe some indicator of low/high is needed, since many pages are actually 1/2 pages? The other thing to consider is SFF-8472 and its use of two different i2c addresses, A0h and A2h. These are different to pages and banks. > 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(); > } > } You want to keep as much of the existing ethtool code as you can, but the basic idea looks O.K. > 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(); > } Things get interesting here. Say this is page 0, and print_human_readable() finds a bit indicating page 1 is valid. So it requests page 1. We go recursive. While deep down in print_human_readable(), we send the next netlink message and call getmodule_reply_cb() when the answer appears. I've had problems with some of the netlink code not liking recursive calls. So i suggest you try to find a different structure for the code. Try to complete the netlink call before doing the decoding. So add the page to the cache and then return. Do the decode after nlsock_sendmsg() has returned. > 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); I would include extack here, so we can get better error messages. Andrew
Powered by blists - more mailing lists