On 11/10/2015 11:52 PM, Willem de Bruijn wrote:
if (sock->type == SOCK_DGRAM) {
- err = dev_hard_header(skb, dev, ntohs(proto), addr,
- NULL, tp_len);
+ /* In DGRAM sockets, we expect struct sockaddr_ll was filled
+ * via struct msghdr, so we have dest mac and skb->protocol.
+ * Otherwise there's not too much useful things we can do in
+ * this flush run.
+ */
+ err = dev_hard_header(skb, dev, ntohs(skb->protocol), addr,
+ NULL, tp_len);
This change is not really necessary.
Sure agreed, I found it helpful though. Don't mind removing it.
if (unlikely(err < 0))
return -EINVAL;
- } else if (dev->hard_header_len) {
Why remove the check on hard_header_len?
Hmm, the patch doesn't remove the check (it's moved further below).
- if (ll_header_truncated(dev, tp_len))
- return -EINVAL;
+ } else {
+ /* If skb->protocol is still 0, try to infer/guess it. Might
+ * not be fully reliable in the sense that a user could still
+ * change/race data afterwards, but on the other hand the proto
The race goes away when probing it after the copy in skb_store_bits.
Then it is also certain that tp_len is long enough to hold the entire
link layer header.
The skb_store_bits() is only done in case we do have a dev->hard_header_len
or in case where we run into a possible situation where we have the additional
4 bytes on a full frame. In that case we need to check them properly, which
requires copying, otherwise we don't copy any header.
+ * can be set arbitrarily anyways. We only need to take care
+ * in case of extra large VLAN frames.
+ */
+ if (!skb->protocol && tp_len >= ETH_HLEN)
+ skb->protocol = ((struct ethhdr *)data)->h_proto;
Packet sockets are not restricted to link layer of type Ethernet.
There are a few other points in this file that also cast mac header
to eth_hdr(skb).
Ok, the set doesn't address this assumption which we have elsewhere, too.
Do you suggest to also check on dev->type for these cases?
Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
