Skip to content
This repository has been archived by the owner on Jun 24, 2021. It is now read-only.

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

Merged
merged 25 commits into from
Oct 22, 2019

Conversation

edk0
Copy link
Contributor

@edk0 edk0 commented Oct 5, 2019

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

@aaronmdjones
Copy link
Contributor

aaronmdjones commented Oct 5, 2019

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
Copy link
Contributor Author

edk0 commented Oct 5, 2019

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 fb8d31f.

@aaronmdjones
Copy link
Contributor

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
Copy link

alyx commented Oct 6, 2019

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

@edk0
Copy link
Contributor Author

edk0 commented Oct 6, 2019

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
Copy link
Contributor

awilfox commented Oct 6, 2019

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
Copy link
Contributor

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.

@aaronmdjones aaronmdjones merged commit 58a7048 into charybdis-ircd:master Oct 22, 2019
@edk0 edk0 deleted the filter branch January 3, 2020 00:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants