> > but the general idea is that we
> > report this status, so if you say that my version is also good for you,
> > I'll leave it as is. It was just a rationale for my decision.
>
> It's fine. But please avoid the code churn in xenvif_tx_submit
> with to extra indentation. This is equivalent:
>
> - if (skb_is_gso(skb)) {
> + if (skb_is_gso(skb) && th_set) {
>
> More fundamentally, the code has the assumption that th_set
> always holds if skb_is_gso(skb). Why add the check? This is
> another example that the return value is not really needed.
What about
if (skb_is_gso(skb)) {
BUG_ON(!skb_transport_header_was_set(skb));
?
I think it's cleaner than skipping the action here if dissect failed and
propagating a potential bug further.
> > > If this is the only reason for the boolean return value, using
> > > skb_transport_header_was_set() is more standard (I immediately know
> > > what's happening when I read it), slightly less code change and avoids
> > > introducing a situation where the majority of callers ignore a return
> > > value. I think it's preferable. But these merits are certainly
> > > debatable, so either is fine.
> >
> > From my side, I wanted to avoid calling skb_transport_header_was_set
> > twice, so I made skb_try_probe_transport_header return whether it
> > succeeded or not. I think "try" in the function name indicates this idea
> > pretty clearly. This result status is pretty useful, it just happened
> > that it's not needed in many places,
>
> Which is an indication that it's perhaps not needed.
Well, from the point of view of the function, it looks reasonable to
notify the caller whether the call was successful or not... You know,
many functions return error codes.
However, this case is rather special, because it turned out that we
don't actually need that error status immediately, but it can be
requested much later, and we already have skb_transport_header_was_set
for that.
So, considering these points, the return value can be removed, as all
use cases are "probe now, check later", not "probe now and handle errors
immediately".
