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: santi...@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."
