On Tue, Apr 30, 2013 at 09:19:51AM -0700, Jesse Gross wrote:

On Mon, Apr 29, 2013 at 8:21 PM, Simon Horman [off-list ref] wrote:

quoted

On Fri, Apr 26, 2013 at 04:03:21PM -0700, Jesse Gross wrote:

quoted

On Thu, Apr 25, 2013 at 12:36 AM, Simon Horman [off-list ref] wrote:

quoted

On Tue, Apr 23, 2013 at 02:00:19PM -0700, Joseph Gasparakis wrote:

quoted

Any particular reason to introduce skb->encapsulation_features instead of
using the existing skb->encapsulation? Also I don't see it used in your
second patch either.

My reasoning is that skb->encapsulation seems to alter the behaviour of
many different locations and I'm not sure that any of them, other than the
one in dev_hard_start_xmit() make sense for MPLS.

The problem is the meaning of skb->encapsulation isn't really defined
clearly and I'm certain that the current implementation is not going
to work in the future. Depending on your perspective, vlans, MPLS,
tunnels, etc. can all be considered forms of encapsulation but clearly
there are many NICs that have different capabilities across those. I
believe the intention here was really to describe L3 tunnels as
encapsulation, in which case MPLS really shouldn't be using this
mechanism at all.

Now there is some overlap, especially today since most currently
shipping silicon wasn't designed to support tunnels and so is using
some form of offset based offloads. In that case, all forms of
encapsulation are pretty similar. However, in the future that won't be
the case as support for specific protocols is implemented for higher
performance and richer support. When that happens, not only will MPLS
and tunnels have different capabilities but various forms tunnels
might as well.

Wouldn't be possible to describe those differences using,
dev->hw_enc_features? I assumed that was its purpose.

If there truly are differences between the offload capabilities of
MPLS and L3 tunnels then no, it's not possible, because it's a single
field. It's certainly not a valid assumption that a NIC that can do
TSO over GRE can also do it over MPLS.

However, it's unlikely that there are truly significant differences
between various encapsulation formats on a per-feature basis. I think
what we need to do is separate out the ability to understand the
headers from the capabilities so you have two fields: header (none,
VLAN, QinQ, MPLS, VXLAN, GRE, etc.) and feature (checksum, SG, TSO,
etc.) rather than the product of each. Otherwise, we end up with a ton
of different combinations.

I'm not quite sure that I follow.

Is your idea to replace skb->encapsulation (a single bit) with
a field that corresponds to the outer-most (encapsulation) header in use
and has bits for none, VLAN, QinQ, MPLS, VXLAN, GRE, etc...?

If so, I believe that would solve the problem I was trying to address
with this patch.