[vbox-dev] Fixes for API changes in kernel 4.20
Aleksey Ilyushin
aleksey.ilyushin at oracle.com
Wed Nov 21 07:23:19 GMT 2018
Hello Larry,
Thanks a lot for your patch. I do have an issue with it though. I think ‘get_link_ksettings’ callback is supposed to return the link settings in the new format, so ‘vboxNetAdpEthGetSettings’ should allocate the old structure, fill it out, then call a function that does the opposite of what ‘convert_link_ksettings_to_legacy_settings’ does, thus filling the new structure provided in the parameters, then free the old structure. Currently the patch does not modify the output structure at all. It looks like you had ’setter’ in mind when implementing this. If you agree I’ll go forward and modify the patch, then apply it.
Best regards,
Aleksey
> On 21 Nov 2018, at 04:24, Larry Finger <Larry.Finger at lwfinger.net> wrote:
>
> On 11/20/18 6:37 AM, Michael Thayer wrote:
>> 08.11.18 21:27, Larry Finger wrote:
>>> Hi,
>>>
>>> There are a number of changes in kernel 4.20 that affect building the VB
>>> modules. These are as follows:
>>>
>>> 1. In struct ethtool_ops, the get_settings member is renamed
>>> get_link_ksettings, and the callback method is changed. To satisfy the
>>> new method, routine convert_link_ksettings_to_legacy_settings(), which
>>> is not exported by the kernel, had to be duplicated.
>>>
>>> 2. Routine ktime_get_real_ts() must be replaced by ktime_get_real_ts64()
>>> and there are other places where 64-bit names have to be used.
>>>
>>> 3. In the drm routines, info->flags no longer accepts
>>> FBINFO_CAN_FORCE_OUTPUT. According to the kernel commit message, this
>>> flag was unused and it can be deleted without any harmful effects.
>>>
>>> The patch file for these changes is attached.
>> Hello Larry,
>> Still waiting for input from the "owner" of the VBoxNetAdp code, but
>> does this slightly adjusted version (patch against trunk) look alright
>> to you?
>> Index: include/iprt/time.h
>> ===================================================================
>> --- include/iprt/time.h (revision 126767)
>> +++ include/iprt/time.h (working copy)
>> @@ -428,6 +428,14 @@
>> {
>> return RTTimeSpecAddNano(RTTimeSpecSetSeconds(pTime,
>> pTimespec->tv_sec), pTimespec->tv_nsec);
>> }
>> +
>> +
>> +# ifdef _LINUX_TIME64_H
>> +DECLINLINE(PRTTIMESPEC) RTTimeSpecSetTimespec64(PRTTIMESPEC pTime,
>> const struct timespec64 *pTimeval)
>> +{
>> + return RTTimeSpecAddNano(RTTimeSpecSetSeconds(pTime,
>> pTimeval->tv_sec), pTimeval->tv_nsec);
>> +}
>> +# endif
>> #endif /* various ways of detecting struct timespec */
>> Index: src/VBox/Runtime/r0drv/linux/time-r0drv-linux.c
>> ===================================================================
>> --- src/VBox/Runtime/r0drv/linux/time-r0drv-linux.c (revision 126767)
>> +++ src/VBox/Runtime/r0drv/linux/time-r0drv-linux.c (working copy)
>> @@ -171,11 +171,20 @@
>> {
>> IPRT_LINUX_SAVE_EFL_AC();
>> #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 16)
>> +/* On Linux 4.20, time.h includes time64.h and we have to use 64-bit
>> times. */
>> +# ifdef _LINUX_TIME64_H
>> + struct timespec64 Ts;
>> + ktime_get_real_ts64(&Ts);
>> +# else
>> struct timespec Ts;
>> ktime_get_real_ts(&Ts);
>> +# endif
>> IPRT_LINUX_RESTORE_EFL_AC();
>> +# ifdef _LINUX_TIME64_H
>> + return RTTimeSpecSetTimespec64(pTime, &Ts);
>> +#else
>> return RTTimeSpecSetTimespec(pTime, &Ts);
>> -
>> +#endif
>> #else /* < 2.6.16 */
>> struct timeval Tv;
>> do_gettimeofday(&Tv);
>> Index: src/VBox/Additions/linux/drm/vbox_fb.c
>> ===================================================================
>> --- src/VBox/Additions/linux/drm/vbox_fb.c (revision 126767)
>> +++ src/VBox/Additions/linux/drm/vbox_fb.c (working copy)
>> @@ -325,8 +325,7 @@
>> * The last flag forces a mode set on VT switches even if the kernel
>> * does not think it is needed.
>> */
>> - info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT |
>> - FBINFO_MISC_ALWAYS_SETPAR;
>> + info->flags = FBINFO_DEFAULT | FBINFO_MISC_ALWAYS_SETPAR;
>> info->fbops = &vboxfb_ops;
>> /*
>> Index: src/VBox/HostDrivers/VBoxNetAdp/linux/VBoxNetAdp-linux.c
>> ===================================================================
>> --- src/VBox/HostDrivers/VBoxNetAdp/linux/VBoxNetAdp-linux.c (revision
>> 126767)
>> +++ src/VBox/HostDrivers/VBoxNetAdp/linux/VBoxNetAdp-linux.c (working copy)
>> @@ -84,9 +84,12 @@
>> #endif /* LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 36) */
>> static void vboxNetAdpEthGetDrvinfo(struct net_device *dev, struct
>> ethtool_drvinfo *info);
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 20, 0)
>> +static int vboxNetAdpEthGetSettings(struct net_device *pNetDev, struct
>> ethtool_link_ksettings *link_ksettings);
>> +#else
>> static int vboxNetAdpEthGetSettings(struct net_device *dev, struct
>> ethtool_cmd *cmd);
>> +#endif
>> -
>> /*********************************************************************************************************************************
>> * Global Variables
>> *
>> *********************************************************************************************************************************/
>> @@ -133,7 +136,11 @@
>> # endif
>> {
>> .get_drvinfo = vboxNetAdpEthGetDrvinfo,
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 20, 0)
>> + .get_link_ksettings = vboxNetAdpEthGetSettings,
>> +#else
>> .get_settings = vboxNetAdpEthGetSettings,
>> +#endif
>> .get_link = ethtool_op_get_link,
>> };
>> @@ -204,10 +211,64 @@
>> "N/A");
>> }
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 20, 0)
>> +static bool
>> +convert_link_ksettings_to_legacy_settings(
>> + struct ethtool_cmd *legacy_settings,
>> + const struct ethtool_link_ksettings *link_ksettings)
>> +{
>> + bool retval = true;
>> + memset(legacy_settings, 0, sizeof(*legacy_settings));
>> + /* this also clears the deprecated fields in legacy structure:
>> + * __u8 transceiver;
>> + * __u32 maxtxpkt;
>> + * __u32 maxrxpkt;
>> + */
>> +
>> + retval &= ethtool_convert_link_mode_to_legacy_u32(
>> + &legacy_settings->supported,
>> + link_ksettings->link_modes.supported);
>> + retval &= ethtool_convert_link_mode_to_legacy_u32(
>> + &legacy_settings->advertising,
>> + link_ksettings->link_modes.advertising);
>> + retval &= ethtool_convert_link_mode_to_legacy_u32(
>> + &legacy_settings->lp_advertising,
>> + link_ksettings->link_modes.lp_advertising);
>> + ethtool_cmd_speed_set(legacy_settings, link_ksettings->base.speed);
>> + legacy_settings->duplex
>> + = link_ksettings->base.duplex;
>> + legacy_settings->port
>> + = link_ksettings->base.port;
>> + legacy_settings->phy_address
>> + = link_ksettings->base.phy_address;
>> + legacy_settings->autoneg
>> + = link_ksettings->base.autoneg;
>> + legacy_settings->mdio_support
>> + = link_ksettings->base.mdio_support;
>> + legacy_settings->eth_tp_mdix
>> + = link_ksettings->base.eth_tp_mdix;
>> + legacy_settings->eth_tp_mdix_ctrl
>> + = link_ksettings->base.eth_tp_mdix_ctrl;
>> + legacy_settings->transceiver
>> + = link_ksettings->base.transceiver;
>> + return retval;
>> +}
>> +#endif
>> +
>> /* ethtool_ops::get_settings */
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 20, 0)
>> +static int vboxNetAdpEthGetSettings(struct net_device *pNetDev, struct
>> ethtool_link_ksettings *link_ksettings)
>> +#else
>> static int vboxNetAdpEthGetSettings(struct net_device *pNetDev, struct
>> ethtool_cmd *cmd)
>> +#endif
>> {
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 20, 0)
>> + struct ethtool_cmd *cmd = kzalloc(sizeof(struct ethtool_cmd),
>> GFP_KERNEL);
>> + if (!cmd)
>> + return 1;
>> + convert_link_ksettings_to_legacy_settings(cmd, link_ksettings);
>> +#endif
>> cmd->supported = 0;
>> cmd->advertising = 0;
>> #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 27)
>> @@ -222,6 +283,9 @@
>> cmd->autoneg = AUTONEG_DISABLE;
>> cmd->maxtxpkt = 0;
>> cmd->maxrxpkt = 0;
>> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 20, 0)
>> + kfree(cmd);
>> +#endif
>> return 0;
>> }
>> --
>>>
>>> Larry--
>
> Michael,
>
> I really like your approach to testing for features in a manner that will be less affected by distros backporting API changes. Unfortunately, code patched using your version fails to build under kernel 4.20.0-rcX. As I noted, the get_settings member of struct ethtool_ops, is renamed get_link_ksettings, and the callback method is changed. Your patch does not handle this change. This results in the error
>
> /usr/src/kernel-modules/virtualbox/src/vboxnetadp/linux/VBoxNetAdp-linux.c:135:6: error: ‘const struct ethtool_ops’ has no member named ‘get_settings’; did you mean ‘get_strings’?
> .get_settings = vboxNetAdpEthGetSettings,
> ^~~~~~~~~~~~
> get_strings
>
> Of course, others follow.
>
> Larry
>
>
>
> _______________________________________________
> vbox-dev mailing list
> vbox-dev at virtualbox.org
> https://www.virtualbox.org/mailman/listinfo/vbox-dev
More information about the vbox-dev
mailing list