[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
2014-11-13 11:24, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2014-11-13 03:17, Liu, Jijiang:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2014-10-23 02:23, Zhang, Helin:
> > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas
> > > > > Monjalon
> > > > > > 2014-10-21 14:14, Liu, Jijiang:
> > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > > > > > + uint16_t packet_type;
> > > > > > > >
> > > > > > > > Why not name it "l2_type"?
> > > > >
> > > > > 'packet_type' is for storing the hardware identified packet type
> > > > > upon different layers of protocols (l2, l3, l4, ...).
> > > > > It is quite useful for user application or middle layer software
> > > > > stacks, it can know what the packet type is without checking the
> > > > > packet too
> > > > much by software.
> > > > > Actually ixgbe already has packet types (less than 10), which is
> > > > > transcoded into
> > > > 'ol_flags'.
> > > > > For i40e, the packet type can represent about 256 types of packet,
> > > > > 'ol_flags' does not have enough bits for it anymore. So put the
> > > > > i40e packet types
> > > > into mbuf would be better.
> > > > > Also this field can be used for NON-Intel NICs, I think there must
> > > > > be the similar concepts of other NICs. And 16 bits 'packet_type'
> > > > > has severl
> > > > reserved bits for future and NON-Intel NICs.
> > > >
> > > > Thanks Helin, that's the best description of packet_type I've seen so
> > > > far.
> > > > It's not so clear in the commit log:
> > > > http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf
> > > >
> > > > > > > In datasheet, this term is called packet type(s).
> > > > > >
> > > > > > That's exactly the point I want you really understand!
> > > > > > This is a field in generic mbuf structure, so your datasheet has no
> > > > > > value here.
> > > > > >
> > > > > > > Personally , I think packet type is more clear what meaning of
> > > > > > > this field is .
> > > > > >
> > > > > > You cannot add an API field without knowing what will be its
> > > > > > generic meaning.
> > > > > > Please think about it and describe its scope.
> > > >
> > > > I integrated this patch with the VXLAN patchset in the hope that
> > > > you'll improve the situation afterwards.
> > > > This is the answer you recently gave to Olivier:
> > > > http://dpdk.org/ml/archives/dev/2014-November/007599.html
> > > > "
> > > > Regarding adding a packet_type in mbuf, we ever had a lot of
> > > > discussions as follows:
> > > > http://dpdk.org/ml/archives/dev/2014-October/007027.html
> > > > http://dpdk.org/ml/archives/dev/2014-September/005240.html
> > > > http://dpdk.org/ml/archives/dev/2014-September/005241.html
> > > > http://dpdk.org/ml/archives/dev/2014-September/005274.html
> > > > "
> > > >
> > > > To sum up the situation:
> > > > - We don't know what are the possible values of packet_type
> > > > - It's only filled by i40e, while other drivers use ol_flags
> > > > - There is no special value "unknown" which should be set by drivers
> > > > not supporting this feature.
> > > > - Its only usage is to print a decimal value in
> > > > app/test-pmd/rxonly.c
> > > >
> > > > It's now clear that nobody cares about this part of the API.
> > > > So I'm going to remove packet_type from mbuf.
> > > > I don't want to keep something that we don't know how to use, that
> > > > is not consistent across drivers, and that overlap another API part
> > > > (ol_flags).
> > >
> > > The packet type in 40e is very important for user, using packet type
> > > can help to speed up packet analysis/identification in their
> > > application, especially tunneling packet format.
> > > Now I'm working on implementing packet type definition in rte_ethdev.h
> > > file and translation table in i40e, which is almost done.
> > > The packet type definition in in rte_ethdev.h file like below.
> > > /*
> > > * Ethernet packet type
> > > */
> > > enum rte_eth_ptype {
> > > /* undefined packet type, means HW can't recognise it */
> > > RTE_PTYPE_UNDEF = 0,
> > > ...
> > >
> > > /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv4 */
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv4FRAG_PAY3,
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_PAY3,
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_UDP_PAY4,
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_TCP_PAY4,
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_SCTP_PAY4,
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_ICMP_PAY4,
> > >
> > > /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv6 */
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv6FRAG_PAY3
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_PAY3,
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_UDP_PAY4,
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_TCP_PAY4,
> > > RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_SCTP_PAY4,
> > >
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
Hi, > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, November 13, 2014 4:53 PM > To: Liu, Jijiang > Cc: Zhang, Helin; dev at dpdk.org; Richardson, Bruce > Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure > changes > > 2014-11-13 03:17, Liu, Jijiang: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > 2014-10-23 02:23, Zhang, Helin: > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas > > > > Monjalon > > > > > 2014-10-21 14:14, Liu, Jijiang: > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > > > > 2014-10-21 16:46, Jijiang Liu: > > > > > > > > + uint16_t packet_type; > > > > > > > > > > > > > > Why not name it "l2_type"? > > > > > > > > 'packet_type' is for storing the hardware identified packet type > > > > upon different layers of protocols (l2, l3, l4, ...). > > > > It is quite useful for user application or middle layer software > > > > stacks, it can know what the packet type is without checking the > > > > packet too > > > much by software. > > > > Actually ixgbe already has packet types (less than 10), which is > > > > transcoded into > > > 'ol_flags'. > > > > For i40e, the packet type can represent about 256 types of packet, > > > > 'ol_flags' does not have enough bits for it anymore. So put the > > > > i40e packet types > > > into mbuf would be better. > > > > Also this field can be used for NON-Intel NICs, I think there must > > > > be the similar concepts of other NICs. And 16 bits 'packet_type' > > > > has severl > > > reserved bits for future and NON-Intel NICs. > > > > > > Thanks Helin, that's the best description of packet_type I've seen so far. > > > It's not so clear in the commit log: > > > http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf > > > > > > > > > In datasheet, this term is called packet type(s). > > > > > > > > > > That's exactly the point I want you really understand! > > > > > This is a field in generic mbuf structure, so your datasheet has no > > > > > value > here. > > > > > > > > > > > Personally , I think packet type is more clear what meaning of > > > > > > this field > is . > > > > > > > > > > You cannot add an API field without knowing what will be its generic > meaning. > > > > > Please think about it and describe its scope. > > > > > > I integrated this patch with the VXLAN patchset in the hope that > > > you'll improve the situation afterwards. > > > This is the answer you recently gave to Olivier: > > > http://dpdk.org/ml/archives/dev/2014-November/007599.html > > > " > > > Regarding adding a packet_type in mbuf, we ever had a lot of > > > discussions as follows: > > > http://dpdk.org/ml/archives/dev/2014-October/007027.html > > > http://dpdk.org/ml/archives/dev/2014-September/005240.html > > > http://dpdk.org/ml/archives/dev/2014-September/005241.html > > > http://dpdk.org/ml/archives/dev/2014-September/005274.html > > > " > > > > > > To sum up the situation: > > > - We don't know what are the possible values of packet_type > > > - It's only filled by i40e, while other drivers use ol_flags > > > - There is no special value "unknown" which should be set by drivers > > > not supporting this feature. > > > - Its only usage is to print a decimal value in > > > app/test-pmd/rxonly.c > > > > > > It's now clear that nobody cares about this part of the API. > > > So I'm going to remove packet_type from mbuf. > > > I don't want to keep something that we don't know how to use, that > > > is not consistent across drivers, and that overlap another API part > > > (ol_flags). > > > > The packet type in 40e is very important for user, using packet type > > can help to speed up packet analysis/identification in their > > application, especially tunneling packet format. > > Now I'm working on implementing packet type definition in rte_ethdev.h > > file and translation table in i40e, which is almost done. > > The packet type definition in in rte_ethdev.h file like below. >
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, November 12, 2014 9:26 PM
> To: Liu, Jijiang
> Cc: Zhang, Helin; dev at dpdk.org; Richardson, Bruce
> Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure
> changes
>
> Hi guys,
>
> We still have some problems with the mbuf changes introduced for VXLAN.
> I want to raise the packet type issue here.
>
> 2014-10-23 02:23, Zhang, Helin:
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > > 2014-10-21 14:14, Liu, Jijiang:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > > - uint16_t reserved2; /**< Unused field. Required for
> > > > > > padding
> */
> > > > > > +
> > > > > > + /**
> > > > > > +* Packet type, which is used to indicate ordinary L2 packet
> format and
> > > > > > +* also tunneled packet format such as IP in IP, IP in GRE, MAC
> > > > > > in
> GRE
> > > > > > +* and MAC in UDP.
> > > > > > +*/
> > > > > > + uint16_t packet_type;
> > > > >
> > > > > Why not name it "l2_type"?
> >
> > 'packet_type' is for storing the hardware identified packet type upon
> > different layers of protocols (l2, l3, l4, ...).
> > It is quite useful for user application or middle layer software
> > stacks, it can know what the packet type is without checking the packet too
> much by software.
> > Actually ixgbe already has packet types (less than 10), which is transcoded
> > into
> 'ol_flags'.
> > For i40e, the packet type can represent about 256 types of packet,
> > 'ol_flags' does not have enough bits for it anymore. So put the i40e packet
> > types
> into mbuf would be better.
> > Also this field can be used for NON-Intel NICs, I think there must be
> > the similar concepts of other NICs. And 16 bits 'packet_type' has severl
> reserved bits for future and NON-Intel NICs.
>
> Thanks Helin, that's the best description of packet_type I've seen so far.
> It's not so clear in the commit log:
> http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf
>
> > > > In datasheet, this term is called packet type(s).
> > >
> > > That's exactly the point I want you really understand!
> > > This is a field in generic mbuf structure, so your datasheet has no value
> > > here.
> > >
> > > > Personally , I think packet type is more clear what meaning of this
> > > > field is .
> > >
> > > You cannot add an API field without knowing what will be its generic
> > > meaning.
> > > Please think about it and describe its scope.
>
> I integrated this patch with the VXLAN patchset in the hope that you'll
> improve
> the situation afterwards.
> This is the answer you recently gave to Olivier:
> http://dpdk.org/ml/archives/dev/2014-November/007599.html
> "
> Regarding adding a packet_type in mbuf, we ever had a lot of discussions
> as follows:
> http://dpdk.org/ml/archives/dev/2014-October/007027.html
> http://dpdk.org/ml/archives/dev/2014-September/005240.html
> http://dpdk.org/ml/archives/dev/2014-September/005241.html
> http://dpdk.org/ml/archives/dev/2014-September/005274.html
> "
>
> To sum up the situation:
> - We don't know what are the possible values of packet_type
> - It's only filled by i40e, while other drivers use ol_flags
> - There is no special value "unknown" which should be set by drivers
> not supporting this feature.
> - Its only usage is to print a decimal value in app/test-pmd/rxonly.c
>
> It's now clear that nobody cares about this part of the API.
> So I'm going to remove packet_type from mbuf.
> I don't want to keep something that we don't know how to use, that is not
> consistent across drivers, and that overlap another API part (ol_flags).
The packet type in 40e is very important for user, using packet type can help
to speed up packet analysis/identification in their application, especially
tunneling packet format.
Now I'm working on implementing packet type definition in rte_ethdev.h file and
translation table in i40e, which is almost done.
The packet type definition in in rte_ethdev.h file like below.
/*
* Ethernet packet type
*/
enum rte_eth_ptype {
/* undefined packet type, means HW can't recognise it */
RTE_PTYPE_UNDEF = 0,
...
/* IPv4
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
2014-11-12 14:31, Zhang, Helin: > > 2014-10-23 02:23, Zhang, Helin: > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > > > > 2014-10-21 14:14, Liu, Jijiang: > > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > > > 2014-10-21 16:46, Jijiang Liu: > > > > > > > + uint16_t packet_type; > > > > > > > > > > > > Why not name it "l2_type"? > > > > > > 'packet_type' is for storing the hardware identified packet type upon > > > different layers of protocols (l2, l3, l4, ...). > > > It is quite useful for user application or middle layer software > > > stacks, it can know what the packet type is without checking the packet > > > too > > much by software. > > > Actually ixgbe already has packet types (less than 10), which is > > > transcoded into > > 'ol_flags'. > > > For i40e, the packet type can represent about 256 types of packet, > > > 'ol_flags' does not have enough bits for it anymore. So put the i40e > > > packet > > types into mbuf would be better. > > > Also this field can be used for NON-Intel NICs, I think there must be > > > the similar concepts of other NICs. And 16 bits 'packet_type' has severl > > reserved bits for future and NON-Intel NICs. > > > > Thanks Helin, that's the best description of packet_type I've seen so far. > > It's not so clear in the commit log: > > http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf > > > > > > > In datasheet, this term is called packet type(s). > > > > > > > > That's exactly the point I want you really understand! > > > > This is a field in generic mbuf structure, so your datasheet has no > > > > value here. > > > > > > > > > Personally , I think packet type is more clear what meaning of this > > > > > field is . > > > > > > > > You cannot add an API field without knowing what will be its generic > > > > meaning. > > > > Please think about it and describe its scope. > > > > I integrated this patch with the VXLAN patchset in the hope that you'll > > improve > > the situation afterwards. > > This is the answer you recently gave to Olivier: > > http://dpdk.org/ml/archives/dev/2014-November/007599.html > > " > > Regarding adding a packet_type in mbuf, we ever had a lot of > > discussions as > > follows: > > http://dpdk.org/ml/archives/dev/2014-October/007027.html > > http://dpdk.org/ml/archives/dev/2014-September/005240.html > > http://dpdk.org/ml/archives/dev/2014-September/005241.html > > http://dpdk.org/ml/archives/dev/2014-September/005274.html > > " > > > > To sum up the situation: > > - We don't know what are the possible values of packet_type > > - It's only filled by i40e, while other drivers use ol_flags > > - There is no special value "unknown" which should be set by drivers > > not supporting this feature. > > - Its only usage is to print a decimal value in app/test-pmd/rxonly.c > > Though I haven't investigate this too much, my opinion is that we should > use packet_type in the future, and rework igb/ixgbe PMD to remove all > packet types in ol_flags and use packet_type instead. > Then example app can use the packet type directly. And all igb, ixgbe and > i40e packet_type are consistent. Sure we might need to define all packet > types in rte_ethdev.h or similar header files. Exact! > > It's now clear that nobody cares about this part of the API. > > So I'm going to remove packet_type from mbuf. > > I don't want to keep something that we don't know how to use, that is not > > consistent across drivers, and that overlap another API part (ol_flags). Helin, I feel you perfectly understood the problem. As the responsible of i40e, you can make a choice for 1.8 release: - remove (incomplete) packet_type - or complete it quickly Thanks -- Thomas
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, November 12, 2014 9:26 PM > To: Liu, Jijiang > Cc: Zhang, Helin; dev at dpdk.org; Richardson, Bruce > Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure > changes > > Hi guys, > > We still have some problems with the mbuf changes introduced for VXLAN. > I want to raise the packet type issue here. > > 2014-10-23 02:23, Zhang, Helin: > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > > > 2014-10-21 14:14, Liu, Jijiang: > > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > > 2014-10-21 16:46, Jijiang Liu: > > > > > > - uint16_t reserved2; /**< Unused field. Required for > > > > > > padding */ > > > > > > + > > > > > > + /** > > > > > > +* Packet type, which is used to indicate ordinary L2 packet > format and > > > > > > +* also tunneled packet format such as IP in IP, IP in GRE, MAC > > > > > > in > GRE > > > > > > +* and MAC in UDP. > > > > > > +*/ > > > > > > + uint16_t packet_type; > > > > > > > > > > Why not name it "l2_type"? > > > > 'packet_type' is for storing the hardware identified packet type upon > > different layers of protocols (l2, l3, l4, ...). > > It is quite useful for user application or middle layer software > > stacks, it can know what the packet type is without checking the packet too > much by software. > > Actually ixgbe already has packet types (less than 10), which is transcoded > > into > 'ol_flags'. > > For i40e, the packet type can represent about 256 types of packet, > > 'ol_flags' does not have enough bits for it anymore. So put the i40e packet > types into mbuf would be better. > > Also this field can be used for NON-Intel NICs, I think there must be > > the similar concepts of other NICs. And 16 bits 'packet_type' has severl > reserved bits for future and NON-Intel NICs. > > Thanks Helin, that's the best description of packet_type I've seen so far. > It's not so clear in the commit log: > http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf > > > > > In datasheet, this term is called packet type(s). > > > > > > That's exactly the point I want you really understand! > > > This is a field in generic mbuf structure, so your datasheet has no value > > > here. > > > > > > > Personally , I think packet type is more clear what meaning of this > > > > field is . > > > > > > You cannot add an API field without knowing what will be its generic > > > meaning. > > > Please think about it and describe its scope. > > I integrated this patch with the VXLAN patchset in the hope that you'll > improve > the situation afterwards. > This is the answer you recently gave to Olivier: > http://dpdk.org/ml/archives/dev/2014-November/007599.html > " > Regarding adding a packet_type in mbuf, we ever had a lot of > discussions as > follows: > http://dpdk.org/ml/archives/dev/2014-October/007027.html > http://dpdk.org/ml/archives/dev/2014-September/005240.html > http://dpdk.org/ml/archives/dev/2014-September/005241.html > http://dpdk.org/ml/archives/dev/2014-September/005274.html > " > > To sum up the situation: > - We don't know what are the possible values of packet_type > - It's only filled by i40e, while other drivers use ol_flags > - There is no special value "unknown" which should be set by drivers > not supporting this feature. > - Its only usage is to print a decimal value in app/test-pmd/rxonly.c Though I haven't investigate this too much, my opinion is that we should use packet_type in the future, and rework igb/ixgbe PMD to remove all packet types in ol_flags and use packet_type instead. Then example app can use the packet type directly. And all igb, ixgbe and i40e packet_type are consistent. Sure we might need to define all packet types in rte_ethdev.h or similar header files. > > It's now clear that nobody cares about this part of the API. > So I'm going to remove packet_type from mbuf. > I don't want to keep something that we don't know how to use, that is not > consistent across drivers, and that overlap another API part (ol_flags). > > -- > Thomas Regards, Helin
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
Hi guys, We still have some problems with the mbuf changes introduced for VXLAN. I want to raise the packet type issue here. 2014-10-23 02:23, Zhang, Helin: > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > > 2014-10-21 14:14, Liu, Jijiang: > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > 2014-10-21 16:46, Jijiang Liu: > > > > > - uint16_t reserved2; /**< Unused field. Required for > > > > > padding */ > > > > > + > > > > > + /** > > > > > + * Packet type, which is used to indicate ordinary L2 packet > > > > > format and > > > > > + * also tunneled packet format such as IP in IP, IP in GRE, MAC > > > > > in GRE > > > > > + * and MAC in UDP. > > > > > + */ > > > > > + uint16_t packet_type; > > > > > > > > Why not name it "l2_type"? > > 'packet_type' is for storing the hardware identified packet type upon > different layers > of protocols (l2, l3, l4, ...). > It is quite useful for user application or middle layer software stacks, it > can know > what the packet type is without checking the packet too much by software. > Actually ixgbe already has packet types (less than 10), which is transcoded > into 'ol_flags'. > For i40e, the packet type can represent about 256 types of packet, 'ol_flags' > does not > have enough bits for it anymore. So put the i40e packet types into mbuf would > be better. > Also this field can be used for NON-Intel NICs, I think there must be the > similar concepts > of other NICs. And 16 bits 'packet_type' has severl reserved bits for future > and NON-Intel NICs. Thanks Helin, that's the best description of packet_type I've seen so far. It's not so clear in the commit log: http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf > > > In datasheet, this term is called packet type(s). > > > > That's exactly the point I want you really understand! > > This is a field in generic mbuf structure, so your datasheet has no value > > here. > > > > > Personally , I think packet type is more clear what meaning of this > > > field is . > > > > You cannot add an API field without knowing what will be its generic > > meaning. > > Please think about it and describe its scope. I integrated this patch with the VXLAN patchset in the hope that you'll improve the situation afterwards. This is the answer you recently gave to Olivier: http://dpdk.org/ml/archives/dev/2014-November/007599.html " Regarding adding a packet_type in mbuf, we ever had a lot of discussions as follows: http://dpdk.org/ml/archives/dev/2014-October/007027.html http://dpdk.org/ml/archives/dev/2014-September/005240.html http://dpdk.org/ml/archives/dev/2014-September/005241.html http://dpdk.org/ml/archives/dev/2014-September/005274.html " To sum up the situation: - We don't know what are the possible values of packet_type - It's only filled by i40e, while other drivers use ol_flags - There is no special value "unknown" which should be set by drivers not supporting this feature. - Its only usage is to print a decimal value in app/test-pmd/rxonly.c It's now clear that nobody cares about this part of the API. So I'm going to remove packet_type from mbuf. I don't want to keep something that we don't know how to use, that is not consistent across drivers, and that overlap another API part (ol_flags). -- Thomas
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
Hi > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon > Sent: Wednesday, October 22, 2014 4:46 PM > To: Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure > changes > > 2014-10-21 14:14, Liu, Jijiang: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > 2014-10-21 16:46, Jijiang Liu: > > > > - uint16_t reserved2; /**< Unused field. Required for > > > > padding > */ > > > > + > > > > + /** > > > > +* Packet type, which is used to indicate ordinary L2 packet > > > > format > and > > > > +* also tunneled packet format such as IP in IP, IP in GRE, MAC > > > > in GRE > > > > +* and MAC in UDP. > > > > +*/ > > > > + uint16_t packet_type; > > > > > > Why not name it "l2_type"? 'packet_type' is for storing the hardware identified packet type upon different layers of protocols (l2, l3, l4, ...). It is quite useful for user application or middle layer software stacks, it can know what the packet type is without checking the packet too much by software. Actually ixgbe already has packet types (less than 10), which is transcoded into 'ol_flags'. For i40e, the packet type can represent about 256 types of packet, 'ol_flags' does not have enough bits for it anymore. So put the i40e packet types into mbuf would be better. Also this field can be used for NON-Intel NICs, I think there must be the similar concepts of other NICs. And 16 bits 'packet_type' has severl reserved bits for future and NON-Intel NICs. > > > > In datasheet, this term is called packet type(s). > > That's exactly the point I want you really understand! > This is a field in generic mbuf structure, so your datasheet has no value > here. > > > Personally , I think packet type is more clear what meaning of this field > > is . > ^_^ > > You cannot add an API field without knowing what will be its generic meaning. > Please think about it and describe its scope. > > Thanks > -- > Thomas Regards, Helin
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
2014-10-21 14:14, Liu, Jijiang: > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > 2014-10-21 16:46, Jijiang Liu: > > > - uint16_t reserved2; /**< Unused field. Required for padding */ > > > + > > > + /** > > > + * Packet type, which is used to indicate ordinary L2 packet format and > > > + * also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE > > > + * and MAC in UDP. > > > + */ > > > + uint16_t packet_type; > > > > Why not name it "l2_type"? > > In datasheet, this term is called packet type(s). That's exactly the point I want you really understand! This is a field in generic mbuf structure, so your datasheet has no value here. > Personally , I think packet type is more clear what meaning of this field is > . ^_^ You cannot add an API field without knowing what will be its generic meaning. Please think about it and describe its scope. Thanks -- Thomas
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, October 22, 2014 4:46 PM > To: Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure > changes > > 2014-10-21 14:14, Liu, Jijiang: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > 2014-10-21 16:46, Jijiang Liu: > > > > - uint16_t reserved2; /**< Unused field. Required for > > > > padding */ > > > > + > > > > + /** > > > > +* Packet type, which is used to indicate ordinary L2 packet > > > > format > and > > > > +* also tunneled packet format such as IP in IP, IP in GRE, MAC > > > > in GRE > > > > +* and MAC in UDP. > > > > +*/ > > > > + uint16_t packet_type; > > > > > > Why not name it "l2_type"? > > > > In datasheet, this term is called packet type(s). > > That's exactly the point I want you really understand! > This is a field in generic mbuf structure, so your datasheet has no value > here. > > > Personally , I think packet type is more clear what meaning of this field > > is . > ^_^ > > You cannot add an API field without knowing what will be its generic > meaning. > Please think about it and describe its scope. its generic meaning is that each UNIT number stand for a kind of packet format? I will add more description for this field. > Thanks > -- > Thomas
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
Remove the "reserved2" field and add the "packet_type" and the
"inner_l2_l3_len" fields in the rte_mbuf structure.
The packet type field is used to indicate ordinary L2 packet format and also
tunneling packet format such as IP in IP,
IP in GRE, MAC in GRE and MAC in UDP.
The inner L2 length and the inner L3 length are used for TX offloading of
tunneling packet.
Signed-off-by: Jijiang Liu
Acked-by: Helin Zhang
Acked-by: Jingjing Wu
---
lib/librte_mbuf/rte_mbuf.h | 25 -
1 files changed, 24 insertions(+), 1 deletions(-)
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index ddadc21..98951a6 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -163,7 +163,14 @@ struct rte_mbuf {
/* remaining bytes are set on RX when pulling packet from descriptor */
MARKER rx_descriptor_fields1;
- uint16_t reserved2; /**< Unused field. Required for padding */
+
+ /**
+* Packet type, which is used to indicate ordinary L2 packet format and
+* also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE
+* and MAC in UDP.
+*/
+ uint16_t packet_type;
+
uint16_t data_len;/**< Amount of data in segment buffer. */
uint32_t pkt_len; /**< Total pkt len: sum of all segments. */
uint16_t vlan_tci;/**< VLAN Tag Control Identifier (CPU order)
*/
@@ -196,6 +203,18 @@ struct rte_mbuf {
uint16_t l2_len:7; /**< L2 (MAC) Header Length. */
};
};
+
+ /* fields for TX offloading of tunnels */
+ union {
+ uint16_t inner_l2_l3_len;
+ /**< combined inner l2/l3 lengths as single var */
+ struct {
+ uint16_t inner_l3_len:9;
+ /**< inner L3 (IP) Header Length. */
+ uint16_t inner_l2_len:7;
+ /**< inner L2 (MAC) Header Length. */
+ };
+ };
} __rte_cache_aligned;
/**
@@ -546,11 +565,13 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
m->next = NULL;
m->pkt_len = 0;
m->l2_l3_len = 0;
+ m->inner_l2_l3_len = 0;
m->vlan_tci = 0;
m->nb_segs = 1;
m->port = 0xff;
m->ol_flags = 0;
+ m->packet_type = 0;
m->data_off = (RTE_PKTMBUF_HEADROOM <= m->buf_len) ?
RTE_PKTMBUF_HEADROOM : m->buf_len;
@@ -614,12 +635,14 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf
*mi, struct rte_mbuf *md)
mi->port = md->port;
mi->vlan_tci = md->vlan_tci;
mi->l2_l3_len = md->l2_l3_len;
+ mi->inner_l2_l3_len = md->inner_l2_l3_len;
mi->hash = md->hash;
mi->next = NULL;
mi->pkt_len = mi->data_len;
mi->nb_segs = 1;
mi->ol_flags = md->ol_flags;
+ mi->packet_type = md->packet_type;
__rte_mbuf_sanity_check(mi, 1);
__rte_mbuf_sanity_check(md, 0);
--
1.7.7.6
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, October 21, 2014 6:26 PM > To: Liu, Jijiang > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure > changes > > Hi Jijiang, > > 2014-10-21 16:46, Jijiang Liu: > > Remove the "reserved2" field and add the "packet_type" > > "Remove and add" can be said "Replace". > > > and the "inner_l2_l3_len" fields in the rte_mbuf structure. > > Please explain that you are using 2 bytes of the second cache line for TX > offloading of tunnels. > > > /* remaining bytes are set on RX when pulling packet from descriptor > */ > > MARKER rx_descriptor_fields1; > > - uint16_t reserved2; /**< Unused field. Required for padding */ > > + > > + /** > > +* Packet type, which is used to indicate ordinary L2 packet format > and > > +* also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE > > +* and MAC in UDP. > > +*/ > > + uint16_t packet_type; > > Why not name it "l2_type"? In datasheet, this term is called packet type(s). Personally , I think packet type is more clear what meaning of this field is . ^_^ > -- > Thomas
[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes
Hi Jijiang, 2014-10-21 16:46, Jijiang Liu: > Remove the "reserved2" field and add the "packet_type" "Remove and add" can be said "Replace". > and the "inner_l2_l3_len" fields in the rte_mbuf structure. Please explain that you are using 2 bytes of the second cache line for TX offloading of tunnels. > /* remaining bytes are set on RX when pulling packet from descriptor */ > MARKER rx_descriptor_fields1; > - uint16_t reserved2; /**< Unused field. Required for padding */ > + > + /** > + * Packet type, which is used to indicate ordinary L2 packet format and > + * also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE > + * and MAC in UDP. > + */ > + uint16_t packet_type; Why not name it "l2_type"? -- Thomas
