Index | Thread | Search

From:
Tom Smyth <tom.smyth@wirelessconnect.eu>
Subject:
Re: ospf{,6}d: replace inet_aton with inet_pton
To:
tech <tech@openbsd.org>
Date:
Thu, 22 Aug 2024 14:45:19 +0100

Download raw body.

Thread
Just checking some docs,

so Arista and Cisco say area id is a 32 bit value  0-(2^32)-1   expressed
as a number or dotted decimal IP

area 0  = 0.0.0.0   in both these implementations

MikroTik RouterOs Rejects  0   and will only accept an IP address....


I think the dotted decima notation of an IP for an area id  makes sense..



On Wed, 21 Aug 2024 at 12:12, Tom Smyth <tom.smyth@wirelessconnect.eu>
wrote:

> for what it is worth it makes sense to me
>
> all documentation on OSPF in other implementations that I encountered
> stated it was a 32 bit value,  and config examples would always have un
> ambiguous  dotted decimal l notation ike Ipv4 addresses
>
>
> On Wed, 21 Aug 2024 at 12:00, Florian Obser <florian@openbsd.org> wrote:
>
>> For the most part this is a mechanical change.
>>
>> The only functional difference is in the area definition. It now has
>> to be a fully spelled out IP address or decimal number. Hex, oct or
>> truncated IP addresses are no longer accepted.
>>
>> OK?
>>
>> diff --git ospf6d/parse.y ospf6d/parse.y
>> index bbf2ca17b21..d6afef141b6 100644
>> --- ospf6d/parse.y
>> +++ ospf6d/parse.y
>> @@ -211,7 +211,7 @@ varset              : STRING '=' string             {
>>                 ;
>>
>>  conf_main      : ROUTERID STRING {
>> -                       if (!inet_aton($2, &conf->rtr_id)) {
>> +                       if (inet_pton(AF_INET, $2, &conf->rtr_id) != 1) {
>>                                 yyerror("error parsing router-id");
>>                                 free($2);
>>                                 YYERROR;
>> @@ -489,7 +489,7 @@ areaid              : NUMBER {
>>                         $$.s_addr = htonl($1);
>>                 }
>>                 | STRING {
>> -                       if (inet_aton($1, &$$) == 0) {
>> +                       if (inet_pton(AF_INET, $1, &$$) != 1) {
>>                                 yyerror("error parsing area");
>>                                 free($1);
>>                                 YYERROR;
>> diff --git ospfd/database.c ospfd/database.c
>> index 5fec976e819..cc110cc9aee 100644
>> --- ospfd/database.c
>> +++ ospfd/database.c
>> @@ -116,7 +116,7 @@ send_db_description(struct nbr *nbr)
>>
>>         switch (nbr->iface->type) {
>>         case IF_TYPE_POINTOPOINT:
>> -               inet_aton(AllSPFRouters, &dst.sin_addr);
>> +               inet_pton(AF_INET, AllSPFRouters, &dst.sin_addr);
>>                 dd_hdr.iface_mtu = htons(nbr->iface->mtu);
>>                 break;
>>         case IF_TYPE_BROADCAST:
>> diff --git ospfd/hello.c ospfd/hello.c
>> index 5b66a92fd21..46715d96a43 100644
>> --- ospfd/hello.c
>> +++ ospfd/hello.c
>> @@ -48,7 +48,7 @@ send_hello(struct iface *iface)
>>         switch (iface->type) {
>>         case IF_TYPE_POINTOPOINT:
>>         case IF_TYPE_BROADCAST:
>> -               inet_aton(AllSPFRouters, &dst.sin_addr);
>> +               inet_pton(AF_INET, AllSPFRouters, &dst.sin_addr);
>>                 break;
>>         case IF_TYPE_NBMA:
>>         case IF_TYPE_POINTOMULTIPOINT:
>> diff --git ospfd/interface.c ospfd/interface.c
>> index c47268c4a4e..b3a7415c352 100644
>> --- ospfd/interface.c
>> +++ ospfd/interface.c
>> @@ -352,7 +352,7 @@ if_act_start(struct iface *iface)
>>
>>         switch (iface->type) {
>>         case IF_TYPE_POINTOPOINT:
>> -               inet_aton(AllSPFRouters, &addr);
>> +               inet_pton(AF_INET, AllSPFRouters, &addr);
>>                 if (if_join_group(iface, &addr))
>>                         return (-1);
>>                 iface->state = IF_STA_POINTTOPOINT;
>> @@ -366,7 +366,7 @@ if_act_start(struct iface *iface)
>>                     if_type_name(iface->type), iface->name);
>>                 return (-1);
>>         case IF_TYPE_BROADCAST:
>> -               inet_aton(AllSPFRouters, &addr);
>> +               inet_pton(AF_INET, AllSPFRouters, &addr);
>>                 if (if_join_group(iface, &addr))
>>                         return (-1);
>>                 if (iface->priority == 0) {
>> @@ -502,7 +502,7 @@ start:
>>         iface->bdr = bdr;
>>
>>         if (changed) {
>> -               inet_aton(AllDRouters, &addr);
>> +               inet_pton(AF_INET, AllDRouters, &addr);
>>                 if (old_state & IF_STA_DRORBDR &&
>>                     (iface->state & IF_STA_DRORBDR) == 0) {
>>                         if (if_leave_group(iface, &addr))
>> @@ -543,10 +543,10 @@ if_act_reset(struct iface *iface)
>>         case IF_TYPE_POINTOPOINT:
>>         case IF_TYPE_BROADCAST:
>>                 /* try to cleanup */
>> -               inet_aton(AllSPFRouters, &addr);
>> +               inet_pton(AF_INET, AllSPFRouters, &addr);
>>                 if_leave_group(iface, &addr);
>>                 if (iface->state & IF_STA_DRORBDR) {
>> -                       inet_aton(AllDRouters, &addr);
>> +                       inet_pton(AF_INET, AllDRouters, &addr);
>>                         if_leave_group(iface, &addr);
>>                 }
>>                 break;
>> diff --git ospfd/lsack.c ospfd/lsack.c
>> index 78cd67added..08b35cb92c9 100644
>> --- ospfd/lsack.c
>> +++ ospfd/lsack.c
>> @@ -268,14 +268,14 @@ ls_ack_tx_timer(int fd, short event, void *arg)
>>                 /* send LS ack(s) but first set correct destination */
>>                 switch (iface->type) {
>>                 case IF_TYPE_POINTOPOINT:
>> -                       inet_aton(AllSPFRouters, &addr);
>> +                       inet_pton(AF_INET, AllSPFRouters, &addr);
>>                         send_ls_ack(iface, addr, buf);
>>                         break;
>>                 case IF_TYPE_BROADCAST:
>>                         if (iface->state & IF_STA_DRORBDR)
>> -                               inet_aton(AllSPFRouters, &addr);
>> +                               inet_pton(AF_INET, AllSPFRouters, &addr);
>>                         else
>> -                               inet_aton(AllDRouters, &addr);
>> +                               inet_pton(AF_INET, AllDRouters, &addr);
>>                         send_ls_ack(iface, addr, buf);
>>                         break;
>>                 case IF_TYPE_NBMA:
>> diff --git ospfd/lsreq.c ospfd/lsreq.c
>> index 6b513ba369e..d1d7da0e405 100644
>> --- ospfd/lsreq.c
>> +++ ospfd/lsreq.c
>> @@ -45,7 +45,7 @@ send_ls_req(struct nbr *nbr)
>>
>>         switch (nbr->iface->type) {
>>         case IF_TYPE_POINTOPOINT:
>> -               inet_aton(AllSPFRouters, &dst.sin_addr);
>> +               inet_pton(AF_INET, AllSPFRouters, &dst.sin_addr);
>>                 break;
>>         case IF_TYPE_BROADCAST:
>>         case IF_TYPE_NBMA:
>> diff --git ospfd/lsupdate.c ospfd/lsupdate.c
>> index 27415eaecb8..cfaa854c259 100644
>> --- ospfd/lsupdate.c
>> +++ ospfd/lsupdate.c
>> @@ -457,7 +457,7 @@ ls_retrans_timer(int fd, short event, void *bula)
>>                  * because the router switched lately to DR or BDR
>>                  */
>>                 if (le->le_oneshot && nbr->iface->state & IF_STA_DRORBDR)
>> -                       inet_aton(AllSPFRouters, &addr);
>> +                       inet_pton(AF_INET, AllSPFRouters, &addr);
>>                 else if (nbr->iface->state & IF_STA_DRORBDR) {
>>                         /*
>>                          * old retransmission needs to be converted into
>> @@ -471,7 +471,7 @@ ls_retrans_timer(int fd, short event, void *bula)
>>                 } else if (nbr->iface->type == IF_TYPE_POINTOPOINT)
>>                         memcpy(&addr, &nbr->addr, sizeof(addr));
>>                 else
>> -                       inet_aton(AllDRouters, &addr);
>> +                       inet_pton(AF_INET, AllDRouters, &addr);
>>         } else
>>                 memcpy(&addr, &nbr->addr, sizeof(addr));
>>
>> diff --git ospfd/packet.c ospfd/packet.c
>> index 33372711009..c3b300b8b98 100644
>> --- ospfd/packet.c
>> +++ ospfd/packet.c
>> @@ -184,9 +184,9 @@ recv_packet(int fd, short event, void *bula)
>>          * or to the address of the interface itself.
>>          * AllDRouters is only valid for DR and BDR but this is checked
>> later.
>>          */
>> -       inet_aton(AllSPFRouters, &addr);
>> +       inet_pton(AF_INET, AllSPFRouters, &addr);
>>         if (ip_hdr.ip_dst.s_addr != addr.s_addr) {
>> -               inet_aton(AllDRouters, &addr);
>> +               inet_pton(AF_INET, AllDRouters, &addr);
>>                 if (ip_hdr.ip_dst.s_addr != addr.s_addr) {
>>                         if (ip_hdr.ip_dst.s_addr != iface->addr.s_addr) {
>>                                 log_debug("recv_packet: packet sent to
>> wrong "
>> @@ -230,7 +230,7 @@ recv_packet(int fd, short event, void *bula)
>>         /* switch OSPF packet type */
>>         switch (ospf_hdr->type) {
>>         case PACKET_TYPE_HELLO:
>> -               inet_aton(AllSPFRouters, &addr);
>> +               inet_pton(AF_INET, AllSPFRouters, &addr);
>>                 if (iface->type == IF_TYPE_BROADCAST ||
>>                     iface->type == IF_TYPE_POINTOPOINT)
>>                         if (ip_hdr.ip_dst.s_addr != addr.s_addr) {
>> @@ -313,8 +313,7 @@ ospf_hdr_sanity_check(const struct ip *ip_hdr, struct
>> ospf_hdr *ospf_hdr,
>>         }
>>
>>         if (iface->type == IF_TYPE_BROADCAST || iface->type ==
>> IF_TYPE_NBMA) {
>> -               if (inet_aton(AllDRouters, &addr) == 0)
>> -                       fatalx("recv_packet: inet_aton");
>> +               inet_pton(AF_INET, AllDRouters, &addr);
>>                 if (ip_hdr->ip_dst.s_addr == addr.s_addr &&
>>                     (iface->state & IF_STA_DRORBDR) == 0) {
>>                         log_debug("recv_packet: invalid destination IP in
>> "
>> diff --git ospfd/parse.y ospfd/parse.y
>> index 82f51bfdd44..10498812688 100644
>> --- ospfd/parse.y
>> +++ ospfd/parse.y
>> @@ -225,7 +225,7 @@ varset              : STRING '=' string             {
>>                 ;
>>
>>  conf_main      : ROUTERID STRING {
>> -                       if (!inet_aton($2, &conf->rtr_id)) {
>> +                       if (inet_pton(AF_INET, $2, &conf->rtr_id) != 1) {
>>                                 yyerror("error parsing router-id");
>>                                 free($2);
>>                                 YYERROR;
>> @@ -620,7 +620,7 @@ areaid              : NUMBER {
>>                         $$.s_addr = htonl($1);
>>                 }
>>                 | STRING {
>> -                       if (inet_aton($1, &$$) == 0) {
>> +               if (inet_pton(AF_INET, $1, &$$) != 1) {
>>                                 yyerror("error parsing area");
>>                                 free($1);
>>                                 YYERROR;
>> @@ -683,7 +683,7 @@ interface   : INTERFACE STRING      {
>>                         s = strchr($2, ':');
>>                         if (s) {
>>                                 *s++ = '\0';
>> -                               if (inet_aton(s, &addr) == 0) {
>> +                               if (inet_pton(AF_INET, s, &addr) != 1) {
>>                                         yyerror(
>>                                             "error parsing interface
>> address");
>>                                         free($2);
>>
>> --
>> In my defence, I have been left unsupervised.
>>
>>
>
> --
> Kindest regards,
> Tom Smyth.
>


-- 
Kindest regards,
Tom Smyth.