On 06/04/2018 20:21, Jason Gunthorpe wrote:
> On Wed, Apr 04, 2018 at 12:15:53PM +0300, Alex Rosenbaum wrote:
>
> Your diff seems to have been whitespace mangled..
>
>> +# SYNOPSIS
>> +
>> +```c
>> +#include <infiniband/verbs.h>
>> +
>> +struct ibv_flow_action *
>> +ibv_create_flow_action_encap(struct ibv_context *ctx,
>> + struct ibv_flow_action_encap_attr *attr);
>> +
>> +int ibv_destroy_flow_action(struct ibv_flow_action *action);
>> +```
>
>> +# DESCRIPTION
>> +
>> +An ENCAP flow steering action allows a flow steering rule to encapsulate
>> +a packet after matching.
>> +
>> +After the action is created, it should be associated with a *struct
>> +ibv_flow_attr* using *struct ibv_flow_spec_action_handle* flow specification.
>> +An action can be associated with multiple flows.
>> +
>> +# ARGUMENTS
>> +
>> +*ctx*
>> +: RDMA device context to create the action on.
>> +
>> +*attr*
>> +: ENCAP type and header to be used.
>> +
>> +*action*
>> +: Existing action to be destroyed.
>> +
>> +## *action* Argument
>> +
>> +```c
>> +struct ibv_flow_action_encap_attr {
>> + enum ibv_flow_action_encap hdr_proto;
>> + uint16_t hdr_len;
>> + void *hdr_ptr;
>> +};
>> +```
>
> We don't really need this struct, we really have struct-itis in verbs,
> just put these three things as function arguments.
>
> hdr_len should be size_t and hdr_ptr is const
>
Will fix.
>> +*hdr_proto*
>> +: Encapsulation protocol to be used.
>> +
>> +*hdr_len*
>> +: Length in bytes of the provided encap headers.
>> +
>> +*hdr_ptr*
>> +: Buffing containing the encap headers.
>
> Describe the lifetime, I assume this function copies the header
> internally so hdr_ptr can be deleted by the caller immediately?
Yep, actually the kernel driver only gets an ID/INDEX to be used.
Will update.
>
>> +## Encapsulate protocols (*ibv_flow_action_encap*)
>> +
>> +A user can create flow actions that implement different encapsulation
>> +protocols.
>> +
>> +*IBV_FLOW_ACTION_ENCAP_RAW* provides the ability to provide unspecified
>> +encapsulation protocol, this can be expressed by passing it in *hdr_proto*.
>
> This man page doesn't actually describe what is supposed to happen.
>
> I guess it is something like
>
> Prepend the given hdr_len bytes of hdr_ptr in front of the packet
> before sending it
>
> ?
Well, this is mostly true, the HW might be doing some modification to the
headers, like writing the correct checksum value/proper length and stuff
like that, but it's all pretty standard stuff when it comes to encapsulation.
One thing I didn't mention, is that there might be some MTU math that needs to
be done by the user, for example if the MTU is 1500, and you know you are going
to do vxlan encap (without vlan) in HW, you probably don't want to send packets bigger than
1450 as the encap operation will add 50 bytes to the packet.
>
> Given that is RAW the right proto mode? Maybe just 'PREPEND' ?
I think RAW is better, as there might be some other vendors that will need to provide
a type, like MPLS/GRE/VXLAN, and then the PREPEND will look out of place as they
all do pre append.
>
> Jason
>
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
