nDPI icon indicating copy to clipboard operation
nDPI copied to clipboard

Important API changes

Open IvanNardi opened this issue 6 months ago • 12 comments

With the next release, we would like to polish the API. Unfortunately that means that there will be some important API changes.

  1. Remove ndpi_set_protocol_detection_bitmask2(): all protocols are enabled by default (DONE; see https://github.com/ntop/nDPI/commit/70a72f163800dd37dca1ec586ae0a58a6cef8206). ~~If you need to disable some protocols you can use ndpi_init_detection_module_ext(). However it is possible that the API to disable protocols will change again before the next release; sorry...~~ If you need to disable some protocols you can use the usual ndpi_set_config (see 978ca1ba1ab0f9d3f7d3c46e6f80a829b08205db)

  2. We want to to get rid of the defines NDPI_MAX_SUPPORTED_PROTOCOLS and NDPI_MAX_NUM_CUSTOM_PROTOCOLS: the number of protocols should be gotten only at runtime. Fist step in #2857 ~~, but it is still a huge WIP~~ DONE

  3. Support for an unlimited number of (custom) protocols (see https://github.com/ntop/nDPI/issues/2136). Right now we have internal + custom protocols < 512. This change will likely have a significant impact throughout the entire API. ~~Most of the work has been done (see the issue for the list of commits involved), but still WIP~~ DONE

  4. Change the return parameter of ndpi_detection_process_packet(): we want to return also the state of the inspection. The goal is to get rid of ndpi_process_extra_packet() and ndpi_extra_dissection_possible(). TODO

  5. The function ndpi_set_proto_defaults() shouldn't be public anymore (DONE; see 5e54531)

As usual, this is a non-binding list.

If you have any opinions on that, please comment below!

Updates:

  1. Remove NDPI_PROTOCOL_BITMASK (because its size is hardcoded to 512, i.e. max num of protocols). Create a new structure, something like ndpi_bitmask, where the max number of bits is specified at runtime. DONE

  2. Remove NDPI_PROTOCOL_ADULT_CONTENT and NDPI_PROTOCOL_ADS_ANALYTICS_TRACK because they are not real protocol: keep only the two similar categories. DONE

IvanNardi avatar Jun 03 '25 08:06 IvanNardi

Pinging @utoni, @0xA50C1A1, @aouinizied, @vel21ripn, @dsokoloski, @koltiradw, @cardigliano

IvanNardi avatar Jun 03 '25 08:06 IvanNardi

@IvanNardi Will there be dedicated API functions to get the maximum number of protocols?

utoni avatar Jun 03 '25 08:06 utoni

ndpi_get_num_supported_protocols(struct ndpi_detection_module_struct *ndpi_st) has now been replaced with ndpi_get_num_protocols(struct ndpi_detection_module_struct *ndpi_st) that needs to be called after ndpi_finalize_initialization ()

lucaderi avatar Jun 03 '25 08:06 lucaderi

@IvanNardi Will there be dedicated API functions to get the maximum number of protocols?

@utoni, no. The idea is that there no real limits (*); you can only query for the number of actual configured protocols via ndpi_get_num_protocols(). You can use it only after ndpi_finalize_initialization(), because only at that point we know the real/final number of protocols...

If you have any suggestions about that...

Please note that, at the moment, the defines are still there.

(*) The hard limit is ~65535 because protocol ids are u_int16_t....

IvanNardi avatar Jun 03 '25 08:06 IvanNardi

@IvanNardi, great work on the changes!

Quick question - are there plans to add category support for custom protocols? This would need config format tweaks, but it’d be a huge help for me. I’m even down to implement it, but we’d need to align on the approach first.

0xA50C1A1 avatar Jun 03 '25 19:06 0xA50C1A1

Quick question - are there plans to add category support for custom protocols? This would need config format tweaks, but it’d be a huge help for me. I’m even down to implement it, but we’d need to align on the approach first.

@0xA50C1A1, there is a dedicated issue about that: https://github.com/ntop/nDPI/issues/2594#issuecomment-2943201294

IvanNardi avatar Jun 05 '25 08:06 IvanNardi

Updated the first post with points 6 and 7

IvanNardi avatar Jun 05 '25 17:06 IvanNardi

@IvanNardi Apologies for the late reply. I was away until this week.

Just a quick comment to say I think these changes will be very welcome. Two thumbs up over here.

dsokoloski avatar Jun 05 '25 18:06 dsokoloski

Updated the first post with points 6 and 7

I didn't quite get that part. Would a new protocol ID be assigned for each SNI instead, or something like that?

0xA50C1A1 avatar Jun 09 '25 10:06 0xA50C1A1

Updated the first post with points 6 and 7

I didn't quite get that part. Would a new protocol ID be assigned for each SNI instead, or something like that?

No. The idea is not to have a dedicated id for these types of traffic, but only a category; after all, we provide only the generic "type" of traffic (i.e. the category), not a specific application/protocol.

So with a TLS session with SNI "smartadserver.com" you get as classification NDPI_PROTOCOL_TLS, category NDPI_CATEGORY_ADS_ANALYTICS_TRACK (or something like that)

IvanNardi avatar Jun 09 '25 12:06 IvanNardi

Updated the first post with points 6 and 7

I didn't quite get that part. Would a new protocol ID be assigned for each SNI instead, or something like that?

No. The idea is not to have a dedicated id for these types of traffic, but only a category; after all, we provide only the generic "type" of traffic (i.e. the category), not a specific application/protocol.

So with a TLS session with SNI "smartadserver.com" you get as classification NDPI_PROTOCOL_TLS, category NDPI_CATEGORY_ADS_ANALYTICS_TRACK (or something like that)

Got it, thank you for the explanation.

0xA50C1A1 avatar Jun 09 '25 12:06 0xA50C1A1

Updated the first post with latest changes

IvanNardi avatar Jun 23 '25 14:06 IvanNardi

Done everything on this list; ready for the next release.

IvanNardi avatar Nov 03 '25 11:11 IvanNardi