On Wed, Oct 27, 2021 at 05:08:07PM +0300, Pavel Tikhomirov wrote: > > > diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c > > > index 2b1d004d6a1f..dfd0a989c4ab 100644 > > > --- a/net/bridge/br_forward.c > > > +++ b/net/bridge/br_forward.c > > > @@ -33,7 +33,10 @@ static inline int should_deliver(const struct > > > net_bridge_port *p, > > > int br_dev_queue_push_xmit(struct net *net, struct sock *sk, struct > > > sk_buff *skb) > > > { > > > skb_push(skb, ETH_HLEN); > > > - if (!(skb->dev->features & NETIF_F_VENET) && > > > + if ( > > > +#ifdef CONFIG_VE > > > + !(skb->dev->ve_features & NETIF_F_VENET) && > > > +#endif > > > !is_skb_forwardable(skb->dev, skb)) > > > goto drop; > > > > Maybe instead of such ugly constructions we could do something like > > > > #ifdef CONFIG_VE > > if (!(skb->dev->ve_features & NETIF_F_VENET)) > > goto drop; > > #endif > > > > if (!is_skb_forwardable(skb->dev, skb)) > > goto drop; > > Wow, your construction is wrong =) > > my: if (!A && !B) > yours: if (!A || !B) > > But I get what you say, and I think "you way" if its don right looks not > much better: > > if (!B) > #ifdef CONFIG_VE > if (!A) > #endif > goto out; >
Indeed, thanks! > > after all the compiler will have to test both conditions so I don't think > > we improve anything if merge them into one ugly if statement. What do you > > think? > > > > > @@ -11425,7 +11425,7 @@ netdev_features_t > > > netdev_increment_features(netdev_features_t all, > > > mask |= NETIF_F_CSUM_MASK; > > > mask |= NETIF_F_VLAN_CHALLENGED; > > > - all |= one & (NETIF_F_ONE_FOR_ALL|NETIF_F_CSUM_MASK|NETIF_F_VIRTUAL) & > > > mask; > > > + all |= one & (NETIF_F_ONE_FOR_ALL | NETIF_F_CSUM_MASK) & mask; > > > all &= one | ~NETIF_F_ALL_FOR_ALL; > > > > This looks somehow suspicious, we no longer consider virtual bit here. > > Are you sure it is expected? > > Yes it's expected: we don't change net_device->features anymore so we can > remove all ugly workarounds we had to make it work with features iteration > logic. Great! > > > +++ b/net/ipv4/ipip.c > > > @@ -384,7 +384,9 @@ static void ipip_tunnel_setup(struct net_device *dev) > > > netif_keep_dst(dev); > > > dev->features |= IPIP_FEATURES; > > > - dev->features |= NETIF_F_VIRTUAL; > > > +#ifdef CONFIG_VE > > > + dev->ve_features = NETIF_F_VIRTUAL; > > > +#endif > > > > Please align assignment to be similar with code around, ie > > > > dev->ve_features = NETIF_F_VIRTUAL; > > I don't think it's strictly required, but ok. Yes please, this makes the code more consistent with previous lines. > > > +++ b/net/netfilter/nf_nat_redirect.c > > > @@ -60,7 +60,7 @@ nf_nat_redirect_ipv4(struct sk_buff *skb, > > > * should use first nonloopback ifa in > > > * the list. > > > */ > > > - if (skb->dev->features & NETIF_F_VENET) { > > > + if (skb->dev->ve_features & NETIF_F_VENET) { > > > > #ifdef CONFIG_VE above? > > It's already surrounded there. Look into nf_nat_redirect_ipv4. Great! Thanks! _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel