On Thu, 7 Apr 2022 19:48:59 -0700
Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> On Thu, 7 Apr 2022 09:16:40 -0700 Stephen Hemminger wrote:
> > > I was wondering why the llc code was getting compiled and it turned out
> > > to be because I had bridging enabled. It turns out to only needs it for
> > > a single function (llc_mac_hdr_init).
>
> > > +static inline int llc_mac_hdr_init(struct sk_buff *skb,
> > > + const unsigned char *sa, const unsigned char *da)
> > > +{
> > > + int rc = -EINVAL;
> > > +
> > > + switch (skb->dev->type) {
> > > + case ARPHRD_ETHER:
> > > + case ARPHRD_LOOPBACK:
> > > + rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > > + skb->len);
> > > + if (rc > 0)
> > > + rc = 0;
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > + return rc;
> > > +}
> > > +
> > >
>
> nit: extra new line
>
> > > -int llc_mac_hdr_init(struct sk_buff *skb,
> > > - const unsigned char *sa, const unsigned char *da)
> > > -{
> > > - int rc = -EINVAL;
> > > -
> > > - switch (skb->dev->type) {
> > > - case ARPHRD_ETHER:
> > > - case ARPHRD_LOOPBACK:
> > > - rc = dev_hard_header(skb, skb->dev, ETH_P_802_2, da, sa,
> > > - skb->len);
> > > - if (rc > 0)
> > > - rc = 0;
> > > - break;
> > > - default:
> > > - break;
> > > - }
> > > - return rc;
> > > -}
>
> There's also an EXPORT somewhere in this file that has to go.
>
> > > /**
> > > * llc_build_and_send_ui_pkt - unitdata request interface for upper layers
> > > * @sap: sap to use
> >
> > You may break other uses of LLC.
> >
> > Why not open code as different function. I used the llc stuff because there
> > were multiple copies of same code (DRY).
>
> I didn't quite get what you mean, Stephen, would you mind restating?
Short answer: it was idea that didn't pan out.
Suggestion: get rid of using LLC in bridge and just rewrite that one place.
