| Message ID | 1447799594-6050-3-git-send-email-alex.aring@gmail.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello. On 11/18/2015 1:33 AM, Alexander Aring wrote: > This patch adds a static inline function ipv6_addr_prefix_cpy which I suggest not to reduce "copy". > copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix. > The prefix len is given by plen as bits. This function mainly based on > ipv6_addr_prefix which copies one address prefix from address into a new > ipv6 address destination and zero all other address bits. > > The difference is that ipv6_addr_prefix_cpy don't get a prefix from an > ipv6 address, it sets a prefix to an ipv6 address with keeping other > address bits. The use case is for context based address compression > inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context > table to lookup address-bits without sending them. > > Cc: David S. Miller <davem@davemloft.net> > Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> > Cc: James Morris <jmorris@namei.org> > Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> > Cc: Patrick McHardy <kaber@trash.net> > Signed-off-by: Alexander Aring <alex.aring@gmail.com> > --- > include/net/ipv6.h | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/include/net/ipv6.h b/include/net/ipv6.h > index e1a10b0..9d38fc2 100644 > --- a/include/net/ipv6.h > +++ b/include/net/ipv6.h > @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx, > pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b); > } > > +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr, > + const struct in6_addr *pfx, > + int plen) > +{ > + /* caller must guarantee 0 <= plen <= 128 */ > + int o = plen >> 3, > + b = plen & 0x7; Unusual declaration style, why not just have *int* on both lines? [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 18/11/15 14:55, Sergei Shtylyov wrote: > Hello. > > On 11/18/2015 1:33 AM, Alexander Aring wrote: > >> This patch adds a static inline function ipv6_addr_prefix_cpy which > > I suggest not to reduce "copy". Agreed. Not worth saving one character here. > >> copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix. >> The prefix len is given by plen as bits. This function mainly based on >> ipv6_addr_prefix which copies one address prefix from address into a new >> ipv6 address destination and zero all other address bits. >> >> The difference is that ipv6_addr_prefix_cpy don't get a prefix from an >> ipv6 address, it sets a prefix to an ipv6 address with keeping other >> address bits. The use case is for context based address compression >> inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context >> table to lookup address-bits without sending them. >> >> Cc: David S. Miller <davem@davemloft.net> >> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> >> Cc: James Morris <jmorris@namei.org> >> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> >> Cc: Patrick McHardy <kaber@trash.net> >> Signed-off-by: Alexander Aring <alex.aring@gmail.com> >> --- >> include/net/ipv6.h | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/include/net/ipv6.h b/include/net/ipv6.h >> index e1a10b0..9d38fc2 100644 >> --- a/include/net/ipv6.h >> +++ b/include/net/ipv6.h >> @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct >> in6_addr *pfx, >> pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b); >> } >> >> +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr, >> + const struct in6_addr *pfx, >> + int plen) >> +{ >> + /* caller must guarantee 0 <= plen <= 128 */ >> + int o = plen >> 3, >> + b = plen & 0x7; > > Unusual declaration style, why not just have *int* on both lines? > He took that from ipv6_addr_prefix() defined above it. I would also prefer a second line with int for the second declaration. But as he followed the coding style already around I think both way are fine. regards Stefan Schmidt -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 17/11/15 23:33, Alexander Aring wrote: > This patch adds a static inline function ipv6_addr_prefix_cpy which > copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix. > The prefix len is given by plen as bits. This function mainly based on > ipv6_addr_prefix which copies one address prefix from address into a new > ipv6 address destination and zero all other address bits. > > The difference is that ipv6_addr_prefix_cpy don't get a prefix from an > ipv6 address, it sets a prefix to an ipv6 address with keeping other > address bits. The use case is for context based address compression > inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context > table to lookup address-bits without sending them. > > Cc: David S. Miller<davem@davemloft.net> > Cc: Alexey Kuznetsov<kuznet@ms2.inr.ac.ru> > Cc: James Morris<jmorris@namei.org> > Cc: Hideaki YOSHIFUJI<yoshfuji@linux-ipv6.org> > Cc: Patrick McHardy<kaber@trash.net> > Signed-off-by: Alexander Aring<alex.aring@gmail.com> > --- > include/net/ipv6.h | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/include/net/ipv6.h b/include/net/ipv6.h > index e1a10b0..9d38fc2 100644 > --- a/include/net/ipv6.h > +++ b/include/net/ipv6.h > @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx, > pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b); > } > > +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr, > + const struct in6_addr *pfx, > + int plen) > +{ Naming it ipv6_addr_prefix_cop, as Sergei suggested, would be easier to read. > + /* caller must guarantee 0 <= plen <= 128 */ > + int o = plen >> 3, > + b = plen & 0x7; > + Any reason you are not doing the memset here before memcpy like it is done in ipv6_addr_prefi()? > + memcpy(addr->s6_addr, pfx, o); > + if (b != 0) { > + addr->s6_addr[o] &= ~(0xff00 >> b); > + addr->s6_addr[o] |= (pfx->s6_addr[o] & (0xff00 >> b)); > + } > +} > + > static inline void __ipv6_addr_set_half(__be32 *addr, > __be32 wh, __be32 wl) > { regards Stefan Schmidt regards Stefan Schmidt -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 25, 2015 at 05:42:52PM +0100, Stefan Schmidt wrote: > Hello. > > On 17/11/15 23:33, Alexander Aring wrote: > >This patch adds a static inline function ipv6_addr_prefix_cpy which > >copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix. > >The prefix len is given by plen as bits. This function mainly based on > >ipv6_addr_prefix which copies one address prefix from address into a new > >ipv6 address destination and zero all other address bits. > > > >The difference is that ipv6_addr_prefix_cpy don't get a prefix from an > >ipv6 address, it sets a prefix to an ipv6 address with keeping other > >address bits. The use case is for context based address compression > >inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context > >table to lookup address-bits without sending them. > > > >Cc: David S. Miller<davem@davemloft.net> > >Cc: Alexey Kuznetsov<kuznet@ms2.inr.ac.ru> > >Cc: James Morris<jmorris@namei.org> > >Cc: Hideaki YOSHIFUJI<yoshfuji@linux-ipv6.org> > >Cc: Patrick McHardy<kaber@trash.net> > >Signed-off-by: Alexander Aring<alex.aring@gmail.com> > >--- > > include/net/ipv6.h | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > >diff --git a/include/net/ipv6.h b/include/net/ipv6.h > >index e1a10b0..9d38fc2 100644 > >--- a/include/net/ipv6.h > >+++ b/include/net/ipv6.h > >@@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx, > > pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b); > > } > >+static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr, > >+ const struct in6_addr *pfx, > >+ int plen) > >+{ > > Naming it ipv6_addr_prefix_cop, as Sergei suggested, would be easier to > read. ok. > >+ /* caller must guarantee 0 <= plen <= 128 */ > >+ int o = plen >> 3, > >+ b = plen & 0x7; > >+ > > Any reason you are not doing the memset here before memcpy like it is done > in ipv6_addr_prefi()? ipv6_addr_prefix is more a "getter" for a prefix from an address. The destination address (which should be the result) will memset the whole address before copy the prefix from address to it. THe ipv6_addr_prefix_copy will set a prefix given by second parameter pfx and set the prefix to the address (given by first parameter). Maybe the ipv6_addr_prefix could call: 1. memset(pfx, sizeof...) 2. ipv6_addr_prefix_copy(pfx, addr, plen); but ipv6_addr_prefix_copy need to care about other bits which are set at destination address, if (plen/8 != 0). I don't want to touch ipv6_addr_prefix function and slow-down some mechanism. - Alex -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 25/11/15 19:17, Alexander Aring wrote: > On Wed, Nov 25, 2015 at 05:42:52PM +0100, Stefan Schmidt wrote: >> Hello. >> >> On 17/11/15 23:33, Alexander Aring wrote: >>> This patch adds a static inline function ipv6_addr_prefix_cpy which >>> copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix. >>> The prefix len is given by plen as bits. This function mainly based on >>> ipv6_addr_prefix which copies one address prefix from address into a new >>> ipv6 address destination and zero all other address bits. >>> >>> The difference is that ipv6_addr_prefix_cpy don't get a prefix from an >>> ipv6 address, it sets a prefix to an ipv6 address with keeping other >>> address bits. The use case is for context based address compression >>> inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context >>> table to lookup address-bits without sending them. >>> >>> Cc: David S. Miller<davem@davemloft.net> >>> Cc: Alexey Kuznetsov<kuznet@ms2.inr.ac.ru> >>> Cc: James Morris<jmorris@namei.org> >>> Cc: Hideaki YOSHIFUJI<yoshfuji@linux-ipv6.org> >>> Cc: Patrick McHardy<kaber@trash.net> >>> Signed-off-by: Alexander Aring<alex.aring@gmail.com> >>> --- >>> include/net/ipv6.h | 15 +++++++++++++++ >>> 1 file changed, 15 insertions(+) >>> >>> diff --git a/include/net/ipv6.h b/include/net/ipv6.h >>> index e1a10b0..9d38fc2 100644 >>> --- a/include/net/ipv6.h >>> +++ b/include/net/ipv6.h >>> @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx, >>> pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b); >>> } >>> +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr, >>> + const struct in6_addr *pfx, >>> + int plen) >>> +{ >> Naming it ipv6_addr_prefix_cop, as Sergei suggested, would be easier to >> read. > ok. > >>> + /* caller must guarantee 0 <= plen <= 128 */ >>> + int o = plen >> 3, >>> + b = plen & 0x7; >>> + >> Any reason you are not doing the memset here before memcpy like it is done >> in ipv6_addr_prefi()? > ipv6_addr_prefix is more a "getter" for a prefix from an address. The > destination address (which should be the result) will memset the whole > address before copy the prefix from address to it. Ah, right. Missed that part. > THe ipv6_addr_prefix_copy will set a prefix given by second parameter > pfx and set the prefix to the address (given by first parameter). > > Maybe the ipv6_addr_prefix could call: > > 1. memset(pfx, sizeof...) > 2. ipv6_addr_prefix_copy(pfx, addr, plen); > > but ipv6_addr_prefix_copy need to care about other bits which are set at > destination address, if (plen/8 != 0). I don't want to touch > ipv6_addr_prefix function and slow-down some mechanism. No need to change this from my side. I just missed the memset being handled in the destination address. With the name change to _copy you can add my Reviewed-by: Stefan Schmidt <stefan@osg.samsung.com> regards Stefan Schmidt -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/ipv6.h b/include/net/ipv6.h index e1a10b0..9d38fc2 100644 --- a/include/net/ipv6.h +++ b/include/net/ipv6.h @@ -382,6 +382,21 @@ static inline void ipv6_addr_prefix(struct in6_addr *pfx, pfx->s6_addr[o] = addr->s6_addr[o] & (0xff00 >> b); } +static inline void ipv6_addr_prefix_cpy(struct in6_addr *addr, + const struct in6_addr *pfx, + int plen) +{ + /* caller must guarantee 0 <= plen <= 128 */ + int o = plen >> 3, + b = plen & 0x7; + + memcpy(addr->s6_addr, pfx, o); + if (b != 0) { + addr->s6_addr[o] &= ~(0xff00 >> b); + addr->s6_addr[o] |= (pfx->s6_addr[o] & (0xff00 >> b)); + } +} + static inline void __ipv6_addr_set_half(__be32 *addr, __be32 wh, __be32 wl) {
This patch adds a static inline function ipv6_addr_prefix_cpy which copies a ipv6 address prefix(argument pfx) into the ipv6 address prefix. The prefix len is given by plen as bits. This function mainly based on ipv6_addr_prefix which copies one address prefix from address into a new ipv6 address destination and zero all other address bits. The difference is that ipv6_addr_prefix_cpy don't get a prefix from an ipv6 address, it sets a prefix to an ipv6 address with keeping other address bits. The use case is for context based address compression inside 6LoWPAN IPHC header which keeping ipv6 prefixes inside a context table to lookup address-bits without sending them. Cc: David S. Miller <davem@davemloft.net> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> Cc: James Morris <jmorris@namei.org> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> Cc: Patrick McHardy <kaber@trash.net> Signed-off-by: Alexander Aring <alex.aring@gmail.com> --- include/net/ipv6.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)