2008-09-12 01:41:58

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/7] lockd: Use sockaddr_storage for h_saddr field

On Wed, Sep 03, 2008 at 02:35:46PM -0400, Chuck Lever wrote:
> @@ -294,7 +294,7 @@ nlm_bind_host(struct nlm_host *host)
> .protocol = host->h_proto,
> .address = nlm_addr(host),
> .addrsize = host->h_addrlen,
> - .saddress = (struct sockaddr *)&host->h_saddr,
> + .saddress = nlm_srcaddr(host),

Just out of curiosity--in the patch 1 and 3, you also add a length
field. Here you don't. Why the difference? None of the new length
fields seem to be used yet.

--b.

> .timeout = &timeparms,
> .servername = host->h_name,
> .program = &nlm_program,
> diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
> index 198b4e5..d3d1330 100644
> --- a/fs/lockd/svcsubs.c
> +++ b/fs/lockd/svcsubs.c
> @@ -418,7 +418,7 @@ EXPORT_SYMBOL_GPL(nlmsvc_unlock_all_by_sb);
> static int
> nlmsvc_match_ip(void *datap, struct nlm_host *host)
> {
> - return nlm_cmp_addr(&host->h_saddr, datap);
> + return nlm_cmp_addr(nlm_srcaddr_in(host), datap);
> }
>
> /**
> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
> index 41d7a8e..964e6c9 100644
> --- a/include/linux/lockd/lockd.h
> +++ b/include/linux/lockd/lockd.h
> @@ -40,7 +40,7 @@ struct nlm_host {
> struct hlist_node h_hash; /* doubly linked list */
> struct sockaddr_storage h_addr; /* peer address */
> size_t h_addrlen;
> - struct sockaddr_in h_saddr; /* our address (optional) */
> + struct sockaddr_storage h_srcaddr; /* our address (optional) */
> struct rpc_clnt * h_rpcclnt; /* RPC client to talk to peer */
> char * h_name; /* remote hostname */
> u32 h_version; /* interface version */
> @@ -64,7 +64,7 @@ struct nlm_host {
> struct nsm_handle * h_nsmhandle; /* NSM status handle */
>
> char h_addrbuf[48], /* address eyecatchers */
> - h_saddrbuf[48];
> + h_srcaddrbuf[48];
> };
>
> struct nsm_handle {
> @@ -90,6 +90,16 @@ static inline struct sockaddr *nlm_addr(const struct nlm_host *host)
> return (struct sockaddr *)&host->h_addr;
> }
>
> +static inline struct sockaddr_in *nlm_srcaddr_in(const struct nlm_host *host)
> +{
> + return (struct sockaddr_in *)&host->h_srcaddr;
> +}
> +
> +static inline struct sockaddr *nlm_srcaddr(const struct nlm_host *host)
> +{
> + return (struct sockaddr *)&host->h_srcaddr;
> +}
> +
> /*
> * Map an fl_owner_t into a unique 32-bit "pid"
> */
>


2008-09-12 03:14:41

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH 2/7] lockd: Use sockaddr_storage for h_saddr field

On Thu, Sep 11, 2008 at 9:41 PM, J. Bruce Fields <[email protected]> wrote:
> On Wed, Sep 03, 2008 at 02:35:46PM -0400, Chuck Lever wrote:
>> @@ -294,7 +294,7 @@ nlm_bind_host(struct nlm_host *host)
>> .protocol = host->h_proto,
>> .address = nlm_addr(host),
>> .addrsize = host->h_addrlen,
>> - .saddress = (struct sockaddr *)&host->h_saddr,
>> + .saddress = nlm_srcaddr(host),
>
> Just out of curiosity--in the patch 1 and 3, you also add a length
> field. Here you don't. Why the difference? None of the new length
> fields seem to be used yet.

h_addrlen should be used in nlm_bind_host already in patch 1. When we
get up to the NSM changes, sm_addrlen will be used in nsm_mon_unmon.

The source address is provided to rpc_create() only, which doesn't
take an address length.

>> .timeout = &timeparms,
>> .servername = host->h_name,
>> .program = &nlm_program,
>> diff --git a/fs/lockd/svcsubs.c b/fs/lockd/svcsubs.c
>> index 198b4e5..d3d1330 100644
>> --- a/fs/lockd/svcsubs.c
>> +++ b/fs/lockd/svcsubs.c
>> @@ -418,7 +418,7 @@ EXPORT_SYMBOL_GPL(nlmsvc_unlock_all_by_sb);
>> static int
>> nlmsvc_match_ip(void *datap, struct nlm_host *host)
>> {
>> - return nlm_cmp_addr(&host->h_saddr, datap);
>> + return nlm_cmp_addr(nlm_srcaddr_in(host), datap);
>> }
>>
>> /**
>> diff --git a/include/linux/lockd/lockd.h b/include/linux/lockd/lockd.h
>> index 41d7a8e..964e6c9 100644
>> --- a/include/linux/lockd/lockd.h
>> +++ b/include/linux/lockd/lockd.h
>> @@ -40,7 +40,7 @@ struct nlm_host {
>> struct hlist_node h_hash; /* doubly linked list */
>> struct sockaddr_storage h_addr; /* peer address */
>> size_t h_addrlen;
>> - struct sockaddr_in h_saddr; /* our address (optional) */
>> + struct sockaddr_storage h_srcaddr; /* our address (optional) */
>> struct rpc_clnt * h_rpcclnt; /* RPC client to talk to peer */
>> char * h_name; /* remote hostname */
>> u32 h_version; /* interface version */
>> @@ -64,7 +64,7 @@ struct nlm_host {
>> struct nsm_handle * h_nsmhandle; /* NSM status handle */
>>
>> char h_addrbuf[48], /* address eyecatchers */
>> - h_saddrbuf[48];
>> + h_srcaddrbuf[48];
>> };
>>
>> struct nsm_handle {
>> @@ -90,6 +90,16 @@ static inline struct sockaddr *nlm_addr(const struct nlm_host *host)
>> return (struct sockaddr *)&host->h_addr;
>> }
>>
>> +static inline struct sockaddr_in *nlm_srcaddr_in(const struct nlm_host *host)
>> +{
>> + return (struct sockaddr_in *)&host->h_srcaddr;
>> +}
>> +
>> +static inline struct sockaddr *nlm_srcaddr(const struct nlm_host *host)
>> +{
>> + return (struct sockaddr *)&host->h_srcaddr;
>> +}
>> +
>> /*
>> * Map an fl_owner_t into a unique 32-bit "pid"
>> */
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>



--
"If you simplify your English, you are freed from the worst follies of
orthodoxy."
-- George Orwell