From: Stefan Assmann <sassmann@kpanic.de>
To: Hauke Mehrtens <hauke@hauke-m.de>
Cc: mcgrof@kernel.org, backports@vger.kernel.org
Subject: Re: [PATCH RFC 05/10] backports: igb fixes for linux-3.6
Date: Fri, 13 Dec 2013 09:01:22 +0100 [thread overview]
Message-ID: <52AABED2.7000506@kpanic.de> (raw)
In-Reply-To: <52AA5901.2060601@hauke-m.de>
On 13.12.2013 01:46, Hauke Mehrtens wrote:
> On 12/12/2013 10:27 AM, Stefan Assmann wrote:
>> - backport ethtool_cmd
>> - backport ethtool_ops
>> - backport mmd_eee_adv_to_ethtool_adv_t
>> - add patches/collateral-evolutions/network/82-ethernet/igb_ptp.patch
>> - add patches/collateral-evolutions/network/82-ethernet/igb_err_handler.patch
>>
>> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>> ---
[...]
>> +struct backport_ethtool_ops {
>> + int (*get_settings)(struct net_device *, struct ethtool_cmd *);
>> + int (*set_settings)(struct net_device *, struct ethtool_cmd *);
>> + void (*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *);
>> + int (*get_regs_len)(struct net_device *);
>> + void (*get_regs)(struct net_device *, struct ethtool_regs *, void *);
>> + void (*get_wol)(struct net_device *, struct ethtool_wolinfo *);
>> + int (*set_wol)(struct net_device *, struct ethtool_wolinfo *);
>> + u32 (*get_msglevel)(struct net_device *);
>> + void (*set_msglevel)(struct net_device *, u32);
>> + int (*nway_reset)(struct net_device *);
>> + u32 (*get_link)(struct net_device *);
>> + int (*get_eeprom_len)(struct net_device *);
>> + int (*get_eeprom)(struct net_device *,
>> + struct ethtool_eeprom *, u8 *);
>> + int (*set_eeprom)(struct net_device *,
>> + struct ethtool_eeprom *, u8 *);
>> + int (*get_coalesce)(struct net_device *, struct ethtool_coalesce *);
>> + int (*set_coalesce)(struct net_device *, struct ethtool_coalesce *);
>> + void (*get_ringparam)(struct net_device *,
>> + struct ethtool_ringparam *);
>> + int (*set_ringparam)(struct net_device *,
>> + struct ethtool_ringparam *);
>> + void (*get_pauseparam)(struct net_device *,
>> + struct ethtool_pauseparam*);
>> + int (*set_pauseparam)(struct net_device *,
>> + struct ethtool_pauseparam*);
>> + void (*self_test)(struct net_device *, struct ethtool_test *, u64 *);
>> + void (*get_strings)(struct net_device *, u32 stringset, u8 *);
>> + int (*set_phys_id)(struct net_device *, enum ethtool_phys_id_state);
>> + void (*get_ethtool_stats)(struct net_device *,
>> + struct ethtool_stats *, u64 *);
>> + int (*begin)(struct net_device *);
>> + void (*complete)(struct net_device *);
>> + u32 (*get_priv_flags)(struct net_device *);
>> + int (*set_priv_flags)(struct net_device *, u32);
>> + int (*get_sset_count)(struct net_device *, int);
>> + int (*get_rxnfc)(struct net_device *,
>> + struct ethtool_rxnfc *, u32 *rule_locs);
>> + int (*set_rxnfc)(struct net_device *, struct ethtool_rxnfc *);
>> + int (*flash_device)(struct net_device *, struct ethtool_flash *);
>> + int (*reset)(struct net_device *, u32 *);
>> + u32 (*get_rxfh_indir_size)(struct net_device *);
>> + int (*get_rxfh_indir)(struct net_device *, u32 *);
>> + int (*set_rxfh_indir)(struct net_device *, const u32 *);
>> + void (*get_channels)(struct net_device *, struct ethtool_channels *);
>> + int (*set_channels)(struct net_device *, struct ethtool_channels *);
>> + int (*get_dump_flag)(struct net_device *, struct ethtool_dump *);
>> + int (*get_dump_data)(struct net_device *,
>> + struct ethtool_dump *, void *);
>> + int (*set_dump)(struct net_device *, struct ethtool_dump *);
>> + int (*get_ts_info)(struct net_device *, struct ethtool_ts_info *);
>> + int (*get_module_info)(struct net_device *,
>> + struct ethtool_modinfo *);
>> + int (*get_module_eeprom)(struct net_device *,
>> + struct ethtool_eeprom *, u8 *);
>> + int (*get_eee)(struct net_device *, struct ethtool_eee *);
>> + int (*set_eee)(struct net_device *, struct ethtool_eee *);
>> +};
>> +#define ethtool_ops LINUX_BACKPORT(ethtool_ops)
>
> Backporting ethtool_ops does not work so easily. The network stack is in
> the old kernel which want to call some function with a pointer from this
> strcut, but it accesses this option not by its name but by the offset in
> this struct it was compiled with. If the kernel and backports are
> getting compiled with a different version of this struct then they could
> have a different opinion on where a certain function pointer is placed
> which will lead to the kernel calling the wrong function. You have to be
> careful when you want to change some structs in backports, this normally
> only works when all users of this strut are in backports.
I agree with you there, so what would be a better way to deal with
changes in struct ethtool_ops? We probably could ifdef out the code in
igb but I assume that would get very ugly rather quickly and would take a
lot of time to maintain and refresh patches against upstream. A solution
which keeps the changes to upstream minimal would be preferable if you
could think of any.
Stefan
next prev parent reply other threads:[~2013-12-13 8:01 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-12 9:27 [PATCH RFC 00/10] backports: add igb driver Stefan Assmann
2013-12-12 9:27 ` [PATCH RFC 01/10] backports: add igb ethernet network driver Stefan Assmann
2013-12-13 0:27 ` Hauke Mehrtens
2013-12-13 7:39 ` Stefan Assmann
2013-12-12 9:27 ` [PATCH RFC 02/10] backports: igb fixes for linux-3.9 Stefan Assmann
2013-12-12 9:27 ` [PATCH RFC 03/10] backports: igb fixes for linux-3.8 Stefan Assmann
2013-12-12 9:27 ` [PATCH RFC 04/10] backports: igb fixes for linux-3.7 Stefan Assmann
2013-12-12 9:27 ` [PATCH RFC 05/10] backports: igb fixes for linux-3.6 Stefan Assmann
2013-12-13 0:46 ` Hauke Mehrtens
2013-12-13 8:01 ` Stefan Assmann [this message]
2013-12-12 9:27 ` [PATCH RFC 06/10] backports: igb fixes for linux-3.5 Stefan Assmann
2013-12-12 9:27 ` [PATCH RFC 07/10] backports: igb fixes for linux-3.4 Stefan Assmann
2013-12-12 9:28 ` [PATCH RFC 08/10] backports: igb fixes for linux-3.3 Stefan Assmann
2013-12-12 9:28 ` [PATCH RFC 09/10] backports: igb fixes for linux-3.2 Stefan Assmann
2013-12-12 9:28 ` [PATCH RFC 10/10] backports: igb fixes for linux-3.1 Stefan Assmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52AABED2.7000506@kpanic.de \
--to=sassmann@kpanic.de \
--cc=backports@vger.kernel.org \
--cc=hauke@hauke-m.de \
--cc=mcgrof@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox