Skip to content

Instantly share code, notes, and snippets.

@codeout
Last active September 4, 2017 04:07
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save codeout/98ffbb78ebd237ad3e013229d3abb65d to your computer and use it in GitHub Desktop.
Save codeout/98ffbb78ebd237ad3e013229d3abb65d to your computer and use it in GitHub Desktop.
NETWAYS/sflow のバグたち

既存の NETWAYS/sflow パーサーに大きめと思われるバグがあり、ユーザーの手元ではデータが壊れていると思われます。

バグは取りつつ互換にするのが理想ではありますが、テストを書きつつトランスレーターを実装するのがまあまあ手間で迷っています。

NETWAYS/sflow のバグたち

バグ1: 複数flow sample が考慮されていない

  • 1 sflow export 中には複数flow sample 格納される
  • 1 flow sample 中には複数flow record 格納される

https://github.com/NETWAYS/sflow/blob/0456dd0d2d41dec48596329d64a2f7573e0c8fdf/lib/sflow/parsers/parsers.rb#L9-L48

☝️ ループの中で @sflow を上書きしているため、 限られた実装 (1 sflow export packet 中に含まれるのが1 flow sample の場合) でないと誤動作しそうです。

ブログに書かれている、

2017-03-24 18:52:52.519715000 +0900 example.sflow: {"agent_address":null,"sampling_rate":"400","i_iface_value":0,"o_iface_value":0,"ipv4_src":"199.59.148.241","ipv4_dst":"192.168.10.17","udp_src_port":1900,"udp_dst_port":57347,"frame_length":1486,"frame_length_multiplied":594400,"tcp_src_port":443,"tcp_dst_port":58076}

http://enukane.github.io/blog/2017/03/24/fluent-plugin-sflow-release/

udp_src_port, udp_dst_port, tcp_src_port, tcp_dst_port が混在しているのは このバグの影響と思われます。 共通のフィールドは、より後ろのflow sample のもので上書きされているはずです。

バグ2: Ethernet interface counters を記録していない

こちらは軽微で、export される情報が不足します。バグ1のようにデータが壊れたりはしないようです。 ( 調査した範囲では、1 flow export 中に複数counters sample が入る実装はなさそうだった )

https://github.com/NETWAYS/sflow/blob/0456dd0d2d41dec48596329d64a2f7573e0c8fdf/lib/sflow/parsers/parsers.rb#L58-L60

問題

互換性をもたせるにあたり、バグまで再現するかどうか。

  • 選択肢1: バグまで再現して、完全に互換動作にする
  • 選択肢2: バグをなおした形で、フィールド名をそろえる
  • 選択肢3: 互換性はあきらめて、フィールド名を変えてしまう

既存ユーザーのデータは、(おそらくですが) バグ1によって壊れていると思われます。 恐縮ながら、動作が変わるという意味で 選択肢2 も3 も大差ないのでは…という気がして参りました。

個人的には 選択肢3 でどうでしょう?という気持ちですが、アドバイス頂ければそれに従います。 選択肢2 も必要ならばやってみようと思います。

アドバイスお願いします 🙇

@enukane
Copy link

enukane commented Sep 4, 2017

選択肢3でよいと思います。

現状壊れており真っ当に使えてるユーザはほぼいないと思われるため、これを機にあるべき形に修正するのがベストだと思います。
この際、非互換は正常化で相殺される程度のものだと思うので一気にやってしまえばよいのではないでしょうか。
(ダウンロード数は、gemチェック用のボット等で上乗せされている場合もあるためこの程度なら考慮しなくてもよいかと)
なので選択肢1, 2をあえて取る必要もないと思います。

併せて大幅に書き換わると思われるので flent-plugin-sflow 自体の移転も進めたく

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment