On Mon, 25 Aug 2025 at 21:58, Mike Pattrick <[email protected]> wrote: >> @@ -1145,53 +1141,82 @@ netdev_bsd_get_features(const struct netdev *netdev, >> error = af_inet_ioctl(SIOCGIFMEDIA, &ifmr); >> if (error) { >> VLOG_DBG_RL(&rl, "%s: ioctl(SIOCGIFMEDIA) failed: %s", >> - netdev_get_name(netdev), ovs_strerror(error)); >> + netdev_get_name(&netdev->up), ovs_strerror(error)); >> goto cleanup; >> } >> >> /* Current settings. */ >> - *current = netdev_bsd_parse_media(ifmr.ifm_active); >> - if (!*current) { >> - *current = NETDEV_F_OTHER; >> + netdev->current = netdev_bsd_parse_media(ifmr.ifm_active); >> + if (!netdev->current) { >> + netdev->current = NETDEV_F_OTHER; >> } >> >> /* Advertised features. */ >> - *advertised = netdev_bsd_parse_media(ifmr.ifm_current); >> + netdev->advertised = netdev_bsd_parse_media(ifmr.ifm_current); >> >> /* Supported features. */ >> - *supported = 0; >> + netdev->supported = 0; >> for (i = 0; i < ifmr.ifm_count; i++) { >> - *supported |= netdev_bsd_parse_media(ifmr.ifm_ulist[i]); >> + netdev->supported |= netdev_bsd_parse_media(ifmr.ifm_ulist[i]); >> } >> >> - /* Peer advertisements. */ >> - *peer = 0; /* XXX */ >> - >> error = 0; >> + >> cleanup: >> free(media_list); >> + netdev->get_features_error = error; > > > Why not just return this value?
I copied this logic from the linux netdev. I am not clear about the original intent. At the moment, it makes the get_features_error and other (current, advertised etc..) fields all populated by this same helper in a unified way (retrieved values are all stored in the netdev specific object). I don't mind changing this here if there is strong opinion against. -- David Marchand _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
