Mailing List Archive: Quagga: Dev

[PATCH 25/25] lib: optimize IPV4_ADDR_[SAME|COPY]()

Index | Next | Previous | View Threaded

jorge at dti2

May 7, 2012, 10:53 AM

Post #1 of 9 (334 views)
Permalink

[PATCH 25/25] lib: optimize IPV4_ADDR_[SAME\|COPY]()

From: "Jorge Boncompte [DTI2]" <jorge [at] dti2>

* lib/prefix.c: (prefix_same) changed to use prefix4 field.
* ospfd/ospf_packet.c: (ospf_ls_upd_send) changed to use prefix4 field.

Now that all callers use the prefix4 field...

* lib/prefix.h: use assignment and comparison instead of memcpy() and
memcmp(). Avoids function calls. Much faster.

Signed-off-by: Jorge Boncompte [DTI2] <jorge [at] dti2>
---
lib/prefix.c | 2 +-
lib/prefix.h | 4 ++--
ospfd/ospf_packet.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/prefix.c b/lib/prefix.c
index a3b1adf..48b3f5b 100644
--- a/lib/prefix.c
+++ b/lib/prefix.c
@@ -281,7 +281,7 @@ prefix_same (const struct prefix *p1, const struct prefix *p2)
if (p1->family == p2->family && p1->prefixlen == p2->prefixlen)
{
if (p1->family == AF_INET)
- if (IPV4_ADDR_SAME (&p1->u.prefix4.s_addr, &p2->u.prefix4.s_addr))
+ if (IPV4_ADDR_SAME (&p1->u.prefix4, &p2->u.prefix4))
return 1;
#ifdef HAVE_IPV6
if (p1->family == AF_INET6 )
diff --git a/lib/prefix.h b/lib/prefix.h
index 7f0d360..a97ff24 100644
--- a/lib/prefix.h
+++ b/lib/prefix.h
@@ -106,8 +106,8 @@ struct prefix_rd
#define IPV4_MAX_BITLEN 32
#define IPV4_MAX_PREFIXLEN 32
#define IPV4_ADDR_CMP(D,S) memcmp ((D), (S), IPV4_MAX_BYTELEN)
-#define IPV4_ADDR_SAME(D,S) (memcmp ((D), (S), IPV4_MAX_BYTELEN) == 0)
-#define IPV4_ADDR_COPY(D,S) memcpy ((D), (S), IPV4_MAX_BYTELEN)
+#define IPV4_ADDR_SAME(D,S) ((D)->s_addr == (S)->s_addr)
+#define IPV4_ADDR_COPY(D,S) ((D)->s_addr = (S)->s_addr)

#define IPV4_NET0(a) ((((u_int32_t) (a)) & 0xff000000) == 0x00000000)
#define IPV4_NET127(a) ((((u_int32_t) (a)) & 0xff000000) == 0x7f000000)
diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 351fb21..03494d1 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -3815,7 +3815,7 @@ ospf_ls_upd_send (struct ospf_neighbor *nbr, struct list *update, int flag)
{
if (flag == OSPF_SEND_PACKET_INDIRECT)
zlog_warn ("* LS-Update is directly sent on NBMA network.");
- if (IPV4_ADDR_SAME(&oi->address->u.prefix4, &p.prefix.s_addr))
+ if (IPV4_ADDR_SAME(&oi->address->u.prefix4, &p.prefix))
zlog_warn ("* LS-Update is sent to myself.");
}

--
1.7.8.3

_______________________________________________
Quagga-dev mailing list
Quagga-dev [at] lists
http://lists.quagga.net/mailman/listinfo/quagga-dev

joakim.tjernlund at transmode

May 7, 2012, 11:40 PM

Post #2 of 9 (321 views)
Permalink

Re: [PATCH 25/25] lib: optimize IPV4_ADDR_[SAME\|COPY]() [In reply to]

>
> From: "Jorge Boncompte [DTI2]" <jorge [at] dti2>
>
> * lib/prefix.c: (prefix_same) changed to use prefix4 field.
> * ospfd/ospf_packet.c: (ospf_ls_upd_send) changed to use prefix4 field.
>
> Now that all callers use the prefix4 field...
>
> * lib/prefix.h: use assignment and comparison instead of memcpy() and
> memcmp(). Avoids function calls. Much faster.
>
> Signed-off-by: Jorge Boncompte [DTI2] <jorge [at] dti2>

You might want to look at my old patch:

http://lists.quagga.net/pipermail/quagga-dev/2009-November/007376.html

Is it a more complete version of this one.

> ---
> lib/prefix.c | 2 +-
> lib/prefix.h | 4 ++--
> ospfd/ospf_packet.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/prefix.c b/lib/prefix.c
> index a3b1adf..48b3f5b 100644
> --- a/lib/prefix.c
> +++ b/lib/prefix.c
> @@ -281,7 +281,7 @@ prefix_same (const struct prefix *p1, const struct prefix *p2)
> if (p1->family == p2->family && p1->prefixlen == p2->prefixlen)
> {
> if (p1->family == AF_INET)
> - if (IPV4_ADDR_SAME (&p1->u.prefix4.s_addr, &p2->u.prefix4.s_addr))
> + if (IPV4_ADDR_SAME (&p1->u.prefix4, &p2->u.prefix4))
> return 1;
> #ifdef HAVE_IPV6
> if (p1->family == AF_INET6 )
> diff --git a/lib/prefix.h b/lib/prefix.h
> index 7f0d360..a97ff24 100644
> --- a/lib/prefix.h
> +++ b/lib/prefix.h
> @@ -106,8 +106,8 @@ struct prefix_rd
> #define IPV4_MAX_BITLEN 32
> #define IPV4_MAX_PREFIXLEN 32
> #define IPV4_ADDR_CMP(D,S) memcmp ((D), (S), IPV4_MAX_BYTELEN)
> -#define IPV4_ADDR_SAME(D,S) (memcmp ((D), (S), IPV4_MAX_BYTELEN) == 0)
> -#define IPV4_ADDR_COPY(D,S) memcpy ((D), (S), IPV4_MAX_BYTELEN)
> +#define IPV4_ADDR_SAME(D,S) ((D)->s_addr == (S)->s_addr)
> +#define IPV4_ADDR_COPY(D,S) ((D)->s_addr = (S)->s_addr)
>
> #define IPV4_NET0(a) ((((u_int32_t) (a)) & 0xff000000) == 0x00000000)
> #define IPV4_NET127(a) ((((u_int32_t) (a)) & 0xff000000) == 0x7f000000)
> diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
> index 351fb21..03494d1 100644
> --- a/ospfd/ospf_packet.c
> +++ b/ospfd/ospf_packet.c
> @@ -3815,7 +3815,7 @@ ospf_ls_upd_send (struct ospf_neighbor *nbr, struct list *update, int flag)
> {
> if (flag == OSPF_SEND_PACKET_INDIRECT)
> zlog_warn ("* LS-Update is directly sent on NBMA network.");
> - if (IPV4_ADDR_SAME(&oi->address->u.prefix4, &p.prefix.s_addr))
> + if (IPV4_ADDR_SAME(&oi->address->u.prefix4, &p.prefix))
> zlog_warn ("* LS-Update is sent to myself.");
> }
>
> --
> 1.7.8.3
>
>
> _______________________________________________
> Quagga-dev mailing list
> Quagga-dev [at] lists
> http://lists.quagga.net/mailman/listinfo/quagga-dev

_______________________________________________
Quagga-dev mailing list
Quagga-dev [at] lists
http://lists.quagga.net/mailman/listinfo/quagga-dev

jorge at dti2

May 8, 2012, 3:14 AM

Post #3 of 9 (321 views)
Permalink

Re: [PATCH 25/25] lib: optimize IPV4_ADDR_[SAME\|COPY]() [In reply to]

El 08/05/2012 8:40, Joakim Tjernlund escribi�:
>
>>
>> From: "Jorge Boncompte [DTI2]" <jorge [at] dti2>
>>
>> * lib/prefix.c: (prefix_same) changed to use prefix4 field.
>> * ospfd/ospf_packet.c: (ospf_ls_upd_send) changed to use prefix4 field.
>>
>> Now that all callers use the prefix4 field...
>>
>> * lib/prefix.h: use assignment and comparison instead of memcpy() and
>> memcmp(). Avoids function calls. Much faster.
>>
>> Signed-off-by: Jorge Boncompte [DTI2] <jorge [at] dti2>
>
> You might want to look at my old patch:
>
> http://lists.quagga.net/pipermail/quagga-dev/2009-November/007376.html

Sorry for missing that. Are you going to resend it?

>
> Is it a more complete version of this one.
>
>> ---
>> lib/prefix.c | 2 +-
>> lib/prefix.h | 4 ++--
>> ospfd/ospf_packet.c | 2 +-
>> 3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/prefix.c b/lib/prefix.c
>> index a3b1adf..48b3f5b 100644
>> --- a/lib/prefix.c
>> +++ b/lib/prefix.c
>> @@ -281,7 +281,7 @@ prefix_same (const struct prefix *p1, const struct prefix *p2)
>> if (p1->family == p2->family && p1->prefixlen == p2->prefixlen)
>> {
>> if (p1->family == AF_INET)
>> - if (IPV4_ADDR_SAME (&p1->u.prefix4.s_addr, &p2->u.prefix4.s_addr))
>> + if (IPV4_ADDR_SAME (&p1->u.prefix4, &p2->u.prefix4))
>> return 1;
>> #ifdef HAVE_IPV6
>> if (p1->family == AF_INET6 )
>> diff --git a/lib/prefix.h b/lib/prefix.h
>> index 7f0d360..a97ff24 100644
>> --- a/lib/prefix.h
>> +++ b/lib/prefix.h
>> @@ -106,8 +106,8 @@ struct prefix_rd
>> #define IPV4_MAX_BITLEN 32
>> #define IPV4_MAX_PREFIXLEN 32
>> #define IPV4_ADDR_CMP(D,S) memcmp ((D), (S), IPV4_MAX_BYTELEN)
>> -#define IPV4_ADDR_SAME(D,S) (memcmp ((D), (S), IPV4_MAX_BYTELEN) == 0)
>> -#define IPV4_ADDR_COPY(D,S) memcpy ((D), (S), IPV4_MAX_BYTELEN)
>> +#define IPV4_ADDR_SAME(D,S) ((D)->s_addr == (S)->s_addr)
>> +#define IPV4_ADDR_COPY(D,S) ((D)->s_addr = (S)->s_addr)
>>
>> #define IPV4_NET0(a) ((((u_int32_t) (a)) & 0xff000000) == 0x00000000)
>> #define IPV4_NET127(a) ((((u_int32_t) (a)) & 0xff000000) == 0x7f000000)
>> diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
>> index 351fb21..03494d1 100644
>> --- a/ospfd/ospf_packet.c
>> +++ b/ospfd/ospf_packet.c
>> @@ -3815,7 +3815,7 @@ ospf_ls_upd_send (struct ospf_neighbor *nbr, struct list *update, int flag)
>> {
>> if (flag == OSPF_SEND_PACKET_INDIRECT)
>> zlog_warn ("* LS-Update is directly sent on NBMA network.");
>> - if (IPV4_ADDR_SAME(&oi->address->u.prefix4, &p.prefix.s_addr))
>> + if (IPV4_ADDR_SAME(&oi->address->u.prefix4, &p.prefix))
>> zlog_warn ("* LS-Update is sent to myself.");
>> }
>>
>> --
>> 1.7.8.3
>>
>>
>> _______________________________________________
>> Quagga-dev mailing list
>> Quagga-dev [at] lists
>> http://lists.quagga.net/mailman/listinfo/quagga-dev
>
> _______________________________________________
> Quagga-dev mailing list
> Quagga-dev [at] lists
> http://lists.quagga.net/mailman/listinfo/quagga-dev
>
>

_______________________________________________
Quagga-dev mailing list
Quagga-dev [at] lists
http://lists.quagga.net/mailman/listinfo/quagga-dev

equinox at diac24

May 8, 2012, 3:55 AM

Post #4 of 9 (320 views)
Permalink

Re: [PATCH 25/25] lib: optimize IPV4_ADDR_[SAME\|COPY]() [In reply to]

On Tue, May 08, 2012 at 08:40:48AM +0200, Joakim Tjernlund wrote:
> > * lib/prefix.c: (prefix_same) changed to use prefix4 field.
> > * ospfd/ospf_packet.c: (ospf_ls_upd_send) changed to use prefix4 field.
> >
> > Now that all callers use the prefix4 field...
> >
> > * lib/prefix.h: use assignment and comparison instead of memcpy() and
> > memcmp(). Avoids function calls. Much faster.
> >
> > Signed-off-by: Jorge Boncompte [DTI2] <jorge [at] dti2>
>
> You might want to look at my old patch:
>
> http://lists.quagga.net/pipermail/quagga-dev/2009-November/007376.html
>
> Is it a more complete version of this one.

Blergh, I'm missing that mail in my archive...

... some perl magic tells me I'm missing more mails *sigh*
6319 -> 6350 31
7351 -> 7393 42

can someone get me copies of those mails? (raw mailbox/maildir copy
preferred, so I can plop them in seamlessly)

-David

Attachments:

signature.asc (0.22 KB)

nick at inex

May 8, 2012, 4:08 AM

Post #5 of 9 (322 views)
Permalink

Re: [PATCH 25/25] lib: optimize IPV4_ADDR_[SAME\|COPY]() [In reply to]

On 08/05/2012 11:55, David Lamparter wrote:
> can someone get me copies of those mails? (raw mailbox/maildir copy
> preferred, so I can plop them in seamlessly)

standard mailman trick:

http://lists.quagga.net/pipermail/quagga-dev.mbox/quagga-dev.mbox

Nick

_______________________________________________
Quagga-dev mailing list
Quagga-dev [at] lists
http://lists.quagga.net/mailman/listinfo/quagga-dev

joakim.tjernlund at transmode

May 8, 2012, 4:10 AM

Post #6 of 9 (323 views)
Permalink

Re: [PATCH 25/25] lib: optimize IPV4_ADDR_[SAME\|COPY]() [In reply to]

"Jorge Boncompte [DTI2]" <jorge [at] dti2> wrote on 2012/05/08 12:14:41:
>
> El 08/05/2012 8:40, Joakim Tjernlund escribi�:
> >
> >>
> >> From: "Jorge Boncompte [DTI2]" <jorge [at] dti2>
> >>
> >> * lib/prefix.c: (prefix_same) changed to use prefix4 field.
> >> * ospfd/ospf_packet.c: (ospf_ls_upd_send) changed to use prefix4 field.
> >>
> >> Now that all callers use the prefix4 field...
> >>
> >> * lib/prefix.h: use assignment and comparison instead of memcpy() and
> >> memcmp(). Avoids function calls. Much faster.
> >>
> >> Signed-off-by: Jorge Boncompte [DTI2] <jorge [at] dti2>
> >
> > You might want to look at my old patch:
> >
> > http://lists.quagga.net/pipermail/quagga-dev/2009-November/007376.html
>
> Sorry for missing that. Are you going to resend it?

No, not for a while anyway. I am very behind mainline Q and I am
not planning to update very soon, to much other stuff going on until fall/winter I guess.

I just point out what I see on the list in case someone wants to pick it up and
do the needed tweaks to match mainline.

Jocke

_______________________________________________
Quagga-dev mailing list
Quagga-dev [at] lists
http://lists.quagga.net/mailman/listinfo/quagga-dev

joakim.tjernlund at transmode

May 9, 2012, 7:47 AM

Post #7 of 9 (323 views)
Permalink

Re: [PATCH 25/25] lib: optimize IPV4_ADDR_[SAME\|COPY]() [In reply to]

David Lamparter <equinox [at] diac24> wrote on 2012/05/08 12:55:44:
>
> On Tue, May 08, 2012 at 08:40:48AM +0200, Joakim Tjernlund wrote:
> > > * lib/prefix.c: (prefix_same) changed to use prefix4 field.
> > > * ospfd/ospf_packet.c: (ospf_ls_upd_send) changed to use prefix4 field.
> > >
> > > Now that all callers use the prefix4 field...
> > >
> > > * lib/prefix.h: use assignment and comparison instead of memcpy() and
> > > memcmp(). Avoids function calls. Much faster.
> > >
> > > Signed-off-by: Jorge Boncompte [DTI2] <jorge [at] dti2>
> >
> > You might want to look at my old patch:
> >
> > http://lists.quagga.net/pipermail/quagga-dev/2009-November/007376.html
> >
> > Is it a more complete version of this one.
>
> Blergh, I'm missing that mail in my archive...
>
> ... some perl magic tells me I'm missing more mails *sigh*
> 6319 -> 6350 31
> 7351 -> 7393 42
>
> can someone get me copies of those mails? (raw mailbox/maildir copy
> preferred, so I can plop them in seamlessly)

David, would be great if you could look at adding this old patch series before
Quagga gets too changed(it is in patchwork too):
http://lists.quagga.net/pipermail/quagga-dev/2009-November/007439.html

I think "ospfd: ospf_hello() should only kick state machine once." is already in though.

Jocke

_______________________________________________
Quagga-dev mailing list
Quagga-dev [at] lists
http://lists.quagga.net/mailman/listinfo/quagga-dev

equinox at diac24

May 22, 2012, 3:29 PM

Post #8 of 9 (248 views)
Permalink

Re: [PATCH 25/25] lib: optimize IPV4_ADDR_[SAME\|COPY]() [In reply to]

On Tue, May 08, 2012 at 08:40:48AM +0200, Joakim Tjernlund wrote:
>
> >
> > From: "Jorge Boncompte [DTI2]" <jorge [at] dti2>
> >
> > * lib/prefix.c: (prefix_same) changed to use prefix4 field.
> > * ospfd/ospf_packet.c: (ospf_ls_upd_send) changed to use prefix4 field.
> >
> > Now that all callers use the prefix4 field...
> >
> > * lib/prefix.h: use assignment and comparison instead of memcpy() and
> > memcmp(). Avoids function calls. Much faster.
> >
> > Signed-off-by: Jorge Boncompte [DTI2] <jorge [at] dti2>
>
> You might want to look at my old patch:
>
> http://lists.quagga.net/pipermail/quagga-dev/2009-November/007376.html
>
> Is it a more complete version of this one.

I'll pick it up after rechecking the BGP remote ID thing. I think the
check is actually 100% wrong and removing it was the correct fix in the
first place, but I'll recheck.

Dropping Jorge's 25/25 for now; remainder inbound on master.

-David

Attachments:

signature.asc (0.22 KB)

equinox at opensourcerouting

May 22, 2012, 3:46 PM

Post #9 of 9 (243 views)
Permalink

Re: [PATCH 25/25] lib: optimize IPV4_ADDR_[SAME\|COPY]() [In reply to]

On Wed, May 23, 2012 at 12:29:23AM +0200, David Lamparter wrote:
> On Tue, May 08, 2012 at 08:40:48AM +0200, Joakim Tjernlund wrote:
> >
> > >
> > > From: "Jorge Boncompte [DTI2]" <jorge [at] dti2>
> > >
> > > * lib/prefix.c: (prefix_same) changed to use prefix4 field.
> > > * ospfd/ospf_packet.c: (ospf_ls_upd_send) changed to use prefix4 field.
> > >
> > > Now that all callers use the prefix4 field...
> > >
> > > * lib/prefix.h: use assignment and comparison instead of memcpy() and
> > > memcmp(). Avoids function calls. Much faster.
> > >
> > > Signed-off-by: Jorge Boncompte [DTI2] <jorge [at] dti2>
> >
> > You might want to look at my old patch:
> >
> > http://lists.quagga.net/pipermail/quagga-dev/2009-November/007376.html
> >
> > Is it a more complete version of this one.
>
> I'll pick it up after rechecking the BGP remote ID thing. I think the
> check is actually 100% wrong and removing it was the correct fix in the
> first place, but I'll recheck.

Actually, that ends up being the idea of "bgpd: don't advertise routes
with peer address as nexthop"; I'll go respin that using peer->nexthop
as I've commented already. After reading RFC4271 5.1.3 I'm confident
that's actually the correct thing to do.

-David

Attachments:

signature.asc (0.22 KB)

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads