Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

HellPot NextGen #163

Draft
wants to merge 30 commits into
base: main
Choose a base branch
from
Draft

HellPot NextGen #163

wants to merge 30 commits into from

Conversation

yunginnanet
Copy link
Owner

@yunginnanet yunginnanet commented Jun 26, 2024

Overview

I accidentally a whole coca cola bottle HellPot

This really is essentially a rewrite.

  • The entire configuration system has been rewritten
  • The logger has been rewritten
  • The main package (app) has been rewritten

The only thing that remains untouched more or less is the heffalump core.

This should resolve countless idiosyncrasies in configuration behavior.

Improvements

  • Much more random, harder to detect
  • Test cases

New Features

  • Allow regular expressions for matching disallowed user agents in addition to the current strings.Contains match functionality
  • Remote syslog output (tcp/udp) (thank you zerolog)
  • Local syslog output for *nix via unix sockets (thank you zerolog)
  • CLI flags for all configuration values

New features by

are scheduled to hitch a ride, pending further review and testing.

Slimfast

Further slimming of dependencies

Issues resolved

ginger51011 and others added 21 commits June 26, 2024 02:16
…zche

While I do think all bots enjoy Nietzche (who doesn't?), I think we should
take a stance to educate them. What better way than to be able to choose
from any book!

Personal suggestions include:

- The Sorrows of Young Werther by Goethe
- Any political manifesto
- The Declaration of Independence

etc. etc.
This removes globals from `heffalumpt/`, which are hard to reason about,
easy to get wrong, and should not be created if they are never used.

A possible drawbacks is if you would create multiple new defaults,
but this should never be the case.
This prints the short variants (like `-c` for `--config`)
in the help.

Also fixes bug where only `-h` flag works, not `--help`.
Tacking this on to trigger github issue notifs:

Related to pull request GH-162:
closes #140
closes #139
additionally:
  - fix `-c` flag
  - make main package test better
@yunginnanet yunginnanet changed the title Development Rewrite Jun 26, 2024
@yunginnanet
Copy link
Owner Author

yunginnanet commented Jun 26, 2024

Mixing and matching configuration sources seems to be working well.

(Also pictured in screenshot: Grimoire test)

Screenshot

https://tcp.ac/i/Smhva.png

@yunginnanet yunginnanet changed the title Rewrite HellPot NextGen Jun 26, 2024
@yunginnanet yunginnanet marked this pull request as draft June 26, 2024 23:19
break
}
n += copy(p[n:], w3)
n += copy(p[n:], "\n")
if time.Now().UnixNano()%10 == 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't run any profiling, but a quick glance makes me think about how much impact this has (but this is a great idea!). This is a call in a very hot loop so it makes me wonder if we can do this cheaper?

Also it might be a good idea to reorder to have the most common case first, but again measure first make GitHub review comments later might be a better plan...

Copy link
Owner Author

@yunginnanet yunginnanet Jul 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I 100% agree that profiling is necessary to determine the cost here. My initial instinct is that there are few things faster than the the time_now exec for a hot path like this that needs (some level of) entropy. but i agree this definitely needs to be explored and proven. Maybe just a simple i++ counter + modulo would be better.

the relevant (linux runtime) assembly is below, but there are some bitwise operations (in Go) that follow it's execution as well; at least for the UnixNano method.

// Copyright 2021 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//go:build !faketime

#include "go_asm.h"
#include "go_tls.h"
#include "textflag.h"

#define SYS_clock_gettime	228

// func time.now() (sec int64, nsec int32, mono int64)
TEXT time·now<ABIInternal>(SB),NOSPLIT,$16-24
	MOVQ	SP, R12 // Save old SP; R12 unchanged by C code.

	MOVQ	g_m(R14), BX // BX unchanged by C code.

	// Set vdsoPC and vdsoSP for SIGPROF traceback.
	// Save the old values on stack and restore them on exit,
	// so this function is reentrant.
	MOVQ	m_vdsoPC(BX), CX
	MOVQ	m_vdsoSP(BX), DX
	MOVQ	CX, 0(SP)
	MOVQ	DX, 8(SP)

	LEAQ	sec+0(FP), DX
	MOVQ	-8(DX), CX	// Sets CX to function return address.
	MOVQ	CX, m_vdsoPC(BX)
	MOVQ	DX, m_vdsoSP(BX)

	CMPQ	R14, m_curg(BX)	// Only switch if on curg.
	JNE	noswitch

	MOVQ	m_g0(BX), DX
	MOVQ	(g_sched+gobuf_sp)(DX), SP	// Set SP to g0 stack

noswitch:
	SUBQ	$32, SP		// Space for two time results
	ANDQ	$~15, SP	// Align for C code

	MOVL	$0, DI // CLOCK_REALTIME
	LEAQ	16(SP), SI
	MOVQ	runtime·vdsoClockgettimeSym(SB), AX
	CMPQ	AX, $0
	JEQ	fallback
	CALL	AX

	MOVL	$1, DI // CLOCK_MONOTONIC
	LEAQ	0(SP), SI
	MOVQ	runtime·vdsoClockgettimeSym(SB), AX
	CALL	AX

ret:
	MOVQ	16(SP), AX	// realtime sec
	MOVQ	24(SP), DI	// realtime nsec (moved to BX below)
	MOVQ	0(SP), CX	// monotonic sec
	IMULQ	$1000000000, CX
	MOVQ	8(SP), DX	// monotonic nsec

	MOVQ	R12, SP		// Restore real SP

	// Restore vdsoPC, vdsoSP
	// We don't worry about being signaled between the two stores.
	// If we are not in a signal handler, we'll restore vdsoSP to 0,
	// and no one will care about vdsoPC. If we are in a signal handler,
	// we cannot receive another signal.
	MOVQ	8(SP), SI
	MOVQ	SI, m_vdsoSP(BX)
	MOVQ	0(SP), SI
	MOVQ	SI, m_vdsoPC(BX)

	// set result registers; AX is already correct
	MOVQ	DI, BX
	ADDQ	DX, CX
	RET

fallback:
	MOVQ	$SYS_clock_gettime, AX
	SYSCALL

	MOVL	$1, DI // CLOCK_MONOTONIC
	LEAQ	0(SP), SI
	MOVQ	$SYS_clock_gettime, AX
	SYSCALL

	JMP	ret

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we could do is to add some source of entropy at startup (or at the start of each request; one call to a random() is cheap), and the do modulo on that in some way. Any constant would make it a bit too predictable I think

Copy link
Owner Author

@yunginnanet yunginnanet Jul 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, and that is a good idea for sure.

I think we should implement that idea in some form or another but I find myself hesitant to actually pull the trigger until we have a proper measuring apparatus set up so we can even rank these implementation concepts.

That's finding itself higher and higher on the to-do list it seems

@ginger51011
Copy link
Collaborator

Great effort, really nice! Don't have that much time to sit and make a review right now, but fun to see great changes!

@yunginnanet
Copy link
Owner Author

I still find myself really frustrated with the configuration system and ironing out all of the idiosyncrasies, even with Koanf vs Viper.

I've been toying with the idea of just doing the toml ourselves. I've been bashing my head against writing the decoder part of it the last few days, but the encoder works well enough (not that that's the important part though).

If there's anything I've learned while doing software development professionally, it's when to stop trying to use overly complex or just simply annoying dependencies that have been the source of endless bugs.

It usually ends up that the best course of action is to instead just write the functionality yourself. To the untrained it may sound like an unnecessary time sink, but one has to take into account the amount of time/money/unbridled rage caused by the dependencies in question. The scales tend to start tipping in a different direction after such considerations.

I digress, but it's an idea anyway. We'll see how it goes.

os.Exit(2)
}

for defaultKey, defaultVal := range Defaults.val {
Copy link
Owner Author

@yunginnanet yunginnanet Jul 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting (for myself even): I was doing this here because of some problematic behavior when it comes to order of initialization and overrides when it comes to CLI vs defaults vs environment variables vs toml config

dependabot bot and others added 2 commits July 1, 2024 17:34
Bumps [github.com/knadh/koanf/providers/file](https://github.com/knadh/koanf) from 0.1.0 to 1.0.0.
- [Release notes](https://github.com/knadh/koanf/releases)
- [Commits](knadh/koanf@v0.1.0...v1.0.0)

---
updated-dependencies:
- dependency-name: github.com/knadh/koanf/providers/file
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

code for cli help is a dumpster fire HellPot desperately needs test cases
2 participants