From owner-freebsd-current@freebsd.org Thu Oct 4 00:10:57 2018 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2F11310B1E44 for ; Thu, 4 Oct 2018 00:10:57 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from CAN01-TO1-obe.outbound.protection.outlook.com (mail-eopbgr670056.outbound.protection.outlook.com [40.107.67.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (Client CN "mail.protection.outlook.com", Issuer "GlobalSign Organization Validation CA - SHA256 - G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C2BFE76112; Thu, 4 Oct 2018 00:10:56 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from YTOPR0101MB1820.CANPRD01.PROD.OUTLOOK.COM (52.132.44.160) by YTOPR0101MB1369.CANPRD01.PROD.OUTLOOK.COM (52.132.45.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1185.25; Thu, 4 Oct 2018 00:10:55 +0000 Received: from YTOPR0101MB1820.CANPRD01.PROD.OUTLOOK.COM ([fe80::65af:417a:161f:f4eb]) by YTOPR0101MB1820.CANPRD01.PROD.OUTLOOK.COM ([fe80::65af:417a:161f:f4eb%3]) with mapi id 15.20.1207.021; Thu, 4 Oct 2018 00:10:55 +0000 From: Rick Macklem To: Brooks Davis CC: "freebsd-current@FreeBSD.org" , Josh Paetzel Subject: Re: which way to update export_args structure? Thread-Topic: which way to update export_args structure? Thread-Index: AQHUWq0WJEV5v0B9NEODb8Fv+fAB/qUNrJCAgACGJVY= Date: Thu, 4 Oct 2018 00:10:55 +0000 Message-ID: References: , <20181003155133.GA57729@spindle.one-eyed-alien.net> In-Reply-To: <20181003155133.GA57729@spindle.one-eyed-alien.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=rmacklem@uoguelph.ca; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; YTOPR0101MB1369; 6:cZoOUsYPeJV20TxxXZ7KvEyYoHZE5wHsRKoBpFHBo8NabSattJpTRgOF09c+3zcjgaUK6r7mhalOr+V2acLUvEGyNvafKO2kUYS4X5rPPqabwiXkQemQWgsK1rjD+G5VLApuwZVlGz0IPDzLby6P3/2EN4UgTrsCWmj/m01Kb0/R9gTqKbrJmHz5QIZwzQPVsoL3pCksg1TEr+Q1U2KZUFD4z0a/3Lb7j7Z3GO5tmKgckPYXnn2Iy28AEo0oJnRANtLy074S4yAVGjfvjDLkZ349+lJU3Vk1RCvXjMslbnqjtMasN9bypcQRyvMC5pwRT7dieNmlKjFvs37mKWDUru7LN6BHdsnhbKSEVJbu5NHUIOHnAqAsn1wh42q/A1weoD/DUE6EKHkeHrcWr5M2NeiYODjpMpfXX8cioKTYg2JGmrfw4HBc/mnO5O3hVwskiuBLXaDOZpZG5eUqrWl20Q==; 5:LRK0V/9xAP4hePTQUmoFAklscZhmVsN3WWutAzF6exvnlETDDN/HOPu1Ciea94AxF1797SEs2ax8ksJxJ900THBJKQrjLLpW1QfboWXfRs6zDnYgf1BNpkhM2sZO3ev5GRhOMwgtaHncRIgY2/cJ97vJ/av2FWxAQlUDQq/B9oY=; 7:c3fQbBf5c313rMUGEDz7TbfVnLpBIWK0/h8sO9XKupuU0VwJAycpLOAdhh66XLwcQ2BjusoNKMWihUTo2+y2cKBOSl27XEpySucRnQdWO7uTUO/dj7CzozHzAIigzyNIfZa71FfnKwObvpacGcCnTbzorvNk5JDlr3zfZb03LUsBhFiVxPXxmTrzNMHMW52Jq4lYXZj+3bD0slJdqbq1JlFuuAnaryhud/7UPm83NQKCU467+6P+ICQBOj3O4AP+ x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: a836e229-bbc9-4ff0-2de6-08d6298dda76 x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(2017052603328)(7153060)(7193020); SRVR:YTOPR0101MB1369; x-ms-traffictypediagnostic: YTOPR0101MB1369: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(192374486261705)(131327999870524); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040522)(2401047)(8121501046)(5005006)(10201501046)(3231355)(944501410)(52105095)(3002001)(93006095)(93001095)(149066)(150057)(6041310)(20161123558120)(20161123564045)(20161123562045)(201703131423095)(201702281529075)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123560045)(201708071742011)(7699051); SRVR:YTOPR0101MB1369; BCL:0; PCL:0; RULEID:; SRVR:YTOPR0101MB1369; x-forefront-prvs: 0815F8251E x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(136003)(376002)(396003)(346002)(39860400002)(199004)(189003)(186003)(105586002)(2900100001)(106356001)(76176011)(7696005)(15650500001)(446003)(5250100002)(476003)(99286004)(11346002)(316002)(97736004)(74316002)(33656002)(8936002)(5660300001)(478600001)(305945005)(14454004)(786003)(486006)(46003)(229853002)(81156014)(53936002)(256004)(8676002)(71190400001)(74482002)(71200400001)(14444005)(68736007)(55016002)(6506007)(102836004)(6246003)(81166006)(6916009)(9686003)(4326008)(6436002)(54906003)(86362001)(25786009)(2906002)(43043002); DIR:OUT; SFP:1101; SCL:1; SRVR:YTOPR0101MB1369; H:YTOPR0101MB1820.CANPRD01.PROD.OUTLOOK.COM; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: uoguelph.ca does not designate permitted sender hosts) x-microsoft-antispam-message-info: xh2DzyF6X3s6rbY35s8p8oVYoYAEJkvi8Se1NfvN6PoFcNvaoQsOAG+JxSpcDyj/3G0QIFlOyLt9zQ61uqA1YbBHwYRR+9Qt4KO2ggAerkyAJM0KP+qxoyyFYIm/vljjXRkfNlh1PZ1xXIbInjfpt6K8Ehsr4kPw12xJx9kPnxLHLXLT75Y8Ti8RpQctEnaj6pTqa+bAboMdYs5JuoWCOE275J1KtRVD4GnuKQZdcbZRxmuBDAxA7KLVWLzK7x8sWaZb8wbD2QL44IZNjLmcQImUEiWXo3aA65V/meQt+HKDUZMGdNr8pHBg7B5eVvZTs/ERGMCiVH/ASYgnkn1hREecoTnF13PK75I9uKXZbPc= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: uoguelph.ca X-MS-Exchange-CrossTenant-Network-Message-Id: a836e229-bbc9-4ff0-2de6-08d6298dda76 X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Oct 2018 00:10:55.0458 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: be62a12b-2cad-49a1-a5fa-85f4f3156a7d X-MS-Exchange-Transport-CrossTenantHeadersStamped: YTOPR0101MB1369 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Oct 2018 00:10:57 -0000 Brooks Davis wrote: >On Wed, Oct 03, 2018 at 12:40:27AM +0000, Rick Macklem wrote: >> Hi, >> >> I am working on updating "struct export_args" to fix/add a few things. >> One of these is that "ex_flags" is an int, but the flags are defined in = mount.h >> as MNT_xx bits that now exceed 32bits (mnt_flag is now uint64_t). >> For now, this doesn't break anything, since the flags used by ex_flags a= re >> all defined in the low order 32bits but...it seems like this should be a= ddressed >> by a new version of "struct export_args". >> >> I have two versions of the updated structure: >> A) >> struct export_args { >> uint64_t ex_flags; /* export related flags */ >> uid_t ex_root; /* mapping for root uid */ >> struct xucred ex_anon; /* mapping for anonymous user */ >> struct sockaddr *ex_addr; /* net address to which exported *= / >> u_char ex_addrlen; /* and the net address length */ >> struct sockaddr *ex_mask; /* mask of valid bits in saddr */ >> u_char ex_masklen; /* and the smask length */ >> char *ex_indexfile; /* index file for WebNFS URLs */ >> int ex_numsecflavors; /* security flavor count */ >> int ex_secflavors[MAXSECFLAVORS]; /* list of security flavors = */ >> int32_t ex_fsid; /* mnt_stat.f_fsid.val[0] if */ >> /* MNT_EXPORTFSID set in ex_flags6= 4 */ >> gid_t *ex_suppgroups; /* Supplemental groups if */ >> /* ex_anon.cr_ngroups > XU_NGROUPS= */ >> }; >> B) >> struct export_args { >> int ex_flags; /* export related flags */ >> uid_t ex_root; /* mapping for root uid */ >> struct xucred ex_anon; /* mapping for anonymous user */ >> struct sockaddr *ex_addr; /* net address to which exported *= / >> u_char ex_addrlen; /* and the net address length */ >> struct sockaddr *ex_mask; /* mask of valid bits in saddr */ >> u_char ex_masklen; /* and the smask length */ >> char *ex_indexfile; /* index file for WebNFS URLs */ >> int ex_numsecflavors; /* security flavor count */ >> int ex_secflavors[MAXSECFLAVORS]; /* list of security flavors = */ >> uint64_t ex_flagshighbits; /* High order bits of mnt_flag */ >> int32_t ex_fsid; /* mnt_stat.f_fsid.val[0] if */ >> /* MNT_EXPORTFSID set in ex_flags6= 4 */ >> gid_t *ex_suppgroups; /* Supplemental groups if */ >> /* ex_anon.cr_ngroups > XU_NGROUPS= */ >> }; >> >> A) does the obvious thing. Unfortunately, this changes the vfs KABI >> (specifically the function vfs_oexport_conv()) such that a file system >> module compiled with an unpatched mount.h could crash a patched system. >> As such, I think it couldn't be MFC'd and would be stuck in head/current >> until FreeBSD13 (or FreeBSD14 if 13 gets skipped over;-). >> >> B) doesn't change any fields, but adds a second ex_flagshighbits for the= high >> order bit. Since it only adds fields where none of those bits are used a= fter >> the exports are processed by vfs_export() and, as such, will not break >> the VFS KABI, since vfs_domount_update() differentiates which version >> of export_args is being used. >> As such, I believe this version can be MFC'd. However, it does seem conf= using >> to have the two ex_flags fields for the low and high 32bits. > >I see you've found a way to do compatibility for a new ABI. If you >wanted to avoid changing the struct size, there is 3 bytes of usable >padding after each ex_addrlen and ex_masklen. Actually, you want the size to change, since that is how the code detects a different version of the struct. (Take a look around line# 1037 of vfs_mo= unt.c). The additions are a lot more than 6bytes. The reason I was a little hesitan= t to change ex_flags to 64bits is that it makes the compatibility code a little = messier, but it isn't that bad. The tricky one is vfs_oexport_conv(), because it doesn't know the size of t= he struct being passed in via a pointer. My current solution is to have this f= unction remain in place for old file system binaries only and add a new function wi= th a different name (and takes a struct length argument as well as the pointer= ) for the new code. This function is only used by three file systems to handl= e the old pre-nmount(2) syscall. >One general question: why does export_args still exist as an interface >between userspace and the kernel? It's passed via nmount so it seems >like the individual entries should be elements in the vector instead. >This would be much friendlier if one wanted to do 32-bit compat support >for mountd. Not sure what you are thinking of here. Right now "struct export_args" is t= he data for a mount option called "export". vfs_getopt() returns the length along w= ith the structure data and that length can be used to differentiate versions of= the structure. (Already done once by dfr@ and this would be a second revision.) If you are thinking that each field should be a separate option, I suppose = that could be done? Josh Paetzel has volunteered to update mountd.c, so he might have some comments w.r.t. how easy it would be to make all of the structure fields separate options? (I don't think the kernel changes would be that hard. Just a bunch of vfs_g= etopt() calls for the new option names.) rick