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

Hostif Generic Netlink interface #936

Merged
merged 3 commits into from
Jul 11, 2019

Conversation

padmanarayana
Copy link
Contributor

A netdev tunnel channel is proposed where an application can create a virtual tunnel interface (over a loopback) to receive trapped packets/buffers from the SAI driver. This can be used as an alternative to callback/FD based mechanisms.

inc/saihostif.h Outdated
*/
typedef enum _sai_hostif_channel_encap_type_t
{
/** Receive packets via GRE/ERSPAN Type 2*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just call it SAI_HOSTIF_TABLE_CHANNEL_ENCAP_TYPE_GRE and add one for IPinIP, then it covers all tunnels:

SAI_TUNNEL_TYPE_IPINIP,
SAI_TUNNEL_TYPE_IPINIP_GRE,
SAI_TUNNEL_TYPE_VXLAN,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a tunnel to lift packets from the SAI driver to applications. We do not plan to leverage the SAI_TUNNEL infra for this..

inc/saihostif.h Outdated

/** End of custom range base */
SAI_HOSTIF_TUNNEL_ATTR_CUSTOM_RANGE_END
} sai_hostif_tunnel_attr_t;
Copy link
Contributor

@atitjain atitjain Apr 4, 2019

Choose a reason for hiding this comment

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

Where is this used? What is the use of this attribute? Cant we just use the tunnel id? Are you planning to extend:

     * @type sai_object_id_t
     * @flags MANDATORY_ON_CREATE | CREATE_ONLY
     * @objects SAI_OBJECT_TYPE_PORT, SAI_OBJECT_TYPE_LAG, SAI_OBJECT_TYPE_ROUTER_INTERFACE
     * @condition SAI_HOSTIF_TABLE_ENTRY_ATTR_TYPE == SAI_HOSTIF_TABLE_ENTRY_TYPE_PORT or SAI_HOSTIF_TABLE_ENTRY_ATTR_TYPE == SAI_HOSTIF_TABLE_ENTRY_TYPE_VLAN or SAI_HOSTIF_TABLE_ENTRY_ATTR_TYPE == SAI_HOSTIF_TABLE_ENTRY_TYPE_LAG
     */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The primary use case for this will be sFlow.

Copy link
Contributor

@atitjain atitjain Apr 15, 2019

Choose a reason for hiding this comment

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

Why not leverage the sFlow proposal for this. In this looks like you will do the sFlow from the CPU path? I think better is to just enable sFlow, like we do from Port, from tunnel id itself? If the idea is to sample the packets from the transit tunnel, you can apply sFlow action using ACL. If the Scaling is concern, we might need a tunnel table entry that doesnt terminate but use action sFlow.

Refer:
saisamplepacket.h
saiport.h

    /**
     * @brief Enable/Disable Samplepacket session
     *
     * Enable ingress sampling by assigning samplepacket object id Disable
     * ingress sampling by assigning #SAI_NULL_OBJECT_ID as attribute value.
     *
     * @type sai_object_id_t
     * @flags CREATE_AND_SET
     * @objects SAI_OBJECT_TYPE_SAMPLEPACKET
     * @allownull true
     * @default SAI_NULL_OBJECT_ID
     */
    SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE,

    /**
     * @brief Enable/Disable Samplepacket session
     *
     * Enable egress sampling by assigning samplepacket object id Disable
     * egress sampling by assigning #SAI_NULL_OBJECT_ID as attribute value.
     *
     * @type sai_object_id_t
     * @flags CREATE_AND_SET
     * @objects SAI_OBJECT_TYPE_SAMPLEPACKET
     * @allownull true
     * @default SAI_NULL_OBJECT_ID
     */
    SAI_PORT_ATTR_EGRESS_SAMPLEPACKET_ENABLE,

saiacl.h

    /** Set ingress packet sampling */
    SAI_ACL_ACTION_TYPE_INGRESS_SAMPLEPACKET_ENABLE,

    /** Set egress packet sampling */
    SAI_ACL_ACTION_TYPE_EGRESS_SAMPLEPACKET_ENABLE,

saitunnel.h
sai_tunnel_term_table_entry_attr_t

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the use of tunnel since it's being misunderstood with the tunnel infra..

@atitjain
Copy link
Contributor

atitjain commented Apr 4, 2019

A netdev tunnel channel is proposed where an application can create a virtual tunnel interface (over a loopback) to receive trapped packets/buffers from the SAI driver. This can be used as an alternative to callback/FD based mechanisms.

In practice, the tunnel is created by using one of the loopback interfaces only and in that case, it's used to terminate the data traffic itself. So I couldn't quite get the use case you are trying.

@kcudnik
Copy link
Collaborator

kcudnik commented Apr 9, 2019

couple issues to fix:

WARNING: coment is ending without space saihostif.h 890: /** Receive packets via GRE/ERSPAN Type 2*/

WARNING: @brief should start with capital letter: saihostif.h 899: * @brief hostif tunnel attributes

WARNING: @brief should start with capital letter: saihostif.h 942: * @brief meta header valid

WARNING: Word 'hostif' is misspelled saihostif.h 899: * @brief hostif tunnel attributes

WARNING: Value SAI_HOSTIF_TABLE_CHANNEL_ENCAP_TYPE_ERSPAN_TYPE_II of sai_hostif_channel_encap_type_t is not prefixed as SAI_HOSTIF_CHANNEL_ENCAP_TYPE_

WARNING: Value SAI_HOSTIF_TABLE_CHANNEL_ENCAP_TYPE_UDP of sai_hostif_channel_encap_type_t is not prefixed as SAI_HOSTIF_CHANNEL_ENCAP_TYPE_

ERROR: enum sai_hostif_table_entry_channel_encap_type_t has no typedef enum sai_hostif_table_entry_channel_encap_type_t

WARNING: Value SAI_HOSTIF_TABLE_CHANNEL_ENCAP_TYPE_ERSPAN_TYPE_II of sai_hostif_channel_encap_type_t is not prefixed as SAI_HOSTIF_CHANNEL_ENCAP_TYPE_

WARNING: Value SAI_HOSTIF_TABLE_CHANNEL_ENCAP_TYPE_UDP of sai_hostif_channel_encap_type_t is not prefixed as SAI_HOSTIF_CHANNEL_ENCAP_TYPE_

@lguohan
Copy link
Collaborator

lguohan commented Apr 9, 2019

I think we need example and scope in the documents.

@kcudnik
Copy link
Collaborator

kcudnik commented Apr 11, 2019

you added sai_hostif_tunnel_attr_t structure attributes but there is missing SAI_OBJECT_TYPE_HOSTIF_TUNNEL object type in sai_object_type_t

@kcudnik
Copy link
Collaborator

kcudnik commented Apr 11, 2019

you introduced new object type, but there are not api's to create/remove/set/get for this object type in sai_hostif_api_t, you need to add 4 new api's:

create_hostif_tunnel,
remove_hostif_tunnel
set_hostif_tunnel_attribute
get_hostif_tunnel_attribute

you can also try building metadata localy, just go to SAI/meta directory and type "make" to see if everything compiles

@marian-pritsak
Copy link
Contributor

Did you consider using the existing psample infrastructure in Linux as an alternative to protobuf? It allows you to carry metadata along with the packet and is already a part of the mainline kernel.

@sflow-rt
Copy link

+1 for using psample netlink channel to deliver packet samples.

psample is a generic (subsystem independent) method of delivering packet samples and associated metadata. It is also extensible so additional metadata can easily be added in future (queue depth, queuing delay, etc.)

psample is supported by the open source Host sFlow agent.

@padmanarayana padmanarayana changed the title Hostif netdev tunnel Hostif Generic Netlink interface May 16, 2019
Add updated Host-Interface proposal

Change copyright header and fix UDP port type

Change copyright year

Fix warnings in saihostif.h

Fix warnings in saihostif.h

Fix warnings in saihostif.h

Fix warnings in saihostif.h

Update saitypes.h to include SAI_OBJECT_TYPE_HOSTIF_TUNNEL

Add encap parameters to hostif, revert the use of tunnel

Fix warning

Add default for SAI_HOSTIF_ATTR_ENCAP_TYPE

Change conditional to validonly

Add defaults for encap attributes

fix warnings

fix warnings

fix warnings

fix warnings

fix warnings

fix warnings

fix warnings

fix warnings

remove use of tunnel in meta header

Revert Netdev tunnel approach and use Generic Netlink

Fix typo

Add netlink to aspell
@padmanarayana padmanarayana force-pushed the hostif_netdev_tunnel branch from d4a5170 to f9610a0 Compare May 29, 2019 23:22
* @type char
* @flags MANDATORY_ON_CREATE | CREATE_ONLY
* @condition SAI_HOSTIF_ATTR_TYPE == SAI_HOSTIF_TYPE_NETDEV
* @condition SAI_HOSTIF_ATTR_TYPE == SAI_HOSTIF_TYPE_NETDEV or SAI_HOSTIF_ATTR_TYPE == SAI_HOSTIF_TYPE_GENETLINK
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it always equal to "NETLINK_GENERIC"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is set to the generic netlink family name whose multicast port id (SAI_HOSTIF_ATTR_GENETLINK_PORT_ID) is used to lift the samples. If the psample driver is used, it should be set to "psample" (refer https://github.com/torvalds/linux/blob/master/include/uapi/linux/psample.h#L33 ).

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.

8 participants