[PATCH] BGP: Add support for BGP hostname capability

[Ondrej Zajicek]

On Wed, Feb 03, 2021 at 07:19:50PM +0100, Vincent Bernat wrote:
> This is an implementation of draft-walton-bgp-hostname-capability-02.
> It's implemented since quite some time for FRR and in datacenter, this
> gives a nice output to avoid using IP addresses.
> 
> Currently, it is always enabled. The hostname is retrieved from
> uname(2) and not configurable. The domain name is never set nor
> displayed. I don't think being able to set the hostname is very
> important. I don't think the domain name is useful. However, maybe the
> capability should not be enabled by default.

Hi

Thanks for the patch. I like this feature, it is simple, i would merge
it. I have several comments:

1) Hostname has similar role like router id. It seems to me that there
should be global hostname config property, like config->router_id,
initialized like router_id in global_commit().

2) Consequently, there should be global option to set it, and it should
report in 'show status'.

3) Function calling uname() should be probably somewhere in sysdep/unix/ code

4) The hostname field in bgp_caps should be ptr, allocated based on
length, instead of preallocatted full-length buffer.

5) As Job Snijders wrote, the capability should be disabled by default.

6) Received hostname should be treated for unsafe characters like text
from RFC 8203 (shutdown communication), see bgp_handle_message().


> Also, the case where the capability is supported but hostname is empty
> is handled as if the capability was not supported. I think this is not
> worth handling such an edge case. The RFC doesn't say if hostname is
> optional or not.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santiago at crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."