Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

htp/table: only fetch element when needed #430

Closed
wants to merge 2 commits into from

Conversation

inashivb
Copy link
Member

It is unnecessary to always fetch the element as it is only needed when there is a match. Get the list item only when the key candidate matches the key parameter and the element needs to be returned. This will ensure lesser calls to htp_list_get fn.

It is unnecessary to always fetch the element as it is only needed when
there is a match. Get the list item only when the key candidate matches
the key parameter and the element needs to be returned. This will ensure
lesser calls to htp_list_get fn.
@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 22052

@catenacyber
Copy link
Contributor

Is there a ticket for this ? A benchmark/profile ?
Should CentOS 7 be replaced with something newer ?

@inashivb
Copy link
Member Author

Is there a ticket for this ?

Nope. Observed this while looking at your report for 7191.

A benchmark/profile ?

None. Just seemed to make logical sense to me. wdyt?

Should CentOS 7 be replaced with something newer ?

Sure. Almalinux? I can only find that we removed CentOS 7 in Suricata, I guess newer versions were already there in the CI there.

@catenacyber
Copy link
Contributor

A benchmark/profile ?

None. Just seemed to make logical sense to me. wdyt?

It makes sense.
But I wonder how the effect can be quantified...

Should CentOS 7 be replaced with something newer ?

Sure. Almalinux? I can only find that we removed CentOS 7 in Suricata, I guess newer versions were already there in the CI there.

@jasonish thoughts ?

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 22056

@jasonish
Copy link
Member

@jasonish thoughts ?

I think we should have AlmaLinux 9 at least. Maybe even 8 as well.

@inashivb
Copy link
Member Author

I think we should have AlmaLinux 9 at least. Maybe even 8 as well.

ok. Thank you! Coming up in next rev

@inashivb inashivb closed this Aug 13, 2024
@inashivb inashivb deleted the htp-table-optimizations/v1 branch August 13, 2024 06:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants