This will crash, you didn't test the new code path that does the
kfree_skb().

You should use an SKB helper function which will realloc the headroom
if ETH_HLEN is not available, instead of failing.
...
That is definitely not the only thing this change is doing:
Previously these paths would return "RMNET_MAP_CONSUMED":
Causing this code to return.
Now, instead the code jumps to fail:

@@ -142,7 +142,11 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,

        skb->protocol = htons(ETH_P_MAP);

-       return RMNET_MAP_SUCCESS;
+       return 0;
+
+fail:
+       kfree_skb(skb);
+       return -ENOMEM;
 }


Which frees the SKB.

This is a behavioral change, perhaps a bug fix.

Well, nobody knows because you do not explain this at all in your
commit message.

Do not mix functional changes with cleanups.  If you want to change how
freeing the SKB is done, do it in a separate change from the patch that
removes this enumeration.

Thank you.

Hi David

I will send the change in freeing behavior as a bug fix for net and then
post updates on this series after it.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Reply via email to