frr icon indicating copy to clipboard operation
frr copied to clipboard

Unable to disable vtysh compilation (compilation from source)

Open jeromeag opened this issue 10 months ago • 10 comments

Description

./configure --disable-vtysh
make

fails with:

  CC       lib/vty.lo
lib/vty.c: In function 'vty_mgmt_resume_response':
lib/vty.c:201:27: error: 'VTYSH_READ' undeclared (first use in this function); did you mean 'VTY_READ'?
  201 |                 vty_event(VTYSH_READ, vty);
      |                           ^~~~~~~~~~
      |                           VTY_READ
lib/vty.c:201:27: note: each undeclared identifier is reported only once for each function it appears in
lib/vty.c: In function 'vty_event_serv':
lib/vty.c:3014:14: error: 'VTYSH_READ' undeclared (first use in this function); did you mean 'VTY_READ'?
 3014 |         case VTYSH_READ:
      |              ^~~~~~~~~~
      |              VTY_READ
lib/vty.c:3015:14: error: 'VTYSH_WRITE' undeclared (first use in this function); did you mean 'VTY_WRITE'?
 3015 |         case VTYSH_WRITE:
      |              ^~~~~~~~~~~
      |              VTY_WRITE
lib/vty.c: In function 'vty_event':
lib/vty.c:3055:14: error: 'VTYSH_SERV' undeclared (first use in this function); did you mean 'VTY_SERV'?
 3055 |         case VTYSH_SERV:
      |              ^~~~~~~~~~
      |              VTY_SERV
lib/vty.c: At top level:
lib/vty.c:95:12: warning: 'vtysh_flush' used but never defined
   95 | static int vtysh_flush(struct vty *vty);
      |            ^~~~~~~~~~~
lib/vty.c:123:13: warning: 'integrate_default' defined but not used [-Wunused-variable]
  123 | static char integrate_default[] = SYSCONFDIR INTEGRATE_DEFAULT_CONFIG;
      |             ^~~~~~~~~~~~~~~~~

Version

commit f26a44f8d

How to reproduce

git clone  https://github.com/FRRouting/frr.git
cd frr
./bootstrap.sh
./configure --disable-vtysh
make

Expected behavior

Succesfull compilation

Actual behavior

Failed compilation

Additional context

No response

Checklist

  • [X] I have searched the open issues for this bug.
  • [X] I have not included sensitive information in this report.

jeromeag avatar Apr 16 '24 08:04 jeromeag

From my perspective vtysh is a required component. We should disable the ability to turn this off. Can you give us an understanding of your actual use case here?

donaldsharp avatar Apr 16 '24 15:04 donaldsharp

For cyber security requirement, we may need to remove every binaries that are not drastically required from the package we deliver. In our environment, we will never use the CLI. So strictly speaking, we do not need the VTY shell.

jeromeag avatar Apr 16 '24 17:04 jeromeag

That's hard to understand (at least for me). If you ever reported a problem, the first things you'd be asked for would be show output, and debug output. At this time, it's pretty hard to imagine operating FRR without vtysh. You might choose not to deliver the vtysh binary with all of your deployments - but it's hard to understand building FRR without cli support.

mjstapp avatar Apr 16 '24 18:04 mjstapp

binaries that are not drastically required from the package we deliver. In our environment, we will never use the CLI. So strictly speaking, we do not need the VTY shell.

vtysh -b is used to load the configuration at startup. It is therefore "drastically"/"strictly speaking" required.

If you are currently using per-daemon configs (/etc/frr/zebra.conf), please refer to https://github.com/FRRouting/frr/blob/master/doc/accords/integrated-config-wins — support for these files is deprecated and will go away at some point.

eqvinox avatar Apr 16 '24 19:04 eqvinox

I just pointed out a compilation configuration issue. I naively thought that the configuration can entirely be loaded by other means (e.g. Netconf).

If this configure option is non sense, one way to fix the issue may be to simply remove the configure option.

jeromeag avatar Apr 16 '24 21:04 jeromeag

Well working with FRR is beyond just loading config. Hence the questions from our side about how do you plan to debug/fix issues on the network when they arrive without vtysh? That is why we consider it integral. Frankly I'm not surprised that the configure option fails to work properly, although I personally am not going to be bothered to submit a fix to disable that command in configure, I would not be opposed to someone else doing the work.

donaldsharp avatar Apr 17 '24 11:04 donaldsharp

It looks like the 'disable-vtysh' is used when cross-compiling, when the clippy tool is being built. That may be a construction of David L's - is it still necessary? or could we make changes to the 'clippy-only' path so that we could remove 'disable-vtysh' ?

mjstapp avatar Apr 17 '24 12:04 mjstapp

It looks like the 'disable-vtysh' is used when cross-compiling, when the clippy tool is being built. That may be a construction of David L's - is it still necessary?

The construct as a whole is still necessary, autoconf can't build a host tool in the same "wash" as target binaries.

or could we make changes to the 'clippy-only' path so that we could remove 'disable-vtysh' ?

This can be done but would mean piling more hacks on top of hacks :( …

eqvinox avatar Apr 21 '24 15:04 eqvinox