search

charybdis-ircd / charybdis

Scalable IRCv3.2 server for large, community-oriented networks
GNU General Public License v2.0
231 stars 102 forks source link

Add extensions/filter (port from ircd-seven) #287

Closed edk0 closed 4 years ago

edk0 commented 4 years ago

I'm not very happy about the pkg-config approach here. Does anyone else want to use their autofu?

aaronmdjones commented 4 years ago

Please try to use PKG_CHECK_MODULES if you can, even better if it includes the header and link-ability test too, as, for example, shown here, and then use your #ifdef HAVE_XXX (see the AC_DEFINE) magic around the #include for said header to fix the build error introduced above.

edk0 commented 4 years ago

I'm not sure how much point there is in trying to build the module if we don't have hyperscan. I suppose it might be useful for sending out SETFILTER messages? But my instinct would be to add the equivalent of fb8d31f34bbc7b7359c0a94b19e968228d3709a2.

aaronmdjones commented 4 years ago

Ah, in that case, only PKG_CHECK_MODULES and a link-test then (so --enable-hyperscan doesn't result in a build-time error if you don't have it, which is necessary since the default is yes) ?

alyx commented 4 years ago

I'm not particularly fond of an approach that requires an AMD64-specific library tbh

edk0 commented 4 years ago

I'm not particularly fond of an approach that requires an AMD64-specific library tbh

I certainly wouldn't want to force anyone to use AMD64, but this already exists, and it's not stopping anyone from adding a cross-platform alternative.

awilfox commented 4 years ago

This isn't even AMD64-specific, it's Intel specific. They specifically mention that they have degraded performance on AMD CPUs.

In addition to not supporting anything other than Intel CPUs, it doesn't seem to work on musl, either. That's "Linux/any CPU/musl", "Linux/ARM/any libc", and "any OS/AMD" unsupported by this library - or roughly half of the platforms noted as "Tier 1" by the Charybdis README.

I don't think something this platform-specific belongs in ircd. If this extension is really needed, it should be rewritten to use something more portable (perhaps PCRE2?).

aaronmdjones commented 4 years ago

The problem with PCRE2 (or any other common regexp library) is that you'd need to match the message against every compiled regexp in series, and the vast majority of messages will be innocuous ones, meaning it's always worst-case (the message doesn't match any of them, but you had to test all of them to find that out).

The primary benefit of Hyperscan (and why this was chosen) is that you can match the message against many compiled regexps simultaneously, with worst-case not significantly-more computationally difficult than matching it against your longest/most-complex regexp. Any other approach simply does not scale.