Code Monkey home page Code Monkey logo

Comments (2)

edwarnicke avatar edwarnicke commented on July 20, 2024

Generally I like trying to move in this direction. I have a few things in my own mind that I have yet to sort out wrt this approach:

  1. This changes the existing Want* api in a breaking way. This is a double edged sword in that it signals folks to fix their usage, but we should be clear up front about the change.
  2. EnableDisable across many users. This message sends the Want* Message every time. This could very easily allow independent users in code in the same process to stomp each other, with one Disabling what another wants enabled.
  3. How do we handle legacy channel senders of Want* messages wrt 2 here?

I think at the end of the day we need to make sure that Connection is handling the 'Wanted' state of events correctly, only Disabling with VPP in the event there is no longer any consumption of the events going on in the process.

from govpp.

ondrej-fabry avatar ondrej-fabry commented on July 20, 2024
1. This changes the existing Want* api in a breaking way.  This is a double edged sword in that it signals folks to _fix_ their usage, but we should be clear up front about the change.

This would only break users of generated RPC services which is kind of in experimental mode since it uses Stream API. However I agree that this will be mentioned in the first section of changelog under Breaking Changes.

2. EnableDisable across many users.  This message sends the Want* Message every time.  This could very easily allow independent users in code in the same process to stomp each other, with one Disabling what another wants enabled.

Yes, this is like the

3. How do we handle legacy channel senders of Want* messages wrt 2 here?

I do not think we should invest too much time into making Channel users happy. We can add some warning where suitable, but I the usage of generated RPC service together with usage of Channel is not intended to be mixed together. And if that is a case, user should take precautions.

I think at the end of the day we need to make sure that Connection is handling the 'Wanted' state of events correctly, only Disabling with VPP in the event there is no longer any consumption of the events going on in the process.

This will be the most difficult part to handle reliably in all situations.

HOW to enable/disable events

To handle the enabling and disabling of events in VPP we first need to either:

  • detect which action (enable or disable) user wants to do
  • or take control over the action and handle it for user.

Both of these scenarios have their shortcomings though. The VPP API, as usual, is not consistent at all here. See list of event messages and their RPC requests that control them:

    EVENTS MESSAGES:
        [
        	  "bfd_udp_session_event",
        	  "sw_interface_event",
        	  "ip6_ra_event",
        	  "ip_neighbor_event",
        	  "ip_neighbor_event_v2",
        	  "l2_macs_event",
        	  "l2_arp_term_event",
        	  "dhcp6_reply_event",
        	  "dhcp_compl_event",
        	  "dhcp6_pd_reply_event",
        	  "igmp_event",
        	  "nat_ha_resync_completed_event",
        	  "nat44_ei_ha_resync_completed_event",
        	  "vrrp_vr_event",
        	  "wireguard_peer_event"
        	]
    WANTS REQUESTS:
        {
        	  "dhcp_client_config": [
        	    "is_add (bool)",
        	    "client (vl_api_dhcp_client_t)"
        	  ],
        	  "nat44_ei_ha_resync": [
        	    "want_resync_event (u8)",
        	    "pid (u32)"
        	  ],
        	  "nat_ha_resync": [
        	    "want_resync_event (u8)",
        	    "pid (u32)"
        	  ],
        	  "want_bfd_events": [
        	    "enable_disable (bool)",
        	    "pid (u32)"
        	  ],
        	  "want_dhcp6_pd_reply_events": [
        	    "enable_disable (bool)",
        	    "pid (u32)"
        	  ],
        	  "want_dhcp6_reply_events": [
        	    "enable_disable (u8)",
        	    "pid (u32)"
        	  ],
        	  "want_igmp_events": [
        	    "enable (u32)",
        	    "pid (u32)"
        	  ],
        	  "want_interface_events": [
        	    "enable_disable (u32)",
        	    "pid (u32)"
        	  ],
        	  "want_ip6_ra_events": [
        	    "enable (bool)",
        	    "pid (u32)"
        	  ],
        	  "want_ip_neighbor_events": [
        	    "enable (bool)",
        	    "pid (u32)",
        	    "ip (vl_api_address_t)",
        	    "sw_if_index (vl_api_interface_index_t)"
        	  ],
        	  "want_ip_neighbor_events_v2": [
        	    "enable (bool)",
        	    "pid (u32)",
        	    "ip (vl_api_address_t)",
        	    "sw_if_index (vl_api_interface_index_t)"
        	  ],
        	  "want_l2_arp_term_events": [
        	    "enable (bool)",
        	    "pid (u32)"
        	  ],
        	  "want_l2_macs_events": [
        	    "learn_limit (u32)",
        	    "scan_delay (u8)",
        	    "max_macs_in_event (u8)",
        	    "enable_disable (bool)",
        	    "pid (u32)"
        	  ],
        	  "want_vrrp_vr_events": [
        	    "enable_disable (bool)",
        	    "pid (u32)"
        	  ],
        	  "want_wireguard_peer_events": [
        	    "sw_if_index (vl_api_interface_index_t)",
        	    "peer_index (u32)",
        	    "enable_disable (u32)",
        	    "pid (u32)"
        	  ]

There are several different names/types for the boolean field.

  • enable (u32/bool)
  • enable_disable (u8/u32/bool)
  • want_XXX_event (u8)
  • and special cases where this field is nested - dhcp_client_config has it under client.want_dhcp_event (dhcp_client type)

On the last community meeting we agreed that I will open discussion on vpp-dev about this so the VPP API is cleaned up and ideally some rules are specified for this including step in the CI pipeline to encorce this for future development. Until then, I guess we could handle the cases mentioned above by hard-coding or checking for possible variations.

WHEN to enable/disable events

The events in the VPP can be enabled/disabled globally and they are received with context=0 so the destination is the connection (or process) itself. This means to allow multiple watchers for single event we will need to keep reference counter for watchers where we send WantXXX request with Enable=true when counter gets from 0->1 and send WantXXX request with Enable=false when it reaches 0 again.

The problems I see with this are:

  1. keeping reference counter in the connection itself and handling it for all WantXXX requests sent including Channel/Stream and generated RPC client (Stream too) it going a bit against the reasons behind the Stream API where the goal is too avoid handling VPP API semantics - also it might confuse users if their request is not really sent when they clearly send it because of some reference counter not reaching 0 yet.
  2. the special cases of the VPP events are possibly more tricky since the requests seem to have filter/target for which they want to enable events - e.g. DHCP events are enabled per client (have not tested this though, just assuming because the field is part of client config) and others like IP neighbor events and Wireguard peer events.. this would get quickly very complex to handle if we needed to keep reference counter for everything...

from govpp.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.