Zeek and the myricom package plugin

Development
1

Background: We like to run ‘master’, but with the name change it broke too many things and we had to stick to 2.6.2 for the time being. Since then, I’ve started trying to convert our ansible scripts and prepare to make the jump. Today I discovered the bro-myricom package would fail to build.

Has anyone attempted this yet? I can get it to build with a couple minor changes:

src/Myricom.cc

@@ -1,4 +1,4 @@
#include “bro-config.h”
#include “zeek-config.h”

tests/Scripts/get-bro-env

@@ -8,7 +8,7 @@ bro=cat ${base}/../../build/CMakeCache.txt | grep BRO_DIST | cut -d = -f 2
if [ “$1” = “brobase” ]; then
echo ${bro}
elif [ “$1” = “bropath” ]; then
${bro}/build/bro-path-dev
${bro}/build/zeek-path-dev

(These are part of https://github.com/dopheide-esnet/zeek-myricom)

Unfortunately, we end up with another problem:
zeek -N
internal error in /home/zeek/zeek-myricom/build/scripts/./init.bro, line 37: bad reference count [0]

Line 37 is just SNF_RSS_IP:
const snf_rss_mode: set[RssField] = {
SNF_RSS_IP,
SNF_RSS_SRC_PORT,
SNF_RSS_DST_PORT
} &redef;

Clearly I’m not gonna get lucky with an easy fix. Doing a simple search/replace for bro/zeek across the whole tree doesn’t seem viable as things like bro-bif.h haven’t changed names. Has anyone started digging into how plugin packages will need to be updated?

Thanks,
-Dop

2

Background: We like to run ‘master’, but with the name change it broke too many things and we had to stick to 2.6.2 for the time being. Since then, I’ve started trying to convert our ansible scripts and prepare to make the jump. Today I discovered the bro-myricom package would fail to build.

Has anyone attempted this yet? I can get it to build with a couple minor changes:

src/Myricom.cc

@@ -1,4 +1,4 @@
#include “bro-config.h”
#include “zeek-config.h”

Can you give more info on how to reproduce this one? The master branch should currently be installing “zeek-config.h” along with a symlink to it named “bro-config.h”, with the ideal being that people wouldn’t have to make this change.

IIRC, since we changed our default install prefix from /usr/local/bro to /usr/local/zeek, there’s also a bit different logic if we find someone is going to install over an old Bro installation that was still at the old prefix, so might be good to know if you’re installing fresh or upgrading from the old version.

tests/Scripts/get-bro-env

@@ -8,7 +8,7 @@ bro=cat ${base}/../../build/CMakeCache.txt | grep BRO_DIST | cut -d = -f 2
if [ "1" = "brobase" ]; then echo {bro}
elif [ "1" = "bropath" ]; then {bro}/build/bro-path-dev
${bro}/build/zeek-path-dev

This one just looks like an oversight on our end, we’ll need to keep creating (or symlinking) that “build/bro-path-dev” file.

Unfortunately, we end up with another problem:
zeek -N
internal error in /home/zeek/zeek-myricom/build/scripts/./init.bro, line 37: bad reference count [0]

Line 37 is just SNF_RSS_IP:
const snf_rss_mode: set[RssField] = {
SNF_RSS_IP,
SNF_RSS_SRC_PORT,
SNF_RSS_DST_PORT
} &redef;

This doesn’t look related to the Bro-Zeek renaming, but possibly some enum optimizations we did that are being tickled by what this plugin is doing. Particularly there’s an enum being declared/defined twice:

https://github.com/sethhall/bro-myricom/blob/89815d89e0ba6957149521cf99e608c0dc909f6d/src/myricom.bif#L9-L15

and

https://github.com/sethhall/bro-myricom/blob/89815d89e0ba6957149521cf99e608c0dc909f6d/scripts/init.bro#L26-L32

Possibly old versions of Bro were fine with that happening, but not anymore. Generally seems odd that we don’t explicitly give an error message to indicate the same enum being defined in two separate places.

I’ll look more into what the proper fix is next week, but if you’re just looking to try to get something working in the meantime, a workaround may be to comment out or remove the entire RssField enum definition inside the init.bro script.

Clearly I’m not gonna get lucky with an easy fix. Doing a simple search/replace for bro/zeek across the whole tree doesn’t seem viable as things like bro-bif.h haven’t changed names. Has anyone started digging into how plugin packages will need to be updated?

Generally the idea is to be mostly backward compatible so people aren’t forced to make any changes to external plugins, but looks like there’s a couple small things we overlooked or had not tested that we’ll want to fix before the 3.0 release, so thanks for the early feedback.

  • Jon
3

We just migrated to master with the name change and the afpacket plugin, so I know the Zeek code is OK.

4

Can you give more info on how to reproduce this one? The master branch should currently be installing “zeek-config.h” along with a symlink to it named “bro-config.h”, with the ideal being that people wouldn’t have to make this change.

IIRC, since we changed our default install prefix from /usr/local/bro to /usr/local/zeek, there’s also a bit different logic if we find someone is going to install over an old Bro installation that was still at the old prefix, so might be good to know if you’re installing fresh or upgrading from the old version.

This is a fresh install. bro-config.h is a symlink in the installed directory, but I think the issue is that the plugin is building against bro-dist, the source tree and [bro-dist]/build/bro-config.h doesn’t exist. I can verify this by making that symlink exist.

This one just looks like an oversight on our end, we’ll need to keep creating (or symlinking) that “build/bro-path-dev” file.

Right on.

This doesn’t look related to the Bro-Zeek renaming, but possibly some enum optimizations we did that are being tickled by what this plugin is doing. Particularly there’s an enum being declared/defined twice:

https://github.com/sethhall/bro-myricom/blob/89815d89e0ba6957149521cf99e608c0dc909f6d/src/myricom.bif#L9-L15

and

https://github.com/sethhall/bro-myricom/blob/89815d89e0ba6957149521cf99e608c0dc909f6d/scripts/init.bro#L26-L32

Possibly old versions of Bro were fine with that happening, but not anymore. Generally seems odd that we don’t explicitly give an error message to indicate the same enum being defined in two separate places.

I’ll look more into what the proper fix is next week, but if you’re just looking to try to get something working in the meantime, a workaround may be to comment out or remove the entire RssField enum definition inside the init.bro script.

Might give that shot depending how the weekend goes. Not in a rush, so we can catch up next week.

Generally the idea is to be mostly backward compatible so people aren’t forced to make any changes to external plugins, but looks like there’s a couple small things we overlooked or had not tested that we’ll want to fix before the 3.0 release, so thanks for the early feedback.

Wait, are we the only ones that try to run ‘master’ in production? :slight_smile:

Side conversation, what are people’s thoughts on how to handle package transition to zeek? Using bro-myricom as an example, I made a zeek-myricom just to help me keep in clear in my head. Repos could just be renamed, but I can see situations where that breaks auto-update procedures without some hand holding. I haven’t looked too closely to see if zkg supports both bro-pkg and zkg in the ‘depends’ field for meta data.

-Dop

5

So https://github.com/J-Gras/bro-af_packet-plugin/issues/11 isn't an issue anymore due to backwards compatible symlinks?

6

Speaking of that, you made this commit:

https://github.com/J-Gras/bro-af_packet-plugin/commit/5a5d8bb74f162841111b26880137f51683e60ac1

which has the new changes(from the skeleton?) that allows it to be built without the full source tree, but the myricom package is still using the old cmake bits.

7

Updating myricom to build against the install tree looks like it’s going to work and will be much cleaner.

-Dop

8

* It's up to the package maintainer whether they want to rename -- we
can't force them.

* For packages maintained on GitHub (effectively all of them), when a
person renames a repository, GitHub automatically allows accessing the
repo via the old name/URL as a redirect to the new URL. So people who
had installed with bro-pkg/zkg using the old URL should still be fine
going forward.

* The one thing that may break is if users had done an explicit `@load
bro-myricom` somewhere, so I've provided an `aliases` metadata field
for package maintainers to add in order to still support loading via
old names:

    https://docs.zeek.org/projects/package-manager/en/stable/package.html#aliases-field

* zkg does recognize either "zkg" or "bro-pkg" in the `depends` field.
Also "zeek" or "bro" are accepted. However, the new "zeek" and "zkg"
names aren't recognized until zkg v2.0, so if the goal is to still
support users of bro-pkg, you'd still need to use "bro" and "bro-pkg".
Full docs:

    https://docs.zeek.org/projects/package-manager/en/stable/package.html#depends-field

- Jon

9

I'll take a look at it.

   .Seth

10

Seth,

github.com/dopheide-esnet/zeek-myricom contains Jan’s changes as well as removes the enum duplicate if you want to steal those.

Dop

11

Ah, thanks! I'll pull from there.

  .Seth

12

Does this branch look crazy to you at all?

My plan is to rename the whole project to zeek-myricom and set the alias as suggested by Jon once I have some external validation that this branch works for more people than just me. :slight_smile:

   .Seth

13

It builds and works for me against master. Should we change the bro-pkg/zkg requirement to > 2.0? It should be backwards compatible only as far as when all the required includes, etc were included in the install tree. Not sure how far back that change was made.

-Dop

14

It builds and works for me against master. Should we change the bro-pkg/zkg requirement to > 2.0?

At a glance, I only see a "bro-pkg >=1.5.0" requirement due to use of
`aliases` metadata field.

It should be backwards compatible only as far as when all the required includes, etc were included in the install tree. Not sure how far back that change was made.

Bro 2.6 is when headers started to get installed such that packages
could be independent from the source-tree, so adding a "bro >=2.6.0"
requirement makes sense to me.

- Jon

15

Ok, I updated the dependencies listed bro-pkg.cfg to get the correct bro-pkg and bro.

Are there suggestions about how I should merge this branch into master and not break people installing this on current releases?

   .Seth

16

Best would be to actually test against 2.6 and master to see if anything breaks.

At a minimum, actually I see these probably need to change to be compatible:

* get-bro-env: if "zeek-path-dev" doesn't exist fallback to "bro-path-dev"

* broctl/myricom.py: may need a try block to see if the ZeekControl
stuff exists, else still use BroControl

- Jon

17

Nope. I can count maybe weeks when Mozilla was running a stable release in production - and we’ve been rocking this boat since like 7 years now?

And now I feel old.

18

More testing revealed another gotcha against master…

CMake is expecting to be able to copy myricom.bif.bro to it’s destination, but only myricom.bif.zeek exists.

make
…
[ 11%] [BIFCL] Processing src/myricom.bif
Error copying file “myricom.bif.bro” to “/home/zeek/.zkg/testing/zeek-myricom/clones/zeek-myricom/build/lib/bif/myricom.bif.bro”.

Not sure if that’s just a problem on my end or more fundamental to the name change with plugins.

-Dop

19

* The thing which produces the .bif.bro versus .bif.zeek file is
`bifcl`. Seems like you've got the new version since it made a
.bif.zeek

* The thing which generates a Makefile that expects to copy .bif.bro
versus .bif.zeek is cmake/BifCl.cmake. Seems you may have an old
version of this since it created a Makefile that still expects a
.bif.bro

I didn't test building the myricom plugin, but a simple plugin
containing a .bif doesn't seem to generally have this problem when
using a fully updated zeek/master: it produced and installed a
.bif.zeek file.

Maybe verify you've got a fully updated and consistent Zeek build
and/or give the full command and build log you're running. The
configuration output from the build log could have a hint that it's
picking up an unexpected Bro/Zeek build or install path.

If installing via zkg, an outdated "zeek_dist" in ~/.zkg/config is a
problem if it's still using a version of the myricom package that uses
it, else if there's an old version of "bro-config" (now "zeek-config")
in PATH, that could be a problem.

- Jon

20

I tested it today and was able to build the myricom plugin without any problems.

It was a clean system, with the fresh copy of Zeek master and a clone of the plugin from Seth repo.

That was without using zkg mind you.