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

pkg/bpf: generate ASM output if debug enabled #599

Merged
merged 1 commit into from
Apr 25, 2017

Conversation

ianvernon
Copy link
Member

@ianvernon ianvernon commented Apr 24, 2017

pkg/endpoint: add GetDebug to Owner interface
daemon: regenerate bindata to account for changes in pkg/bpf/join_ep.sh

Add GetDebug to Owner interface, implement in daemon, and utilize this method to determine whether debug mode is enabled to pass in as a parameter to join_ep.sh to generate ASM when BPF is generated.

Fixes #429

Signed-off by: Ian Vernon ian@covalent.io

@ianvernon ianvernon requested review from tgraf and aanm April 24, 2017 23:49
daemon/daemon.go Outdated
@@ -198,6 +198,10 @@ func (d *Daemon) PolicyEnabled() bool {
return d.conf.Opts.IsEnabled(endpoint.OptionPolicy)
}

func (d *Daemon) GetDebug() bool {

Choose a reason for hiding this comment

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

exported method Daemon.GetDebug should have comment or be unexported

daemon/daemon.go Outdated
@@ -198,6 +198,10 @@ func (d *Daemon) PolicyEnabled() bool {
return d.conf.Opts.IsEnabled(endpoint.OptionPolicy)
}

func (d *Daemon) GetDebug() bool {

Choose a reason for hiding this comment

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

exported method Daemon.GetDebug should have comment or be unexported

Copy link
Member

Choose a reason for hiding this comment

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

I would pay more attention to hound if they'd use a puppy as a logo but we should add a comment to the function nevertheless.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do - I'll only add reviewers after I placate the hound in the future :)

daemon/daemon.go Outdated
@@ -198,6 +198,10 @@ func (d *Daemon) PolicyEnabled() bool {
return d.conf.Opts.IsEnabled(endpoint.OptionPolicy)
}

func (d *Daemon) GetDebug() bool {
Copy link
Member

Choose a reason for hiding this comment

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

I would pay more attention to hound if they'd use a puppy as a logo but we should add a comment to the function nevertheless.

Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

@tgraf should we also generate ASM output of bpf_netdev, bpf_lb and bpf_overlay?

daemon/daemon.go Outdated
@@ -198,6 +198,11 @@ func (d *Daemon) PolicyEnabled() bool {
return d.conf.Opts.IsEnabled(endpoint.OptionPolicy)
}

// GetDebug returns whether if debug mode is enabled.
func (d *Daemon) GetDebug() bool {
Copy link
Member

Choose a reason for hiding this comment

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

Why not DebugEnabled()? In consistency with the DryModeEnabled() and TracingEnabled()

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in latest upload.

pkg/endpoint: add DebugEnabled to Owner interface
daemon: regenerate bindata to account for changes in pkg/bpf/join_ep.sh

Add DebugEnabled to Owner interface, implement in daemon, and utilize this method to determine whether debug mode is enabled to pass in as a parameter to join_ep.sh to generate ASM when BPF is generated.

Signed-off by: Ian Vernon <ian@covalent.io>
@tgraf
Copy link
Member

tgraf commented Apr 25, 2017

@tgraf should we also generate ASM output of bpf_netdev, bpf_lb and bpf_overlay?

I think this is fine to get started. There is no code generation for these static programs so one can always invoke clang manually to generate it.

@tgraf tgraf merged commit 026ded3 into master Apr 25, 2017
@tgraf tgraf deleted the 429-bpf-asm-output-debug branch April 25, 2017 09:14
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.

None yet

4 participants