All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
@ 2017-01-12 11:56 David Howells
  2017-01-12 11:56 ` [PATCH 2/2] afs: Use core kernel UUID generation David Howells
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: David Howells @ 2017-01-12 11:56 UTC (permalink / raw)
  To: arnd; +Cc: dhowells, linux-kernel, ruchandani.tina, linux-afs

Move the afs_uuid struct to linux/uuid.h and rename it to uuid_v1.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/cmservice.c   |   22 +++++++++++-----------
 fs/afs/internal.h    |   26 +-------------------------
 fs/afs/main.c        |   25 +++++++++++++------------
 include/linux/uuid.h |   24 ++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 48 deletions(-)

diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index e349a3316303..ee55172f4863 100644
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -351,7 +351,7 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
 {
 	struct sockaddr_rxrpc srx;
 	struct afs_server *server;
-	struct afs_uuid *r;
+	struct uuid_v1 *r;
 	unsigned loop;
 	__be32 *b;
 	int ret;
@@ -381,15 +381,15 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
 		}
 
 		_debug("unmarshall UUID");
-		call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
+		call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
 		if (!call->request)
 			return -ENOMEM;
 
 		b = call->buffer;
 		r = call->request;
-		r->time_low			= ntohl(b[0]);
-		r->time_mid			= ntohl(b[1]);
-		r->time_hi_and_version		= ntohl(b[2]);
+		r->time_low			= b[0];
+		r->time_mid			= htons(ntohl(b[1]));
+		r->time_hi_and_version		= htons(ntohl(b[2]));
 		r->clock_seq_hi_and_reserved 	= ntohl(b[3]);
 		r->clock_seq_low		= ntohl(b[4]);
 
@@ -454,7 +454,7 @@ static int afs_deliver_cb_probe(struct afs_call *call)
 static void SRXAFSCB_ProbeUuid(struct work_struct *work)
 {
 	struct afs_call *call = container_of(work, struct afs_call, work);
-	struct afs_uuid *r = call->request;
+	struct uuid_v1 *r = call->request;
 
 	struct {
 		__be32	match;
@@ -477,7 +477,7 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
  */
 static int afs_deliver_cb_probe_uuid(struct afs_call *call)
 {
-	struct afs_uuid *r;
+	struct uuid_v1 *r;
 	unsigned loop;
 	__be32 *b;
 	int ret;
@@ -503,7 +503,7 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call)
 		}
 
 		_debug("unmarshall UUID");
-		call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
+		call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
 		if (!call->request)
 			return -ENOMEM;
 
@@ -569,9 +569,9 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct *work)
 	memset(&reply, 0, sizeof(reply));
 	reply.ia.nifs = htonl(nifs);
 
-	reply.ia.uuid[0] = htonl(afs_uuid.time_low);
-	reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
-	reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
+	reply.ia.uuid[0] = afs_uuid.time_low;
+	reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
+	reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
 	reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
 	reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
 	for (loop = 0; loop < 6; loop++)
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 65504e218d35..bb72dc6f7541 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -407,30 +407,6 @@ struct afs_interface {
 	unsigned	mtu;		/* MTU of interface */
 };
 
-/*
- * UUID definition [internet draft]
- * - the timestamp is a 60-bit value, split 32/16/12, and goes in 100ns
- *   increments since midnight 15th October 1582
- *   - add AFS_UUID_TO_UNIX_TIME to convert unix time in 100ns units to UUID
- *     time
- * - the clock sequence is a 14-bit counter to avoid duplicate times
- */
-struct afs_uuid {
-	u32		time_low;			/* low part of timestamp */
-	u16		time_mid;			/* mid part of timestamp */
-	u16		time_hi_and_version;		/* high part of timestamp and version  */
-#define AFS_UUID_TO_UNIX_TIME	0x01b21dd213814000ULL
-#define AFS_UUID_TIMEHI_MASK	0x0fff
-#define AFS_UUID_VERSION_TIME	0x1000	/* time-based UUID */
-#define AFS_UUID_VERSION_NAME	0x3000	/* name-based UUID */
-#define AFS_UUID_VERSION_RANDOM	0x4000	/* (pseudo-)random generated UUID */
-	u8		clock_seq_hi_and_reserved;	/* clock seq hi and variant */
-#define AFS_UUID_CLOCKHI_MASK	0x3f
-#define AFS_UUID_VARIANT_STD	0x80
-	u8		clock_seq_low;			/* clock seq low */
-	u8		node[6];			/* spatially unique node ID (MAC addr) */
-};
-
 /*****************************************************************************/
 /*
  * cache.c
@@ -565,7 +541,7 @@ extern int afs_drop_inode(struct inode *);
  * main.c
  */
 extern struct workqueue_struct *afs_wq;
-extern struct afs_uuid afs_uuid;
+extern struct uuid_v1 afs_uuid;
 
 /*
  * misc.c
diff --git a/fs/afs/main.c b/fs/afs/main.c
index f8188feb03ad..a07c14df3fd1 100644
--- a/fs/afs/main.c
+++ b/fs/afs/main.c
@@ -31,7 +31,7 @@ static char *rootcell;
 module_param(rootcell, charp, 0);
 MODULE_PARM_DESC(rootcell, "root AFS cell name and VL server IP addr list");
 
-struct afs_uuid afs_uuid;
+struct uuid_v1 afs_uuid;
 struct workqueue_struct *afs_wq;
 
 /*
@@ -41,7 +41,7 @@ static int __init afs_get_client_UUID(void)
 {
 	struct timespec ts;
 	u64 uuidtime;
-	u16 clockseq;
+	u16 clockseq, hi_v;
 	int ret;
 
 	/* read the MAC address of one of the external interfaces and construct
@@ -53,22 +53,23 @@ static int __init afs_get_client_UUID(void)
 	getnstimeofday(&ts);
 	uuidtime = (u64) ts.tv_sec * 1000 * 1000 * 10;
 	uuidtime += ts.tv_nsec / 100;
-	uuidtime += AFS_UUID_TO_UNIX_TIME;
-	afs_uuid.time_low = uuidtime;
-	afs_uuid.time_mid = uuidtime >> 32;
-	afs_uuid.time_hi_and_version = (uuidtime >> 48) & AFS_UUID_TIMEHI_MASK;
-	afs_uuid.time_hi_and_version |= AFS_UUID_VERSION_TIME;
+	uuidtime += UUID_TO_UNIX_TIME;
+	afs_uuid.time_low = htonl(uuidtime);
+	afs_uuid.time_mid = htons(uuidtime >> 32);
+	hi_v = (uuidtime >> 48) & UUID_TIMEHI_MASK;
+	hi_v |= UUID_VERSION_TIME;
+	afs_uuid.time_hi_and_version = htons(hi_v);
 
 	get_random_bytes(&clockseq, 2);
 	afs_uuid.clock_seq_low = clockseq;
 	afs_uuid.clock_seq_hi_and_reserved =
-		(clockseq >> 8) & AFS_UUID_CLOCKHI_MASK;
-	afs_uuid.clock_seq_hi_and_reserved |= AFS_UUID_VARIANT_STD;
+		(clockseq >> 8) & UUID_CLOCKHI_MASK;
+	afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
 
 	_debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
-	       afs_uuid.time_low,
-	       afs_uuid.time_mid,
-	       afs_uuid.time_hi_and_version,
+	       ntohl(afs_uuid.time_low),
+	       ntohs(afs_uuid.time_mid),
+	       ntohs(afs_uuid.time_hi_and_version),
 	       afs_uuid.clock_seq_hi_and_reserved,
 	       afs_uuid.clock_seq_low,
 	       afs_uuid.node[0], afs_uuid.node[1], afs_uuid.node[2],
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 2d095fc60204..4dff73a89758 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -19,6 +19,30 @@
 #include <uapi/linux/uuid.h>
 
 /*
+ * V1 (time-based) UUID definition [RFC 4122].
+ * - the timestamp is a 60-bit value, split 32/16/12, and goes in 100ns
+ *   increments since midnight 15th October 1582
+ *   - add AFS_UUID_TO_UNIX_TIME to convert unix time in 100ns units to UUID
+ *     time
+ * - the clock sequence is a 14-bit counter to avoid duplicate times
+ */
+struct uuid_v1 {
+	__be32		time_low;			/* low part of timestamp */
+	__be16		time_mid;			/* mid part of timestamp */
+	__be16		time_hi_and_version;		/* high part of timestamp and version  */
+#define UUID_TO_UNIX_TIME	0x01b21dd213814000ULL
+#define UUID_TIMEHI_MASK	0x0fff
+#define UUID_VERSION_TIME	0x1000	/* time-based UUID */
+#define UUID_VERSION_NAME	0x3000	/* name-based UUID */
+#define UUID_VERSION_RANDOM	0x4000	/* (pseudo-)random generated UUID */
+	u8		clock_seq_hi_and_reserved;	/* clock seq hi and variant */
+#define UUID_CLOCKHI_MASK	0x3f
+#define UUID_VARIANT_STD	0x80
+	u8		clock_seq_low;			/* clock seq low */
+	u8		node[6];			/* spatially unique node ID (MAC addr) */
+};
+
+/*
  * The length of a UUID string ("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
  * not including trailing NUL.
  */

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] afs: Use core kernel UUID generation
  2017-01-12 11:56 [PATCH 1/2] afs: Move UUID struct to linux/uuid.h David Howells
@ 2017-01-12 11:56 ` David Howells
  2017-01-12 13:14 ` [PATCH 1/2] afs: Move UUID struct to linux/uuid.h Arnd Bergmann
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: David Howells @ 2017-01-12 11:56 UTC (permalink / raw)
  To: arnd; +Cc: dhowells, linux-kernel, ruchandani.tina, linux-afs

From: Arnd Bergmann <arnd@arndb.de>

AFS uses a time based UUID to identify the host itself.  This requires
getting a timestamp which is currently done through the getnstimeofday()
interface that we want to eventually get rid of.

Instead of replacing it with a ktime-based interface, simply remove the
entire function and use generate_random_uuid() instead, which has a v4
("completely random") UUID instead of the time-based one.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/internal.h   |   11 +++++------
 fs/afs/main.c       |   48 +-----------------------------------------------
 fs/afs/netdevices.c |   21 ---------------------
 3 files changed, 6 insertions(+), 74 deletions(-)

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index bb72dc6f7541..171e1f56c5bf 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -560,6 +560,11 @@ extern int afs_mntpt_check_symlink(struct afs_vnode *, struct key *);
 extern void afs_mntpt_kill_timer(void);
 
 /*
+ * netdevices.c
+ */
+extern int afs_get_ipv4_interfaces(struct afs_interface *, size_t, bool);
+
+/*
  * proc.c
  */
 extern int afs_proc_init(void);
@@ -623,12 +628,6 @@ extern int afs_fs_init(void);
 extern void afs_fs_exit(void);
 
 /*
- * use-rtnetlink.c
- */
-extern int afs_get_ipv4_interfaces(struct afs_interface *, size_t, bool);
-extern int afs_get_MAC_address(u8 *, size_t);
-
-/*
  * vlclient.c
  */
 extern int afs_vl_get_entry_by_name(struct in_addr *, struct key *,
diff --git a/fs/afs/main.c b/fs/afs/main.c
index a07c14df3fd1..51d7d17bca57 100644
--- a/fs/afs/main.c
+++ b/fs/afs/main.c
@@ -35,50 +35,6 @@ struct uuid_v1 afs_uuid;
 struct workqueue_struct *afs_wq;
 
 /*
- * get a client UUID
- */
-static int __init afs_get_client_UUID(void)
-{
-	struct timespec ts;
-	u64 uuidtime;
-	u16 clockseq, hi_v;
-	int ret;
-
-	/* read the MAC address of one of the external interfaces and construct
-	 * a UUID from it */
-	ret = afs_get_MAC_address(afs_uuid.node, sizeof(afs_uuid.node));
-	if (ret < 0)
-		return ret;
-
-	getnstimeofday(&ts);
-	uuidtime = (u64) ts.tv_sec * 1000 * 1000 * 10;
-	uuidtime += ts.tv_nsec / 100;
-	uuidtime += UUID_TO_UNIX_TIME;
-	afs_uuid.time_low = htonl(uuidtime);
-	afs_uuid.time_mid = htons(uuidtime >> 32);
-	hi_v = (uuidtime >> 48) & UUID_TIMEHI_MASK;
-	hi_v |= UUID_VERSION_TIME;
-	afs_uuid.time_hi_and_version = htons(hi_v);
-
-	get_random_bytes(&clockseq, 2);
-	afs_uuid.clock_seq_low = clockseq;
-	afs_uuid.clock_seq_hi_and_reserved =
-		(clockseq >> 8) & UUID_CLOCKHI_MASK;
-	afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
-
-	_debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
-	       ntohl(afs_uuid.time_low),
-	       ntohs(afs_uuid.time_mid),
-	       ntohs(afs_uuid.time_hi_and_version),
-	       afs_uuid.clock_seq_hi_and_reserved,
-	       afs_uuid.clock_seq_low,
-	       afs_uuid.node[0], afs_uuid.node[1], afs_uuid.node[2],
-	       afs_uuid.node[3], afs_uuid.node[4], afs_uuid.node[5]);
-
-	return 0;
-}
-
-/*
  * initialise the AFS client FS module
  */
 static int __init afs_init(void)
@@ -87,9 +43,7 @@ static int __init afs_init(void)
 
 	printk(KERN_INFO "kAFS: Red Hat AFS client v0.1 registering.\n");
 
-	ret = afs_get_client_UUID();
-	if (ret < 0)
-		return ret;
+	generate_random_uuid((unsigned char *)&afs_uuid);
 
 	/* create workqueue */
 	ret = -ENOMEM;
diff --git a/fs/afs/netdevices.c b/fs/afs/netdevices.c
index 7ad36506c256..40b2bab3e401 100644
--- a/fs/afs/netdevices.c
+++ b/fs/afs/netdevices.c
@@ -12,27 +12,6 @@
 #include "internal.h"
 
 /*
- * get a MAC address from a random ethernet interface that has a real one
- * - the buffer will normally be 6 bytes in size
- */
-int afs_get_MAC_address(u8 *mac, size_t maclen)
-{
-	struct net_device *dev;
-	int ret = -ENODEV;
-
-	BUG_ON(maclen != ETH_ALEN);
-
-	rtnl_lock();
-	dev = __dev_getfirstbyhwtype(&init_net, ARPHRD_ETHER);
-	if (dev) {
-		memcpy(mac, dev->dev_addr, maclen);
-		ret = 0;
-	}
-	rtnl_unlock();
-	return ret;
-}
-
-/*
  * get a list of this system's interface IPv4 addresses, netmasks and MTUs
  * - maxbufs must be at least 1
  * - returns the number of interface records in the buffer

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
  2017-01-12 11:56 [PATCH 1/2] afs: Move UUID struct to linux/uuid.h David Howells
  2017-01-12 11:56 ` [PATCH 2/2] afs: Use core kernel UUID generation David Howells
@ 2017-01-12 13:14 ` Arnd Bergmann
  2017-01-12 14:12 ` David Howells
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2017-01-12 13:14 UTC (permalink / raw)
  To: David Howells; +Cc: linux-kernel, ruchandani.tina, linux-afs

On Thursday, January 12, 2017 11:56:34 AM CET David Howells wrote:
> Move the afs_uuid struct to linux/uuid.h and rename it to uuid_v1.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> 

Looks good to me, but I wonder if this part:

                r = call->request;
-               r->time_low                     = ntohl(b[0]);
-               r->time_mid                     = ntohl(b[1]);
-               r->time_hi_and_version          = ntohl(b[2]);
+               r->time_low                     = b[0];
+               r->time_mid                     = htons(ntohl(b[1]));
+               r->time_hi_and_version          = htons(ntohl(b[2]));
                r->clock_seq_hi_and_reserved    = ntohl(b[3]);
                r->clock_seq_low                = ntohl(b[4]);
 
should be considered a bugfix and split out into a
separate patch. From what I understand about the mess in UUID
formats, the time fields can either be big-endian (as defined)
or little-endian (for all things Microsoft), and you are changing
the representation from CPU-specific to big-endian, which makes
it different for x86 and most ARM at least.

	Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
  2017-01-12 11:56 [PATCH 1/2] afs: Move UUID struct to linux/uuid.h David Howells
  2017-01-12 11:56 ` [PATCH 2/2] afs: Use core kernel UUID generation David Howells
  2017-01-12 13:14 ` [PATCH 1/2] afs: Move UUID struct to linux/uuid.h Arnd Bergmann
@ 2017-01-12 14:12 ` David Howells
  2017-01-12 15:40   ` Arnd Bergmann
  2017-01-12 16:14   ` David Howells
  2017-01-13  9:22 ` kbuild test robot
  2017-01-13 10:48 ` kbuild test robot
  4 siblings, 2 replies; 11+ messages in thread
From: David Howells @ 2017-01-12 14:12 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: dhowells, linux-kernel, ruchandani.tina, linux-afs

Arnd Bergmann <arnd@arndb.de> wrote:

> Looks good to me, but I wonder if this part:
> 
>                 r = call->request;
> -               r->time_low                     = ntohl(b[0]);
> -               r->time_mid                     = ntohl(b[1]);
> -               r->time_hi_and_version          = ntohl(b[2]);
> +               r->time_low                     = b[0];
> +               r->time_mid                     = htons(ntohl(b[1]));
> +               r->time_hi_and_version          = htons(ntohl(b[2]));
>                 r->clock_seq_hi_and_reserved    = ntohl(b[3]);
>                 r->clock_seq_low                = ntohl(b[4]);
>  
> should be considered a bugfix and split out into a
> separate patch.

I changed the definitions in the struct from u16/u32 to __be16/__be32 so it's
not a bugfix.

For some reason, rather than specifying UUIDs as just a 16-octet field, the
AFS protocol breaks the UUID down into pieces and converts them into 32-bit
fields (apparently signed in some places:-/).

> From what I understand about the mess in UUID formats, the time fields can
> either be big-endian (as defined) or little-endian (for all things
> Microsoft),

RFC 4122 specified that the multi-octet fields are stored MSB-first.

> and you are changing the representation from CPU-specific to big-endian,
> which makes it different for x86 and most ARM at least.

In-kernel, not in the protocol.

The problem is that you can't do what you put in your suggested patch and just
copy the UUID produced by the generate_random_uuid() over the afs_uuid struct
since that puts the version in the wrong place.

David

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
  2017-01-12 14:12 ` David Howells
@ 2017-01-12 15:40   ` Arnd Bergmann
  2017-01-12 16:14   ` David Howells
  1 sibling, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2017-01-12 15:40 UTC (permalink / raw)
  To: David Howells; +Cc: linux-kernel, ruchandani.tina, linux-afs

On Thursday, January 12, 2017 2:12:56 PM CET David Howells wrote:
> Arnd Bergmann <arnd@arndb.de> wrote:
> 
> > Looks good to me, but I wonder if this part:
> > 
> >                 r = call->request;
> > -               r->time_low                     = ntohl(b[0]);
> > -               r->time_mid                     = ntohl(b[1]);
> > -               r->time_hi_and_version          = ntohl(b[2]);
> > +               r->time_low                     = b[0];
> > +               r->time_mid                     = htons(ntohl(b[1]));
> > +               r->time_hi_and_version          = htons(ntohl(b[2]));
> >                 r->clock_seq_hi_and_reserved    = ntohl(b[3]);
> >                 r->clock_seq_low                = ntohl(b[4]);
> >  
> > should be considered a bugfix and split out into a
> > separate patch.
> 
> I changed the definitions in the struct from u16/u32 to __be16/__be32 so it's
> not a bugfix.

Ok.

> > From what I understand about the mess in UUID formats, the time fields can
> > either be big-endian (as defined) or little-endian (for all things
> > Microsoft),
> 
> RFC 4122 specified that the multi-octet fields are stored MSB-first.
> 
> > and you are changing the representation from CPU-specific to big-endian,
> > which makes it different for x86 and most ARM at least.
> 
> In-kernel, not in the protocol.

Ok, I assumed that the uuid was later sent out over the wire again
in the in-memory format, but you are right, it does get sent out in
the AFS specific format as a series of 32-bit big-endian values
rather than the RFC4122 format.
 
> The problem is that you can't do what you put in your suggested patch and just
> copy the UUID produced by the generate_random_uuid() over the afs_uuid struct
> since that puts the version in the wrong place.

Got it. One more thing then:

> @@ -569,9 +569,9 @@ static void SRXAFSCB_TellMeAboutYourself(struct
> work_struct *work)> 
>         memset(&reply, 0, sizeof(reply));
>         reply.ia.nifs = htonl(nifs);
> 
> -       reply.ia.uuid[0] = htonl(afs_uuid.time_low);
> -       reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
> -       reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
> +       reply.ia.uuid[0] = afs_uuid.time_low;
> +       reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
> +       reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
> 
>         reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
>         reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
>         for (loop = 0; loop < 6; loop++)

Shouldn't this be ntohs() instead of ntohl(), like this:

       reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
       reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));

My head is spinning a little from all the byteswapping, but it
looks to me like the data here ends up in the wrong half of the
on-wire data. Can you double-check this?

	Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
  2017-01-12 14:12 ` David Howells
  2017-01-12 15:40   ` Arnd Bergmann
@ 2017-01-12 16:14   ` David Howells
  2017-01-12 16:23     ` Arnd Bergmann
  2017-01-12 16:40     ` David Howells
  1 sibling, 2 replies; 11+ messages in thread
From: David Howells @ 2017-01-12 16:14 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: dhowells, linux-kernel, ruchandani.tina, linux-afs

Arnd Bergmann <arnd@arndb.de> wrote:

> > -       reply.ia.uuid[0] = htonl(afs_uuid.time_low);
> > -       reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
> > -       reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
> > +       reply.ia.uuid[0] = afs_uuid.time_low;
> > +       reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
> > +       reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
> > 
> >         reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
> >         reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
> >         for (loop = 0; loop < 6; loop++)
> 
> Shouldn't this be ntohs() instead of ntohl(), like this:
> 
>        reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
>        reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));

I think you forgot to change ntohl() to ntohs() in that - and you're right.

Okay, how about the attached?

David
---
commit 459327d2968663ad3c5377d84c357e8f0b5fcd83
Author: David Howells <dhowells@redhat.com>
Date:   Thu Jan 12 11:32:10 2017 +0000

    afs: Move UUID struct to linux/uuid.h
    
    Move the afs_uuid struct to linux/uuid.h, rename it to uuid_v1 and change
    the u16/u32 fields to __be16/__be32 instead so that the structure can be
    cast to a 16-octet network-order buffer.
    
    Signed-off-by: David Howells <dhowells@redhat.com>

diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index e349a3316303..2edbdcbf6432 100644
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -351,7 +351,7 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
 {
 	struct sockaddr_rxrpc srx;
 	struct afs_server *server;
-	struct afs_uuid *r;
+	struct uuid_v1 *r;
 	unsigned loop;
 	__be32 *b;
 	int ret;
@@ -381,15 +381,15 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
 		}
 
 		_debug("unmarshall UUID");
-		call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
+		call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
 		if (!call->request)
 			return -ENOMEM;
 
 		b = call->buffer;
 		r = call->request;
-		r->time_low			= ntohl(b[0]);
-		r->time_mid			= ntohl(b[1]);
-		r->time_hi_and_version		= ntohl(b[2]);
+		r->time_low			= b[0];
+		r->time_mid			= htons(ntohl(b[1]));
+		r->time_hi_and_version		= htons(ntohl(b[2]));
 		r->clock_seq_hi_and_reserved 	= ntohl(b[3]);
 		r->clock_seq_low		= ntohl(b[4]);
 
@@ -454,7 +454,7 @@ static int afs_deliver_cb_probe(struct afs_call *call)
 static void SRXAFSCB_ProbeUuid(struct work_struct *work)
 {
 	struct afs_call *call = container_of(work, struct afs_call, work);
-	struct afs_uuid *r = call->request;
+	struct uuid_v1 *r = call->request;
 
 	struct {
 		__be32	match;
@@ -477,7 +477,7 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
  */
 static int afs_deliver_cb_probe_uuid(struct afs_call *call)
 {
-	struct afs_uuid *r;
+	struct uuid_v1 *r;
 	unsigned loop;
 	__be32 *b;
 	int ret;
@@ -503,15 +503,15 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call)
 		}
 
 		_debug("unmarshall UUID");
-		call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
+		call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
 		if (!call->request)
 			return -ENOMEM;
 
 		b = call->buffer;
 		r = call->request;
-		r->time_low			= ntohl(b[0]);
-		r->time_mid			= ntohl(b[1]);
-		r->time_hi_and_version		= ntohl(b[2]);
+		r->time_low			= b[0];
+		r->time_mid			= htons(ntohl(b[1]));
+		r->time_hi_and_version		= htons(ntohl(b[2]));
 		r->clock_seq_hi_and_reserved 	= ntohl(b[3]);
 		r->clock_seq_low		= ntohl(b[4]);
 
@@ -569,9 +569,9 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct *work)
 	memset(&reply, 0, sizeof(reply));
 	reply.ia.nifs = htonl(nifs);
 
-	reply.ia.uuid[0] = htonl(afs_uuid.time_low);
-	reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
-	reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
+	reply.ia.uuid[0] = afs_uuid.time_low;
+	reply.ia.uuid[1] = htonl(ntohs(afs_uuid.time_mid));
+	reply.ia.uuid[2] = htonl(ntohs(afs_uuid.time_hi_and_version));
 	reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
 	reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
 	for (loop = 0; loop < 6; loop++)
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 65504e218d35..bb72dc6f7541 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -407,30 +407,6 @@ struct afs_interface {
 	unsigned	mtu;		/* MTU of interface */
 };
 
-/*
- * UUID definition [internet draft]
- * - the timestamp is a 60-bit value, split 32/16/12, and goes in 100ns
- *   increments since midnight 15th October 1582
- *   - add AFS_UUID_TO_UNIX_TIME to convert unix time in 100ns units to UUID
- *     time
- * - the clock sequence is a 14-bit counter to avoid duplicate times
- */
-struct afs_uuid {
-	u32		time_low;			/* low part of timestamp */
-	u16		time_mid;			/* mid part of timestamp */
-	u16		time_hi_and_version;		/* high part of timestamp and version  */
-#define AFS_UUID_TO_UNIX_TIME	0x01b21dd213814000ULL
-#define AFS_UUID_TIMEHI_MASK	0x0fff
-#define AFS_UUID_VERSION_TIME	0x1000	/* time-based UUID */
-#define AFS_UUID_VERSION_NAME	0x3000	/* name-based UUID */
-#define AFS_UUID_VERSION_RANDOM	0x4000	/* (pseudo-)random generated UUID */
-	u8		clock_seq_hi_and_reserved;	/* clock seq hi and variant */
-#define AFS_UUID_CLOCKHI_MASK	0x3f
-#define AFS_UUID_VARIANT_STD	0x80
-	u8		clock_seq_low;			/* clock seq low */
-	u8		node[6];			/* spatially unique node ID (MAC addr) */
-};
-
 /*****************************************************************************/
 /*
  * cache.c
@@ -565,7 +541,7 @@ extern int afs_drop_inode(struct inode *);
  * main.c
  */
 extern struct workqueue_struct *afs_wq;
-extern struct afs_uuid afs_uuid;
+extern struct uuid_v1 afs_uuid;
 
 /*
  * misc.c
diff --git a/fs/afs/main.c b/fs/afs/main.c
index f8188feb03ad..a07c14df3fd1 100644
--- a/fs/afs/main.c
+++ b/fs/afs/main.c
@@ -31,7 +31,7 @@ static char *rootcell;
 module_param(rootcell, charp, 0);
 MODULE_PARM_DESC(rootcell, "root AFS cell name and VL server IP addr list");
 
-struct afs_uuid afs_uuid;
+struct uuid_v1 afs_uuid;
 struct workqueue_struct *afs_wq;
 
 /*
@@ -41,7 +41,7 @@ static int __init afs_get_client_UUID(void)
 {
 	struct timespec ts;
 	u64 uuidtime;
-	u16 clockseq;
+	u16 clockseq, hi_v;
 	int ret;
 
 	/* read the MAC address of one of the external interfaces and construct
@@ -53,22 +53,23 @@ static int __init afs_get_client_UUID(void)
 	getnstimeofday(&ts);
 	uuidtime = (u64) ts.tv_sec * 1000 * 1000 * 10;
 	uuidtime += ts.tv_nsec / 100;
-	uuidtime += AFS_UUID_TO_UNIX_TIME;
-	afs_uuid.time_low = uuidtime;
-	afs_uuid.time_mid = uuidtime >> 32;
-	afs_uuid.time_hi_and_version = (uuidtime >> 48) & AFS_UUID_TIMEHI_MASK;
-	afs_uuid.time_hi_and_version |= AFS_UUID_VERSION_TIME;
+	uuidtime += UUID_TO_UNIX_TIME;
+	afs_uuid.time_low = htonl(uuidtime);
+	afs_uuid.time_mid = htons(uuidtime >> 32);
+	hi_v = (uuidtime >> 48) & UUID_TIMEHI_MASK;
+	hi_v |= UUID_VERSION_TIME;
+	afs_uuid.time_hi_and_version = htons(hi_v);
 
 	get_random_bytes(&clockseq, 2);
 	afs_uuid.clock_seq_low = clockseq;
 	afs_uuid.clock_seq_hi_and_reserved =
-		(clockseq >> 8) & AFS_UUID_CLOCKHI_MASK;
-	afs_uuid.clock_seq_hi_and_reserved |= AFS_UUID_VARIANT_STD;
+		(clockseq >> 8) & UUID_CLOCKHI_MASK;
+	afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
 
 	_debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
-	       afs_uuid.time_low,
-	       afs_uuid.time_mid,
-	       afs_uuid.time_hi_and_version,
+	       ntohl(afs_uuid.time_low),
+	       ntohs(afs_uuid.time_mid),
+	       ntohs(afs_uuid.time_hi_and_version),
 	       afs_uuid.clock_seq_hi_and_reserved,
 	       afs_uuid.clock_seq_low,
 	       afs_uuid.node[0], afs_uuid.node[1], afs_uuid.node[2],
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 2d095fc60204..4dff73a89758 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -19,6 +19,30 @@
 #include <uapi/linux/uuid.h>
 
 /*
+ * V1 (time-based) UUID definition [RFC 4122].
+ * - the timestamp is a 60-bit value, split 32/16/12, and goes in 100ns
+ *   increments since midnight 15th October 1582
+ *   - add AFS_UUID_TO_UNIX_TIME to convert unix time in 100ns units to UUID
+ *     time
+ * - the clock sequence is a 14-bit counter to avoid duplicate times
+ */
+struct uuid_v1 {
+	__be32		time_low;			/* low part of timestamp */
+	__be16		time_mid;			/* mid part of timestamp */
+	__be16		time_hi_and_version;		/* high part of timestamp and version  */
+#define UUID_TO_UNIX_TIME	0x01b21dd213814000ULL
+#define UUID_TIMEHI_MASK	0x0fff
+#define UUID_VERSION_TIME	0x1000	/* time-based UUID */
+#define UUID_VERSION_NAME	0x3000	/* name-based UUID */
+#define UUID_VERSION_RANDOM	0x4000	/* (pseudo-)random generated UUID */
+	u8		clock_seq_hi_and_reserved;	/* clock seq hi and variant */
+#define UUID_CLOCKHI_MASK	0x3f
+#define UUID_VARIANT_STD	0x80
+	u8		clock_seq_low;			/* clock seq low */
+	u8		node[6];			/* spatially unique node ID (MAC addr) */
+};
+
+/*
  * The length of a UUID string ("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
  * not including trailing NUL.
  */

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
  2017-01-12 16:14   ` David Howells
@ 2017-01-12 16:23     ` Arnd Bergmann
  2017-01-12 16:40     ` David Howells
  1 sibling, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2017-01-12 16:23 UTC (permalink / raw)
  To: David Howells; +Cc: Linux Kernel Mailing List, Tina Ruchandani, linux-afs

On Thu, Jan 12, 2017 at 5:14 PM, David Howells <dhowells@redhat.com> wrote:
>> >         reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
>> >         reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
>> >         for (loop = 0; loop < 6; loop++)
>>
>> Shouldn't this be ntohs() instead of ntohl(), like this:
>>
>>        reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
>>        reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
>
> I think you forgot to change ntohl() to ntohs() in that - and you're right.
>
> Okay, how about the attached?

Yes, looks good to me.

      Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
  2017-01-12 16:14   ` David Howells
  2017-01-12 16:23     ` Arnd Bergmann
@ 2017-01-12 16:40     ` David Howells
  1 sibling, 0 replies; 11+ messages in thread
From: David Howells @ 2017-01-12 16:40 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: dhowells, Linux Kernel Mailing List, Tina Ruchandani, linux-afs

Arnd Bergmann <arnd@arndb.de> wrote:

> Yes, looks good to me.

Can I put that down as a Reviewed-by?

David

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
  2017-01-12 11:56 [PATCH 1/2] afs: Move UUID struct to linux/uuid.h David Howells
                   ` (2 preceding siblings ...)
  2017-01-12 14:12 ` David Howells
@ 2017-01-13  9:22 ` kbuild test robot
  2017-01-13 10:28   ` David Howells
  2017-01-13 10:48 ` kbuild test robot
  4 siblings, 1 reply; 11+ messages in thread
From: kbuild test robot @ 2017-01-13  9:22 UTC (permalink / raw)
  To: David Howells
  Cc: kbuild-all, arnd, dhowells, linux-kernel, ruchandani.tina, linux-afs

[-- Attachment #1: Type: text/plain, Size: 11587 bytes --]

Hi David,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/David-Howells/afs-Move-UUID-struct-to-linux-uuid-h/20170113-134212
config: x86_64-randconfig-s4-01131622 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs/afs/cmservice.c: In function 'afs_deliver_cb_init_call_back_state3':
>> fs/afs/cmservice.c:365:34: error: invalid application of 'sizeof' to incomplete type 'struct uuid_v1'
      call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
                                     ^~~~~~
>> fs/afs/cmservice.c:371:4: error: dereferencing pointer to incomplete type 'struct uuid_v1'
      r->time_low   = b[0];
       ^~
   fs/afs/cmservice.c: In function 'SRXAFSCB_ProbeUuid':
   fs/afs/cmservice.c:449:33: error: invalid application of 'sizeof' to incomplete type 'struct uuid_v1'
     if (memcmp(r, &afs_uuid, sizeof(afs_uuid)) == 0)
                                    ^
   fs/afs/cmservice.c: In function 'afs_deliver_cb_probe_uuid':
   fs/afs/cmservice.c:489:34: error: invalid application of 'sizeof' to incomplete type 'struct uuid_v1'
      call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
                                     ^~~~~~
   fs/afs/cmservice.c: In function 'SRXAFSCB_TellMeAboutYourself':
>> fs/afs/cmservice.c:557:2: error: invalid use of undefined type 'struct uuid_v1'
     reply.ia.uuid[0] = afs_uuid.time_low;
     ^~~~~
   fs/afs/cmservice.c:558:2: error: invalid use of undefined type 'struct uuid_v1'
     reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
     ^~~~~
   fs/afs/cmservice.c:559:2: error: invalid use of undefined type 'struct uuid_v1'
     reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
     ^~~~~
   fs/afs/cmservice.c:560:2: error: invalid use of undefined type 'struct uuid_v1'
     reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
     ^~~~~
   fs/afs/cmservice.c:561:2: error: invalid use of undefined type 'struct uuid_v1'
     reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
     ^~~~~
   fs/afs/cmservice.c:563:3: error: invalid use of undefined type 'struct uuid_v1'
      reply.ia.uuid[loop + 5] = htonl((s8) afs_uuid.node[loop]);
      ^~~~~
--
   fs/afs/main.c: In function 'afs_get_client_UUID':
>> fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
     ret = afs_get_MAC_address(afs_uuid.node, sizeof(afs_uuid.node));
     ^~~
>> fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
>> fs/afs/main.c:55:14: error: 'UUID_TO_UNIX_TIME' undeclared (first use in this function)
     uuidtime += UUID_TO_UNIX_TIME;
                 ^~~~~~~~~~~~~~~~~
   fs/afs/main.c:55:14: note: each undeclared identifier is reported only once for each function it appears in
   fs/afs/main.c:56:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.time_low = htonl(uuidtime);
     ^~~~~~~~
   fs/afs/main.c:57:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.time_mid = htons(uuidtime >> 32);
     ^~~~~~~~
>> fs/afs/main.c:58:28: error: 'UUID_TIMEHI_MASK' undeclared (first use in this function)
     hi_v = (uuidtime >> 48) & UUID_TIMEHI_MASK;
                               ^~~~~~~~~~~~~~~~
>> fs/afs/main.c:59:10: error: 'UUID_VERSION_TIME' undeclared (first use in this function)
     hi_v |= UUID_VERSION_TIME;
             ^~~~~~~~~~~~~~~~~
   fs/afs/main.c:60:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.time_hi_and_version = htons(hi_v);
     ^~~~~~~~
   fs/afs/main.c:63:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.clock_seq_low = clockseq;
     ^~~~~~~~
   fs/afs/main.c:64:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.clock_seq_hi_and_reserved =
     ^~~~~~~~
>> fs/afs/main.c:65:21: error: 'UUID_CLOCKHI_MASK' undeclared (first use in this function)
      (clockseq >> 8) & UUID_CLOCKHI_MASK;
                        ^~~~~~~~~~~~~~~~~
   fs/afs/main.c:66:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
     ^~~~~~~~
>> fs/afs/main.c:66:40: error: 'UUID_VARIANT_STD' undeclared (first use in this function)
     afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
                                           ^~~~~~~~~~~~~~~~
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c: At top level:
>> fs/afs/main.c:33:16: error: storage size of 'afs_uuid' isn't known
    struct uuid_v1 afs_uuid;
                   ^~~~~~~~

vim +365 fs/afs/cmservice.c

   359			case 0:		break;
   360			case -EAGAIN:	return 0;
   361			default:	return ret;
   362			}
   363	
   364			_debug("unmarshall UUID");
 > 365			call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
   366			if (!call->request)
   367				return -ENOMEM;
   368	
   369			b = call->buffer;
   370			r = call->request;
 > 371			r->time_low			= b[0];
   372			r->time_mid			= htons(ntohl(b[1]));
   373			r->time_hi_and_version		= htons(ntohl(b[2]));
   374			r->clock_seq_hi_and_reserved 	= ntohl(b[3]);
   375			r->clock_seq_low		= ntohl(b[4]);
   376	
   377			for (loop = 0; loop < 6; loop++)
   378				r->node[loop] = ntohl(b[loop + 5]);
   379	
   380			call->offset = 0;
   381			call->unmarshall++;
   382	
   383		case 2:
   384			break;
   385		}
   386	
   387		/* no unmarshalling required */
   388		call->state = AFS_CALL_REPLYING;
   389	
   390		/* we'll need the file server record as that tells us which set of
   391		 * vnodes to operate upon */
   392		server = afs_find_server(&srx);
   393		if (!server)
   394			return -ENOTCONN;
   395		call->server = server;
   396	
   397		INIT_WORK(&call->work, SRXAFSCB_InitCallBackState);
   398		queue_work(afs_wq, &call->work);
   399		return 0;
   400	}
   401	
   402	/*
   403	 * allow the fileserver to see if the cache manager is still alive
   404	 */
   405	static void SRXAFSCB_Probe(struct work_struct *work)
   406	{
   407		struct afs_call *call = container_of(work, struct afs_call, work);
   408	
   409		_enter("");
   410		afs_send_empty_reply(call);
   411		_leave("");
   412	}
   413	
   414	/*
   415	 * deliver request data to a CB.Probe call
   416	 */
   417	static int afs_deliver_cb_probe(struct afs_call *call)
   418	{
   419		int ret;
   420	
   421		_enter("");
   422	
   423		ret = afs_extract_data(call, NULL, 0, false);
   424		if (ret < 0)
   425			return ret;
   426	
   427		/* no unmarshalling required */
   428		call->state = AFS_CALL_REPLYING;
   429	
   430		INIT_WORK(&call->work, SRXAFSCB_Probe);
   431		queue_work(afs_wq, &call->work);
   432		return 0;
   433	}
   434	
   435	/*
   436	 * allow the fileserver to quickly find out if the fileserver has been rebooted
   437	 */
   438	static void SRXAFSCB_ProbeUuid(struct work_struct *work)
   439	{
   440		struct afs_call *call = container_of(work, struct afs_call, work);
   441		struct uuid_v1 *r = call->request;
   442	
   443		struct {
   444			__be32	match;
   445		} reply;
   446	
   447		_enter("");
   448	
 > 449		if (memcmp(r, &afs_uuid, sizeof(afs_uuid)) == 0)
   450			reply.match = htonl(0);
   451		else
   452			reply.match = htonl(1);
   453	
   454		afs_send_simple_reply(call, &reply, sizeof(reply));
   455		_leave("");
   456	}
   457	
   458	/*
   459	 * deliver request data to a CB.ProbeUuid call
   460	 */
   461	static int afs_deliver_cb_probe_uuid(struct afs_call *call)
   462	{
   463		struct uuid_v1 *r;
   464		unsigned loop;
   465		__be32 *b;
   466		int ret;
   467	
   468		_enter("{%u}", call->unmarshall);
   469	
   470		switch (call->unmarshall) {
   471		case 0:
   472			call->offset = 0;
   473			call->buffer = kmalloc(11 * sizeof(__be32), GFP_KERNEL);
   474			if (!call->buffer)
   475				return -ENOMEM;
   476			call->unmarshall++;
   477	
   478		case 1:
   479			_debug("extract UUID");
   480			ret = afs_extract_data(call, call->buffer,
   481					       11 * sizeof(__be32), false);
   482			switch (ret) {
   483			case 0:		break;
   484			case -EAGAIN:	return 0;
   485			default:	return ret;
   486			}
   487	
   488			_debug("unmarshall UUID");
   489			call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
   490			if (!call->request)
   491				return -ENOMEM;
   492	
   493			b = call->buffer;
   494			r = call->request;
   495			r->time_low			= ntohl(b[0]);
   496			r->time_mid			= ntohl(b[1]);
   497			r->time_hi_and_version		= ntohl(b[2]);
   498			r->clock_seq_hi_and_reserved 	= ntohl(b[3]);
   499			r->clock_seq_low		= ntohl(b[4]);
   500	
   501			for (loop = 0; loop < 6; loop++)
   502				r->node[loop] = ntohl(b[loop + 5]);
   503	
   504			call->offset = 0;
   505			call->unmarshall++;
   506	
   507		case 2:
   508			break;
   509		}
   510	
   511		call->state = AFS_CALL_REPLYING;
   512	
   513		INIT_WORK(&call->work, SRXAFSCB_ProbeUuid);
   514		queue_work(afs_wq, &call->work);
   515		return 0;
   516	}
   517	
   518	/*
   519	 * allow the fileserver to ask about the cache manager's capabilities
   520	 */
   521	static void SRXAFSCB_TellMeAboutYourself(struct work_struct *work)
   522	{
   523		struct afs_interface *ifs;
   524		struct afs_call *call = container_of(work, struct afs_call, work);
   525		int loop, nifs;
   526	
   527		struct {
   528			struct /* InterfaceAddr */ {
   529				__be32 nifs;
   530				__be32 uuid[11];
   531				__be32 ifaddr[32];
   532				__be32 netmask[32];
   533				__be32 mtu[32];
   534			} ia;
   535			struct /* Capabilities */ {
   536				__be32 capcount;
   537				__be32 caps[1];
   538			} cap;
   539		} reply;
   540	
   541		_enter("");
   542	
   543		nifs = 0;
   544		ifs = kcalloc(32, sizeof(*ifs), GFP_KERNEL);
   545		if (ifs) {
   546			nifs = afs_get_ipv4_interfaces(ifs, 32, false);
   547			if (nifs < 0) {
   548				kfree(ifs);
   549				ifs = NULL;
   550				nifs = 0;
   551			}
   552		}
   553	
   554		memset(&reply, 0, sizeof(reply));
   555		reply.ia.nifs = htonl(nifs);
   556	
 > 557		reply.ia.uuid[0] = afs_uuid.time_low;
   558		reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
   559		reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
   560		reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25064 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
  2017-01-13  9:22 ` kbuild test robot
@ 2017-01-13 10:28   ` David Howells
  0 siblings, 0 replies; 11+ messages in thread
From: David Howells @ 2017-01-13 10:28 UTC (permalink / raw)
  To: kbuild test robot
  Cc: dhowells, kbuild-all, arnd, linux-kernel, ruchandani.tina, linux-afs

kbuild test robot <lkp@intel.com> wrote:

>    fs/afs/cmservice.c: In function 'afs_deliver_cb_init_call_back_state3':
> >> fs/afs/cmservice.c:365:34: error: invalid application of 'sizeof' to incomplete type 'struct uuid_v1'
>       call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
>                                      ^~~~~~

Ah...  Missing #include of linux/uuid.h.  Under some circumstances it is
included indirectly.

David

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h
  2017-01-12 11:56 [PATCH 1/2] afs: Move UUID struct to linux/uuid.h David Howells
                   ` (3 preceding siblings ...)
  2017-01-13  9:22 ` kbuild test robot
@ 2017-01-13 10:48 ` kbuild test robot
  4 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2017-01-13 10:48 UTC (permalink / raw)
  To: David Howells
  Cc: kbuild-all, arnd, dhowells, linux-kernel, ruchandani.tina, linux-afs

[-- Attachment #1: Type: text/plain, Size: 11315 bytes --]

Hi David,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/David-Howells/afs-Move-UUID-struct-to-linux-uuid-h/20170113-134212
config: x86_64-randconfig-b0-01131757 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/David-Howells/afs-Move-UUID-struct-to-linux-uuid-h/20170113-134212 HEAD c4259bc3968ea592af3a1599099327eb67e6019d builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   fs/afs/main.c: In function 'afs_get_client_UUID':
   fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
     ret = afs_get_MAC_address(afs_uuid.node, sizeof(afs_uuid.node));
     ^~~
   fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:55:14: error: 'UUID_TO_UNIX_TIME' undeclared (first use in this function)
     uuidtime += UUID_TO_UNIX_TIME;
                 ^~~~~~~~~~~~~~~~~
   fs/afs/main.c:55:14: note: each undeclared identifier is reported only once for each function it appears in
   fs/afs/main.c:56:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.time_low = htonl(uuidtime);
     ^~~~~~~~
   fs/afs/main.c:57:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.time_mid = htons(uuidtime >> 32);
     ^~~~~~~~
   fs/afs/main.c:58:28: error: 'UUID_TIMEHI_MASK' undeclared (first use in this function)
     hi_v = (uuidtime >> 48) & UUID_TIMEHI_MASK;
                               ^~~~~~~~~~~~~~~~
   fs/afs/main.c:59:10: error: 'UUID_VERSION_TIME' undeclared (first use in this function)
     hi_v |= UUID_VERSION_TIME;
             ^~~~~~~~~~~~~~~~~
   fs/afs/main.c:60:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.time_hi_and_version = htons(hi_v);
     ^~~~~~~~
   fs/afs/main.c:63:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.clock_seq_low = clockseq;
     ^~~~~~~~
   fs/afs/main.c:64:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.clock_seq_hi_and_reserved =
     ^~~~~~~~
   fs/afs/main.c:65:21: error: 'UUID_CLOCKHI_MASK' undeclared (first use in this function)
      (clockseq >> 8) & UUID_CLOCKHI_MASK;
                        ^~~~~~~~~~~~~~~~~
   fs/afs/main.c:66:2: error: invalid use of undefined type 'struct uuid_v1'
     afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
     ^~~~~~~~
   fs/afs/main.c:66:40: error: 'UUID_VARIANT_STD' undeclared (first use in this function)
     afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
                                           ^~~~~~~~~~~~~~~~
   In file included from include/uapi/linux/stddef.h:1:0,
                    from include/linux/stddef.h:4,
                    from include/uapi/linux/posix_types.h:4,
                    from include/uapi/linux/types.h:13,
                    from include/linux/types.h:5,
                    from include/linux/list.h:4,
                    from include/linux/module.h:9,
                    from fs/afs/main.c:12:
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 'struct uuid_v1'
       static struct ftrace_branch_data  \
                     ^
   include/linux/compiler.h:139:58: note: in expansion of macro '__branch_check__'
    #  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : __branch_check__(x, 0))
                                                             ^~~~~~~~~~~~~~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
     if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
         ^~~~~~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
     _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
     ^~~~~~

vim +117 include/linux/compiler.h

1f0d69a9 Steven Rostedt 2008-11-12  111  
1f0d69a9 Steven Rostedt 2008-11-12  112  #define likely_notrace(x)	__builtin_expect(!!(x), 1)
1f0d69a9 Steven Rostedt 2008-11-12  113  #define unlikely_notrace(x)	__builtin_expect(!!(x), 0)
1f0d69a9 Steven Rostedt 2008-11-12  114  
45b79749 Steven Rostedt 2008-11-21  115  #define __branch_check__(x, expect) ({					\
1f0d69a9 Steven Rostedt 2008-11-12  116  			int ______r;					\
2ed84eeb Steven Rostedt 2008-11-12 @117  			static struct ftrace_branch_data		\
1f0d69a9 Steven Rostedt 2008-11-12  118  				__attribute__((__aligned__(4)))		\
45b79749 Steven Rostedt 2008-11-21  119  				__attribute__((section("_ftrace_annotated_branch"))) \
1f0d69a9 Steven Rostedt 2008-11-12  120  				______f = {				\

:::::: The code at line 117 was first introduced by commit
:::::: 2ed84eeb8808cf3c9f039213ca137ffd7d753f0e trace: rename unlikely profiler to branch profiler

:::::: TO: Steven Rostedt <srostedt@redhat.com>
:::::: CC: Ingo Molnar <mingo@elte.hu>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23523 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-01-13 10:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-12 11:56 [PATCH 1/2] afs: Move UUID struct to linux/uuid.h David Howells
2017-01-12 11:56 ` [PATCH 2/2] afs: Use core kernel UUID generation David Howells
2017-01-12 13:14 ` [PATCH 1/2] afs: Move UUID struct to linux/uuid.h Arnd Bergmann
2017-01-12 14:12 ` David Howells
2017-01-12 15:40   ` Arnd Bergmann
2017-01-12 16:14   ` David Howells
2017-01-12 16:23     ` Arnd Bergmann
2017-01-12 16:40     ` David Howells
2017-01-13  9:22 ` kbuild test robot
2017-01-13 10:28   ` David Howells
2017-01-13 10:48 ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.