Hi,
The API patch should make it more clear what MTU means in the
context of ODP packet I/O.
I think it should mean something close to this:
ODP must be able to send and receive packets up to the MTU size
succesfully. Packets larger than the MTU may get dropped or may
get received successfully (i.e. setting MTU does not guarantee
that larger packets get dropped by ODP). Trying to send packets
larger than MTU results in undefined behavior (or maybe that is
too harsh and it should be that such packets may get dropped?).
The transmit behavior with respect to MTU in case of IPsec inline
output probably requires clarification.
An exact definition of which packet (or L2 frame) bytes get counted
towards the MTU is also needed (e.g. is Ethernet FCS counted? Or
VLAN tag?)
The API should document what the default MTU value is before
the API user calls odp_pktio_mtu_set().
> >> + /** Allow app to set MTU size */
> >> + uint32_t mtu_set : 1;
It is not clear why this would be needed. If the underlying
implementation has a fixed MTU, then at API level ODP could
still allow the MTU to be set to any lower value. It just would
not affect anything.
> sure, will add min_mtu_size field in odp_paktio_capability_t.
Similarly, it is not clear why min MTU is needed. If an application
says MTU of 1 byte is enough, the ODP implementation could just
accept it, even if some underlying HW MTU would be left to some
larger value.
If the MTU setting guaranteed that all packets bigger than the
MTU would be accurately dropped, then the API would probably
need the min MTU. But I do not see a real use case for such
accurate packet length based filtering and it could be a bit
of a paint for some implementations or some pktio types.
Janne
> -----Original Message-----
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of vamsi a
> Sent: Friday, August 11, 2017 8:22 AM
> To: Bill Fischofer <bill.fischo...@linaro.org>
> Cc: Mahipal Challa <mcha...@cavium.com>; vattun...@cavium.com; Shally Verma
> <sve...@cavium.com>; lng-odp-forward <lng-odp@lists.linaro.org>
> Subject: Re: [lng-odp] [PATCH API-NEXT v1 1/1] api:pktio: Adds MTU set
> function.
>
> On Tue, Aug 8, 2017 at 5:08 PM, Bill Fischofer <bill.fischo...@linaro.org>
> wrote:
>
> >
> >
> > On Tue, Aug 8, 2017 at 2:38 AM, Vamsi Attunuru <vamsi.cav...@gmail.com>
> > wrote:
> >
> >> Signed-off-by: Vamsi Attunuru <vattun...@cavium.com>
> >> Signed-off-by: Shally Verma <sve...@cavium.com>
> >> Signed-off-by: Mahipal Challa <mcha...@cavium.com>
> >>
> >> ---
> >> include/odp/api/spec/packet_io.h | 21 +++++++++++++++++++++
> >> 1 file changed, 21 insertions(+)
> >>
> >> diff --git a/include/odp/api/spec/packet_io.h
> >> b/include/odp/api/spec/packet_io.h
> >> index d42cebf..be81c3d 100644
> >> --- a/include/odp/api/spec/packet_io.h
> >> +++ b/include/odp/api/spec/packet_io.h
> >> @@ -452,6 +452,9 @@ typedef union odp_pktio_set_op_t {
> >> struct {
> >> /** Promiscuous mode */
> >> uint32_t promisc_mode : 1;
> >> +
> >> + /** Allow app to set MTU size */
> >> + uint32_t mtu_set : 1;
> >> } op;
> >> /** All bits of the bit field structure.
> >> * This field can be used to set/clear all flags, or bitwise
> >> @@ -480,6 +483,9 @@ typedef struct odp_pktio_capability_t {
> >>
> >> /** @deprecated Use enable_loop inside odp_pktin_config_t */
> >> odp_bool_t ODP_DEPRECATE(loop_supported);
> >> +
> >> + /** Maximum MTU size supported */
> >> + uint32_t max_mtu_size;
> >> } odp_pktio_capability_t;
> >>
> >> /**
> >> @@ -910,6 +916,21 @@ int odp_pktout_send(odp_pktout_queue_t queue, const
> >> odp_packet_t packets[],
> >> uint32_t odp_pktio_mtu(odp_pktio_t pktio);
> >>
> >> /**
> >> + * Set MTU value of a packet IO interface.
> >> + *
> >> + * Application should pass value upto max_mtu_size as indicated by
> >> + * odp_pktio_capability_t:max_mtu_size. Any value beyond max_mtu_size
> >> + * limit will result in failure. mtu value < 68 also results in failure.
> >>
> >
> > Seems like it would be better to have an explicit min_mtu_size in the
> > odp_pktio_capability() than this arbitrary note buried as a comment.
> >
>
> sure, will add min_mtu_size field in odp_paktio_capability_t.
>
> From the last discussion, odp_pktio_config() api was suggested instead of
> set_mtu() api, if my understanding is correct, Is the following sequence
> allowed to occur multiple times like stop pktio interface, call
> pktio_config() and start pktio interface.
>
> >
> >
> >> + *
> >> + * @param pktio Packet IO handle.
> >> + * @param mtu MTU value to be set.
> >> + *
> >> + * @return 0 on success
> >> + * @retval <0 on failure
> >> + */
> >> +int odp_pktio_mtu_set(odp_pktio_t pktio, uint32_t mtu);
> >> +
> >> +/**
> >> * Enable/Disable promiscuous mode on a packet IO interface.
> >> *
> >> * @param[in] pktio Packet IO handle.
> >> --
> >> 1.9.3
> >>
> >>
> >
