On Thu, Jan 17, 2019 at 4:10 AM Maxim Mikityanskiy <maxi...@mellanox.com> wrote: > > > This is a lot of code change. This would do. > > > > @@ -2434,8 +2434,6 @@ static inline void > > skb_probe_transport_header(struct sk_buff *skb, > > > > if (skb_flow_dissect_flow_keys_basic(skb, &keys, NULL, 0, 0, 0, 0)) > > skb_set_transport_header(skb, keys.control.thoff); > > - else > > - skb_set_transport_header(skb, offset_hint); > > } > > > > Though leaving an unused argument is a bit ugly. For net-next, indeed > > better to clean up (please mark your patchset with net or net-next, > > btw) > > It's for net-next (I'll resend with the correct mark), so I'll stick > with the current implementation.
Absolutely, sounds good. > > > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen- > > netback/netback.c > > > index 80aae3a32c2a..b49b6e56ca47 100644 > > > --- a/drivers/net/xen-netback/netback.c > > > +++ b/drivers/net/xen-netback/netback.c > > > @@ -1105,6 +1105,7 @@ static int xenvif_tx_submit(struct xenvif_queue > > *queue) > > > struct xen_netif_tx_request *txp; > > > u16 pending_idx; > > > unsigned data_len; > > > + bool th_set; > > > > > > pending_idx = XENVIF_TX_CB(skb)->pending_idx; > > > txp = &queue->pending_tx_info[pending_idx].req; > > > @@ -1169,20 +1170,22 @@ static int xenvif_tx_submit(struct xenvif_queue > > *queue) > > > continue; > > > } > > > > > > - skb_probe_transport_header(skb, 0); > > > + th_set = skb_try_probe_transport_header(skb); > > > > Can use skb_transport_header_was_set(). Then at least there is no need > > to change the function's return value. > > I suppose this comment relates to the previous one, and if we do it for > net-next, it's fine to make change I made, isn't it? 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.
