Ongy / netlink-hs

Netlink communication for Haskell
BSD 3-Clause "New" or "Revised" License
2 stars 7 forks source link

Netlink Packet Attributes #7

Open CMCDragonkai opened 5 years ago

CMCDragonkai commented 5 years ago

We are attempting to use this library for communicating with netlink to create a new veth interface.

We have to construct nested packet attributes.

In your API for the packetAttributes, it set to be a map from integers to bytestrings.

What is the reason for doing this, and is there plans to create an API for an easier construction of nested attributes for the Netlink packet?

Furthermore, there's a bunch of constants that are missing from this library, especially things relating to veth interfaces:

-- if_link.h
eIFLA_INFO_DATA :: (Num a) => a
eIFLA_INFO_DATA = 2

-- veth.h
eVETH_INFO_PEER :: (Num a) => a
eVETH_INFO_PEER = 1

The above constants we had to define as well. Would you be intererested in adding extra constants to your package? Note that some of these constants have the same numbers as your existing constants, and your packet pretty printer thingks IFLA_INFO_DATA is `IFLA_BROADCAST?

Ongy commented 5 years ago

The constants are generated with the scrips in the repo, and rather old by now. Feel free to extend them/build and run on a current system, whichever is necessary

The pretty printers are best effort and it's probably picking the wrong subtype.

Map Int BytesString is pretty close to the wire format. Building something more interesting and still generic on top might be hard If you have a good idea how to design a better interface, I'm all ears

CMCDragonkai commented 5 years ago

I'm actually quite confused by the constants in netlink. They are not very well documented. For example the IFLA_INFO_* constants are within their own "enum":

enum {
    IFLA_INFO_UNSPEC,
    IFLA_INFO_KIND,
    IFLA_INFO_DATA,
    IFLA_INFO_XSTATS,
    IFLA_INFO_SLAVE_KIND,
    IFLA_INFO_SLAVE_DATA,
    __IFLA_INFO_MAX,
};

See: https://elixir.bootlin.com/linux/v5.0.7/source/include/uapi/linux/if_link.h

If we were to add this into your constants.h. I think we would have to instead divide them into subgroups. Maybe proper data constructors like:

data IFLA_INFO = IFLA_INFO_UNSPEC | IFLA_INFO_KIND | ...

However then something will need to convert them into numbers. Which can be statically written into your constants.

With regards to a better design. The go netlink library https://github.com/vishvananda/netlink/blob/master/link_linux.go seems the most comprehensive, and I was going to explore it and compare it to the Haskell design.

CMCDragonkai commented 5 years ago

I see your Scripts/Generate.hs. Not sure how to use it but, I suggest adding this for a start:

mkEnum "LinkAttrInfoType" $ enum "^IFLA_INFO_"

This relates to the:

enum {
    IFLA_INFO_UNSPEC,
    IFLA_INFO_KIND,
    IFLA_INFO_DATA,
    IFLA_INFO_XSTATS,
    IFLA_INFO_SLAVE_KIND,
    IFLA_INFO_SLAVE_DATA,
    __IFLA_INFO_MAX,
};

So we would get:

eIFLA_INFO_UNSPEC :: (Num a) => a
eIFLA_INFO_UNSPEC = 0
-- ...
newtype LinkAttrInfoType = LinkAttrInfoType Int deriving (Eq, Enum, Integral, Num, Ord, Real, Show)
-- ...
showLinkAttrInfoType :: (Num a) => (Show a) => (Eq a) => a -> String
showLinkAttrInfoType 0 = "IFLA_INFO_UNSPEC"

These link attr info types are part of the IFLA_LINKINFO attribute. And they represent "nested" attributes of the IFLA_LINKINFO: https://stackoverflow.com/a/31005872/582917

CMCDragonkai commented 5 years ago

Attempting to run this on my own fork: https://github.com/MatrixAI/netlink-hs

You're still using quite an old version of language-c, and the new one has API incompatibility, especially with the position function. So I had to try and bring the older version.

CMCDragonkai commented 5 years ago

So I have successfully run the Generate.hs and I get these changes: https://github.com/MatrixAI/netlink-hs/commit/46b5c83d27d6ed2f07a66c61e05f551eda9119be

I added in:

     mkEnum "LinkAttrInfoType" $ enum "^IFLA_INFO_",

Not sure why there's a bunch of stuff being removed though. Maybe those were generated from header files from a different source? I'm running this on NixOS.

CMCDragonkai commented 5 years ago

I've upgraded the language-c to 0.8.2. by fixing the position call.

This is the newly generated constants: https://github.com/MatrixAI/netlink-hs/blob/4dce0ac1bb6da600815dc27318f0965945ce20aa/System/Linux/Netlink/Constants.hs

All you need is this in Helpers to change to 0.8.2.:

    initPos = position 0 "" 0 0 Nothing

I've also added the VETH_INFO types, not sure if that's the best name for them.