Re: change ifmedia word from int to uint64_t

[Jason Thorpe <thorpej%me.com@localhost>]

> On Apr 18, 2019, at 6:52 AM, Jason Thorpe <thorpej%me.com@localhost> wrote:
> 
> 
> 
>> On Apr 18, 2019, at 2:32 AM, Masanobu SAITOH <msaitoh%execsw.org@localhost> wrote:
>> 
>> Hi.
>> 
>> I'm now working to extend ifmedia word to be able to support
>> more than 10Gbps media.
>> 
>> Patch:
>> 
>> 	http://www.netbsd.org/~msaitoh/ifmedia64-20190418-0.dif
> 
> It might be nice to use __BIT(), __SHIFT*() etc. macros (if they can do ULL constants)... I think it would make all of the constants a lot easier to read.
> 
> I suppose we could add an ifmword_t ... but if we have to enlarge it again, it's going to become a struct (or something else entirely), right?  I guess it makes the "grep" phase of the next overhaul a little easier :-)

Also:

Index: sys/sys/sockio.h
===================================================================
RCS file: /cvsroot/src/sys/sys/sockio.h,v
retrieving revision 1.36
diff -u -p -r1.36 sockio.h
--- sys/sys/sockio.h	21 Dec 2018 08:58:08 -0000	1.36
+++ sys/sys/sockio.h	18 Apr 2019 09:12:46 -0000
@@ -90,8 +90,9 @@
 #define	SIOCGETVIFCNT	_IOWR('u', 51, struct sioc_vif_req)/* vif pkt cnt */
 #define	SIOCGETSGCNT	_IOWR('u', 52, struct sioc_sg_req) /* sg pkt cnt */
 
-#define	SIOCSIFMEDIA	_IOWR('i', 53, struct ifreq)	/* set net media */
-#define	SIOCGIFMEDIA	_IOWR('i', 54, struct ifmediareq) /* get net media */
+/* 53 and 54 used to be SIOC[SG]IFMEDIA with a 32 bit media word */
+#define	SIOCSIFMEDIA	_IOWR('i', 55, struct ifreq)	/* set net media */
+#define	SIOCGIFMEDIA	_IOWR('i', 56, struct ifmediareq) /* get net media */
 
 #define	SIOCSIFGENERIC	 _IOW('i', 57, struct ifreq)	/* generic IF set op */
 #define	SIOCGIFGENERIC	_IOWR('i', 58, struct ifreq)	/* generic IF get op */

You don't have to allocate new numbers; the size of the argument structure changes, and therefore the value of SIOCSIFMEDIA / SIOCGIFMEDIA changes.


> 
>> 
>> I changed ifmedia's word from int to uint64_t. It's almost the
>> same as OpenBSD who did 5 years ago.
>> 
>> What do you think about the above diff?
>> IMHO, it would be better to change ifmword_t(with uint64_t)
>> than uint64_t. For example
>> 
>> (from ifconfig/media.c's diff)
>>> @@ -374,7 +374,8 @@ media_status(prop_dictionary_t env, prop
>>> 	/* Interface link status is queried through SIOCGIFMEDIA.
>>> 	 * Not all interfaces have actual media. */
>>> 	if (ifmr.ifm_count != 0) {
>>> -		media_list = (int *)malloc(ifmr.ifm_count * sizeof(int));
>>> +		media_list = (uint64_t *)malloc(ifmr.ifm_count
>>> +		    * sizeof(*media_list));
>>> 		if (media_list == NULL)
>>> 			err(EXIT_FAILURE, "malloc");
>>> 		ifmr.ifm_ulist = media_list;
>> 
>> If we use ifmword_t, it makes easy to change the size in future.
>> 
>> 
>> This patch has some TODOs:
>> 
>> - COMPAT_NETBSD32? (currently i386 old binary doesn't work on
>>  amd64)
>> - I would change the hook point. Current location is not good
>>  because we have to add "case: OSIOC[GS]IFMEDIA" to all driver
>>  which calls ifmedia_ioctl(). (see if_wm.c's patch)
>> 
>> 
>> Thanks.
>> 
>> -- 
>> -----------------------------------------------
>>               SAITOH Masanobu (msaitoh%execsw.org@localhost
>>                                msaitoh%netbsd.org@localhost)
> 
> -- thorpej

-- thorpej