flower_packet:ofp_port/1 type confusion

[lukego]

Howdy!

I think that flower_packet:ofp_port/1 is really not one but two functions: ofp_name_to_port/1 and ofp_port_to_name/1. That is: the caller always knows whether it prefers names or numbers on the return value, but it may not know exactly what type of parameter it is passing in.

Practically speaking: If client code passes in 16#ffffe as a port then flower is going to crash, for example in encode_match(), because it will call ofp_port() which converts 16#fffe to the atom local and then tries to use that as an integer with the bit syntax.

I have changed this locally and it should filter up to Tobbe's branch soon. I wanted to open an issue so that you can say if it sounds like the wrong idea, or otherwise know the reason when the change filters in.

This is the backwards-compatible way that I wrote the change for now:

-spec ofp_port(non_neg_integer()) -> ofp_port_name() | non_neg_integer();
              (ofp_port_name()) -> non_neg_integer().
%% Port numbering.  Physical ports are numbered starting from 1.
ofp_port(P) when is_integer(P) -> ofp_port_to_name(P);
ofp_port(P) when is_atom(P)    -> ofp_name_to_port(P).

ofp_port_to_name(16#fff8) -> in_port;
ofp_port_to_name(16#fff9) -> table;
ofp_port_to_name(16#fffa) -> normal;
ofp_port_to_name(16#fffb) -> flood;
ofp_port_to_name(16#fffc) -> all;
ofp_port_to_name(16#fffd) -> controller;
ofp_port_to_name(16#fffe) -> local;
ofp_port_to_name(16#ffff) -> none;
ofp_port_to_name(X) when is_integer(X) -> X.

ofp_name_to_port(in_port)    -> 16#fff8;
ofp_name_to_port(table)      -> 16#fff9;
ofp_name_to_port(normal)     -> 16#fffa;
ofp_name_to_port(flood)      -> 16#fffb;
ofp_name_to_port(all)        -> 16#fffc;
ofp_name_to_port(controller) -> 16#fffd;
ofp_name_to_port(local)      -> 16#fffe;
ofp_name_to_port(none)       -> 16#ffff;
ofp_name_to_port(X) when is_integer(X) -> X.

and I made all encoding functions use ofp_name_to_port() instead of ofp_port() to guarantee that they get an integer result.